[webkit-reviews] review requested: [Bug 110466] Web Inspector: Remove CPU profile from a group causes exception : [Attachment 189533] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Feb 22 01:54:51 PST 2013


Alexei Filippov <alph at chromium.org> has asked  for review:
Bug 110466: Web Inspector: Remove CPU profile from a group causes exception
https://bugs.webkit.org/show_bug.cgi?id=110466

Attachment 189533: Patch
https://bugs.webkit.org/attachment.cgi?id=189533&action=review

------- Additional Comments from Alexei Filippov <alph at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=189533&action=review


>> Source/WebCore/inspector/front-end/ProfilesPanel.js:559
>> +	    if
(!WebInspector.ProfilesPanelDescriptor.isUserInitiatedProfile(profile.title) &&
!profile.isTemporary) {
> 
> Why do we skip temporary profiles?

I don't want'em to form groups.

>> LayoutTests/inspector/profiler/cpu-profiler-profile-removal.html:18
>> +	InspectorTest.runProfilerTestSuite([
> 
> You don't need the suite since there is only one test.

I made it consistent to other tests, e.g. cpu-profiler-profiling.html
Well, removed the suite usage.

>> LayoutTests/inspector/profiler/cpu-profiler-profile-removal.html:24
>> +			profiles._removeProfileHeader(profiles._profiles[0]);
> 
> Can you check that the group and all profiles are actually removed from the
profiles panel sidebar?

_profiles should be empty because of the loop condition. Added a check for
_profileGroups.


More information about the webkit-reviews mailing list