Bug 106548 – [chromium] move webpermissionclient related methods to TestRunner library

• Component: New Bugs
• Status: RESOLVED FIXED

[Dan Carney]

[chromium] move webpermissionclient related methods to TestRunner library

[Dan Carney]

Created attachment 182108 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] 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)?

[Dan Carney]

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

[Dan Carney]

Created attachment 182148 [details]
Patch

[WebKit Review Bot]

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

[WebKit Review Bot]

Comment on attachment 182148 [details]
Patch

Clearing flags on attachment: 182148

Committed r139355: <http://trac.webkit.org/changeset/139355>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.