Bug 42332 – WebKitTestRunner needs layoutTestController.dumpResourceLoadCallbacks

• Component: WebKit2
• Status: RESOLVED FIXED

[Maciej Stachowiak]

WebKitTestRunner needs layoutTestController.dumpResourceLoadCallbacks

[Maciej Stachowiak]

<rdar://problem/8193642>

[Mikhail Pozdnyakov]

Taking it.

[Mikhail Pozdnyakov]

Created attachment 158246 [details]
patch

[Build Bot]

Comment on attachment 158246 [details]
patch

Attachment 158246 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13485802

[Mikhail Pozdnyakov]

Created attachment 158270 [details]
patch

Try again.

[Kenneth Rohde Christiansen]

Comment on attachment 158270 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=158270&action=review

Looks pretty ok

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:219
> +static inline bool isFileScheme(WKStringRef scheme)

I think this is called something like isLocal elsewhere in WebCore. You should have a look. Or do you really need it to be file? Like Qt for instnace supports build in resources using qrc://

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:226
> +static inline WTF::String pathSuitableForTestResult(WKURLRef url)

The name of the method doesn't make it obvious to me what the argument represents

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:248
> +    } else {
> +        stringBuilder.append(divider);
> +        stringBuilder.append(pathString);
> +    }

so if you dont find a / you just end up with "/path" Is that what you want? Maybe a small concise comment on how you want the target string would be nice

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:492
> +    // We need to do some error mapping here to match
> +    // the test expectations.

I would just make that one line

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:494
> +        errorDomain = adoptWK(WKStringCreateWithUTF8CString("NSURLErrorDomain"));

So the tests expects Mac error names? ie. NSURL ?

[Mikhail Pozdnyakov]

Created attachment 158280 [details]
patch v2

Kenneth, thanks for review!

[Build Bot]

Comment on attachment 158280 [details]
patch v2

Attachment 158280 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13504126

[Kenneth Rohde Christiansen]

Comment on attachment 158280 [details]
patch v2

Please fix the build

[Mikhail Pozdnyakov]

Created attachment 158315 [details]
to be landed

To be landed if builds on MAC

[WebKit Review Bot]

Comment on attachment 158315 [details]
to be landed

Clearing flags on attachment: 158315

Committed r125593: <http://trac.webkit.org/changeset/125593>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.