Bug 61079

Created attachment 93958 [details] Patch

Also, see how I cleaned things up in CachedResourceRequest.cpp? I like the new cachedResourceTypeToTargetType a lot better.

Attachment 93958 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/loader/cache/CachedResourceLoader.h:73: The parameter name "priority" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.

Unfortunately, this has tickled me before, the style failure doesn't recognise a default parameter as sufficient reason to name an argument.

Comment on attachment 93958 [details] Patch bad patch. new upload pending.

Created attachment 93961 [details] Patch

Comment on attachment 93961 [details] Patch Better patch. Thank you dominich@chromium.org.

Attachment 93961 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/loader/cache/CachedResourceLoader.h:73: The parameter name "priority" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 93961 [details] Patch Attachment 93961 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/8707547

Created attachment 93966 [details] Patch

Comment on attachment 93966 [details] Patch Fix non-prefetching builds!

Attachment 93966 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/loader/cache/CachedResourceLoader.h:73: The parameter name "priority" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 93966 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=93966&action=review > Source/WebCore/html/HTMLLinkElement.cpp:263 > priority = ResourceLoadPriorityLow; We don't want to set priority = ResourceLoadPriorityLow for prerender as well?

Comment on attachment 93966 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=93966&action=review >> Source/WebCore/html/HTMLLinkElement.cpp:263 >> priority = ResourceLoadPriorityLow; > > We don't want to set priority = ResourceLoadPriorityLow for prerender as well? I think no; the VeryLow that it gets by default is probably correct. Priority in WebKit loading only controls when requests are sent to the network layer (the lower the priority, the later they go out), and for prerender being late like a prefetch is probably correct; the prerender fetch then won't interfere as much with main resource loading. In the subresource case, the contrast is that the resource is associated with the current page, and Low priority mirrors what something like an image would get.

The commit-queue encountered the following flaky tests while processing attachment 93966 [details]: http/tests/websocket/tests/handshake-fail-by-cross-origin.html bug 54147 (author: abarth@webkit.org) The commit-queue is continuing to process your patch.

Comment on attachment 93966 [details] Patch Clearing flags on attachment: 93966 Committed r87020: <http://trac.webkit.org/changeset/87020>

All reviewed patches have been landed. Closing bug.

The commit-queue encountered the following flaky tests while processing attachment 93966 [details]: http/tests/websocket/tests/close-event.html bug 61240 (author: yutak@chromium.org) The commit-queue is continuing to process your patch.

I have learned a valuable lesson; always test the chrome side of a webkit patch that has chrome implication before uploading to WebKit: https://bugs.webkit.org/show_bug.cgi?id=61297 catches two mistakes that shouldn't have been in this bug.