Summary: | WebKitTestRunner needs to support layoutTestController.setJavaScriptProfilingEnabled | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Maciej Stachowiak <mjs> | ||||||||||||||
Component: | WebKit2 | Assignee: | Jesus Sanchez-Palencia <jesus> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | abarth, dglazkov, fishd, gustavo, jamesr, jesus, morrita, rakuco, rniwa, tkent, tkent+wkapi, webkit.review.bot, xan.lopez | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | PC | ||||||||||||||||
OS: | OS X 10.5 | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 87284 | ||||||||||||||||
Attachments: |
|
Description
Maciej Stachowiak
2010-07-14 20:53:41 PDT
Added inspector/profiler/cpu-profiler-profiling-without-inspector.html to the skipped list in http://trac.webkit.org/changeset/88597. Created attachment 143883 [details]
WIP for ews testing
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI. Comment on attachment 143883 [details] WIP for ews testing Attachment 143883 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12803245 Comment on attachment 143883 [details] WIP for ews testing Attachment 143883 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12816002 Created attachment 143893 [details]
WIP for ews testing v2
Created attachment 144076 [details]
Patch
Comment on attachment 144076 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144076&action=review > Source/WebCore/ChangeLog:8 > + Add setJavaScriptProfilingEnabled() to window.internals.settings. No new tests, but this Given the last reviews related to window.internals, I think would be good to have some rationale why this function is a good fit for moving there. > Source/WebCore/testing/InternalSettings.cpp:120 > +#if ENABLE(INSPECTOR) > + if (page() && page()->inspectorController()) > + page()->inspectorController()->disableProfiler(); > +#endif Why not save and use the original value like the others? I'm also wondering whether we could have InspectorController::setProfilerEnabled(bool) instead of disableProfiler()/enableProfiler(). (In reply to comment #9) Thanks for having a look! > > Source/WebCore/testing/InternalSettings.cpp:120 > > +#if ENABLE(INSPECTOR) > > + if (page() && page()->inspectorController()) > > + page()->inspectorController()->disableProfiler(); > > +#endif > > Why not save and use the original value like the others? > > I'm also wondering whether we could have InspectorController::setProfilerEnabled(bool) instead of disableProfiler()/enableProfiler(). I like the idea. I will upload a new patch soon with this and also with the default value coming from InspectorController::profilerEnabled(). Created attachment 144126 [details]
WIP for ews testing v3
Comment on attachment 144126 [details] WIP for ews testing v3 Attachment 144126 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12803577 Comment on attachment 144126 [details] WIP for ews testing v3 Attachment 144126 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12801549 Comment on attachment 144126 [details] WIP for ews testing v3 Attachment 144126 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12802540 Created attachment 144145 [details]
Patch
Comment on attachment 144145 [details] Patch Attachment 144145 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12805587 It looks you forgot to remove now-invalidated symbols from WebCore.exp.in. Created attachment 144347 [details]
Patch
Comment on attachment 144347 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144347&action=review Seems reasonable. > Source/WebCore/testing/InternalSettings.cpp:105 > + , m_originalJavaScriptProfilingEnabled((page() && page()->inspectorController()) ? page()->inspectorController()->profilerEnabled() : false) This could be an && instead of a ternary. (In reply to comment #19) > (From update of attachment 144347 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=144347&action=review > > Seems reasonable. > > > Source/WebCore/testing/InternalSettings.cpp:105 > > + , m_originalJavaScriptProfilingEnabled((page() && page()->inspectorController()) ? page()->inspectorController()->profilerEnabled() : false) > > This could be an && instead of a ternary. Thanks! I will fix it before landing. Committed r118705: <http://trac.webkit.org/changeset/118705> |