RESOLVED FIXED 42332
WebKitTestRunner needs layoutTestController.dumpResourceLoadCallbacks 
https://bugs.webkit.org/show_bug.cgi?id=42332
Summary WebKitTestRunner needs layoutTestController.dumpResourceLoadCallbacks
Maciej Stachowiak
Reported 2010-07-14 20:55:52 PDT
WebKitTestRunner needs layoutTestController.dumpResourceLoadCallbacks
Attachments
patch (26.21 KB, patch)
2012-08-14 00:36 PDT, Mikhail Pozdnyakov 		buildbot: commit-queue-
DetailsFormatted DiffDiff
patch (26.19 KB, patch)
2012-08-14 02:40 PDT, Mikhail Pozdnyakov 		no flags
DetailsFormatted DiffDiff
patch v2 (26.33 KB, patch)
2012-08-14 03:33 PDT, Mikhail Pozdnyakov 		kenneth: review+
buildbot: commit-queue-
DetailsFormatted DiffDiff
to be landed (27.68 KB, patch)
2012-08-14 06:41 PDT, Mikhail Pozdnyakov 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Maciej Stachowiak
Comment 1 2010-07-14 20:59:41 PDT
<rdar://problem/8193642>
Mikhail Pozdnyakov
Comment 2 2012-08-12 13:49:32 PDT
Taking it.
Mikhail Pozdnyakov
Comment 3 2012-08-14 00:36:54 PDT
Created attachment 158246 [details] patch
Build Bot
Comment 4 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
Mikhail Pozdnyakov
Comment 5 2012-08-14 02:40:16 PDT
Created attachment 158270 [details] patch Try again.
Kenneth Rohde Christiansen
Comment 6 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 ?
Mikhail Pozdnyakov
Comment 7 2012-08-14 03:33:38 PDT
Created attachment 158280 [details] patch v2 Kenneth, thanks for review!
Build Bot
Comment 8 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
Kenneth Rohde Christiansen
Comment 9 2012-08-14 03:58:02 PDT
Comment on attachment 158280 [details] patch v2 Please fix the build
Mikhail Pozdnyakov
Comment 10 2012-08-14 06:41:00 PDT
Created attachment 158315 [details] to be landed To be landed if builds on MAC
WebKit Review Bot
Comment 11 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>
WebKit Review Bot
Comment 12 2012-08-14 12:50:02 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.