Summary: | [chromium] move webpermissionclient related methods to TestRunner library | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dan Carney <dcarney> | ||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, dglazkov, fishd, jamesr, jochen, tkent+wkapi, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Dan Carney
2013-01-10 03:52:18 PST
Created attachment 182108 [details]
Patch
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 182108 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=182108&action=review > Tools/DumpRenderTree/chromium/TestRunner/public/WebTestDelegate.h:81 > + // the location of the layout test in the file system. no need to put the comment here > Tools/DumpRenderTree/chromium/TestRunner/public/WebTestRunner.h:65 > + virtual WebKit::WebPermissionClient* webPermissions() { return 0; } should be const > Tools/DumpRenderTree/chromium/TestRunner/src/WebPermissions.cpp:35 > +#include "WebTestRunner.h" not needed? > Tools/DumpRenderTree/chromium/TestRunner/src/WebPermissions.cpp:42 > + : m_delegate(delegate) just initialize this to 0 instead of passing 0 as parameter > Tools/DumpRenderTree/chromium/TestRunner/src/WebPermissions.h:39 > +class WebTestRunner; not needed? > Tools/DumpRenderTree/chromium/TestRunner/src/WebPermissions.h:69 > + remove empty line here > Tools/DumpRenderTree/chromium/TestRunner/src/WebPermissions.h:82 > +} and add one here after the } > Tools/DumpRenderTree/chromium/WebViewHost.cpp:1085 > + return normalizeLayoutTestURLInternal(url); instead of copying the code, can you do m_shell->normalizeLayoutTestURL(urL)? Created attachment 182143 [details]
Patch
Comment on attachment 182143 [details] Patch very nice! View in context: https://bugs.webkit.org/attachment.cgi?id=182143&action=review > Tools/ChangeLog:65 > + (normalizeLayoutTestURLInternal): please update the changelog > Tools/DumpRenderTree/chromium/TestRunner/src/TestRunner.cpp:86 > + , m_webPermissions(adoptPtr(new WebPermissions())) nit no () required after WebPermissions Created attachment 182148 [details]
Patch
Comment on attachment 182148 [details] Patch Rejecting attachment 182148 [details] from commit-queue. New failing tests: inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html Full output: http://queues.webkit.org/results/15766891 Comment on attachment 182148 [details] Patch Clearing flags on attachment: 182148 Committed r139355: <http://trac.webkit.org/changeset/139355> All reviewed patches have been landed. Closing bug. |