[Webkit-unassigned] [Bug 30332] Refactor ProfilesPanel to support multiple profile types
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Oct 14 01:15:13 PDT 2009
https://bugs.webkit.org/show_bug.cgi?id=30332
Pavel Feldman <pfeldman at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #41100|review? |review-
Flag| |
--- Comment #2 from Pavel Feldman <pfeldman at chromium.org> 2009-10-14 01:15:13 PDT ---
(From update of attachment 41100)
I'd suggest that you organize classes related to profile types a bit
differently. I would rename ProfileView.js to CPUProfile.js and put all
CPUProfile-related classes there:
- WebInspector.CPUProfileType (profile type)
- WebInspector.CPUProfile (profile wrapper around the native object)
- WebInspector.CPUProfileView (profile view)
>> createViewForProfile: function(profile)
Define createViewForProfile on profile itself - it anyway requires profile
instance
>> buttonClicked: function()
Given that you only expose buttonClicked to the profile implementor, how do you
treat changing the style for toggle buttons? One of the solutions would be to
re-read buttonTooltip, buttonStyle and buttonCaption upon button click.
>> this.registerProfileType(new WebInspector.ProfilesPanel.CPUProfileType());
and
>> this.registerProfileType(new WebInspector.ProfilesPanel.HeapSnapshotProfileType());
I think these should go into inspector.js instead. profiles panel should not
really be aware of profile types.
>> _forEachProfile: function(func)
I don't think you need this - it just iterates over array. Several diffs in
this patch are driven by this change and are not necessary.
>> this.showProfile(this._profilesIdMap[this._makeKey(uid, WebInspector.ProfilesPanel.CPUProfileType.TYPE_ID)]);
Can you change signature of showProfileById to showProfile(type, uid) or
showProfile(url) or something to remove the hardcoded CPUProfilerType?
>> setRecordingProfile: function(isProfiling)
This method should be defined on CPUProfileType only. Make a call straight into
it from inspector.js. You will need to care about updating button caption I was
talking above.
>> var cpuButton = this._getButton(WebInspector.ProfilesPanel.CPUProfileType.TYPE_ID);
Why not to iterate over all buttons and disable them?
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list