Bug 42332 - WebKitTestRunner needs layoutTestController.dumpResourceLoadCallbacks
Summary: WebKitTestRunner needs layoutTestController.dumpResourceLoadCallbacks
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Mikhail Pozdnyakov
URL:
Keywords: InRadar
Depends on: 93825
Blocks: 61838
  Show dependency treegraph
 
Reported: 2010-07-14 20:55 PDT by Maciej Stachowiak
Modified: 2012-08-14 12:50 PDT (History)
5 users (show)

See Also:


Attachments
patch (26.21 KB, patch)
2012-08-14 00:36 PDT, Mikhail Pozdnyakov
buildbot: commit-queue-
Details | Formatted Diff | Diff
patch (26.19 KB, patch)
2012-08-14 02:40 PDT, Mikhail Pozdnyakov
no flags Details | Formatted Diff | Diff
patch v2 (26.33 KB, patch)
2012-08-14 03:33 PDT, Mikhail Pozdnyakov
kenneth: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
to be landed (27.68 KB, patch)
2012-08-14 06:41 PDT, Mikhail Pozdnyakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maciej Stachowiak 2010-07-14 20:55:52 PDT
WebKitTestRunner needs layoutTestController.dumpResourceLoadCallbacks
Comment 1 Maciej Stachowiak 2010-07-14 20:59:41 PDT
<rdar://problem/8193642>
Comment 2 Mikhail Pozdnyakov 2012-08-12 13:49:32 PDT
Taking it.
Comment 3 Mikhail Pozdnyakov 2012-08-14 00:36:54 PDT
Created attachment 158246 [details]
patch
Comment 4 Build Bot 2012-08-14 00:50:06 PDT
Comment on attachment 158246 [details]
patch

Attachment 158246 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13485802
Comment 5 Mikhail Pozdnyakov 2012-08-14 02:40:16 PDT
Created attachment 158270 [details]
patch

Try again.
Comment 6 Kenneth Rohde Christiansen 2012-08-14 02:57:55 PDT
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 ?
Comment 7 Mikhail Pozdnyakov 2012-08-14 03:33:38 PDT
Created attachment 158280 [details]
patch v2

Kenneth, thanks for review!
Comment 8 Build Bot 2012-08-14 03:57:25 PDT
Comment on attachment 158280 [details]
patch v2

Attachment 158280 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13504126
Comment 9 Kenneth Rohde Christiansen 2012-08-14 03:58:02 PDT
Comment on attachment 158280 [details]
patch v2

Please fix the build
Comment 10 Mikhail Pozdnyakov 2012-08-14 06:41:00 PDT
Created attachment 158315 [details]
to be landed

To be landed if builds on MAC
Comment 11 WebKit Review Bot 2012-08-14 12:49:56 PDT
Comment on attachment 158315 [details]
to be landed

Clearing flags on attachment: 158315

Committed r125593: <http://trac.webkit.org/changeset/125593>
Comment 12 WebKit Review Bot 2012-08-14 12:50:02 PDT
All reviewed patches have been landed.  Closing bug.