[Webkit-unassigned] [Bug 30520] Enable creation of custom SidebarTreeElements for different ProfileTypes

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=30520


Timothy Hatcher <timothy at hatcher.name> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #41433|review?                     |review-
               Flag|                            |




--- Comment #2 from Timothy Hatcher <timothy at hatcher.name>  2009-10-19 11:40:48 PDT ---
(From update of attachment 41433)
> +    sidebarTreeElementForProfile: function(profile)
> +    {
> +        return new WebInspector.ProfileSidebarTreeElement(profile);
> +    },

Why does this return a new tree element each time? Shouldn't this return the
same one each time? If not, then this function needs a "create" prefix.


> +    createView: function(profile)
> +    {
> +        return undefined;
> +    },
> +
> +    sidebarTreeElementForProfile: function(profile)
> +    {
> +        return undefined;
> +    },

It is better to use null in cases like this. But why do these exist?


> +    viewForProfile: function(profile)
> +    {
> +        if (!profile._profileView)
> +            profile._profileView = this.createView(profile);
> +        return profile._profileView;
>      }

I am confused why there are multiple versions of these functions. Also this
clearly calls "this.createView", which just returns undefined. Can you explain
this? I don't think the functions that just return "undefined" are needed or
useful.


> +    _parseKey: function(key)
> +    {
> +        var match = key.match(/^([^\/]+)\/([^\/]+)$/);
> +        if (match)
> +            return [unescape(match[1]), unescape(match[2])];
> +        return undefined;
> +    },

Better to return null.


> +    _viewForProfile: function(profile)
> +    {
> +        for (var key in this._profilesIdMap) {
> +            if (this._profilesIdMap[key] === profile) {
> +                var parsedKey = this._parseKey(key);
> +                if (parsedKey)
> +                    return this.getProfileType(parsedKey[1]).viewForProfile(profile);
> +            }
> +        }
> +        return undefined;
> +    },

Better to return null.


r-. This needs clerified and not have functions that do nothing but return
undefined. (Or explained with good comments.)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.