[Webkit-unassigned] [Bug 55794] Web Inspector: Need new graphic icon for garbage collect button

[bugzilla-daemon at webkit.org]

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


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #84802|                            |review-
               Flag|                            |




--- Comment #4 from Pavel Feldman <pfeldman at chromium.org>  2011-03-06 04:51:25 PST ---
(From update of attachment 84802)
View in context: https://bugs.webkit.org/attachment.cgi?id=84802&action=review

This looks good except for a bunch of nits and a lack of test. You could add one into LayoutTests/inspector/timeline/(timeline-gc.html)? You can't use timeline runner harness as is since your action is triggered by the front-end, but you should do a manual timeline test. For that:

1. copy timeline-script-tab-1.html
2. nuke performActions
3. Test would look like:

function test() 
{
    InspectorTest.startTimeline(step1);

    function step1()
    {
        ProfilerAgent.collectGarbage(step2);
    }

    function step2()
    {
        InspectorTest.stopTimeline(step3);
        InspectorTest.printTimelineRecords();
    }

    function step3()
    {
        InspectorTest.printTimelineRecords();
        InspectorTest.completeTest();
    }
}

You will most likely end up with flaky parameters / entries in the timeline dump, you can use InspectorTest.printTimelineRecords's filter / formatter as in timeline-script-tag-1.html to resolve that.

> Source/WebCore/bindings/js/ScriptProfiler.cpp:34
> +#include "GCController.h"

Nit: mind alphabetic order.

> Source/WebCore/inspector/InspectorProfilerAgent.cpp:47
> +#include "ScriptController.h"

Alphabetic order please.

> Source/WebCore/inspector/InspectorProfilerAgent.cpp:48
> +#include "Frame.h"

Is this import needed?

> Source/WebCore/inspector/front-end/Images/garbageCollectButtonGlyph.png:1
> +‰PNG

What generated this patch? Usually webkit-patch, svn-patch or git diff --binary results in image previews available in bugzilla.

> Source/WebCore/inspector/front-end/TimelinePanel.js:212
> +        this.garbageCollectButton = new WebInspector.StatusBarButton(WebInspector.UIString("Collect Garbage"), "garbage-collect-status-bar-item");

You need to add this string to WebCore/English.lproj/localizedStrings.js (watch out, utf-16 binary file).

> Source/WebCore/inspector/front-end/TimelinePanel.js:293
> +    	ProfilerAgent.collectGarbage();

Weird indentation.

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