Bug 61079 - Add rel type prerender to distinguish prerender from prefetch
: Add rel type prerender to distinguish prerender from prefetch
Status: RESOLVED FIXED
: WebKit
New Bugs
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2011-05-18 10:57 PST by
Modified: 2011-05-23 12:27 PST (History)


Attachments
Patch (18.49 KB, patch)
2011-05-18 11:48 PST, Gavin Peters
no flags Review Patch | Details | Formatted Diff | Diff
Patch (18.47 KB, patch)
2011-05-18 12:06 PST, Gavin Peters
no flags Review Patch | Details | Formatted Diff | Diff
Patch (18.56 KB, patch)
2011-05-18 12:33 PST, Gavin Peters
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-05-18 10:57:22 PST
Add rel type prerender to distinguish prerender from prefetch
------- Comment #1 From 2011-05-18 11:48:57 PST -------
Created an attachment (id=93958) [details]
Patch
------- Comment #2 From 2011-05-18 11:50:01 PST -------
Also, see how I cleaned things up in CachedResourceRequest.cpp?  I like the new cachedResourceTypeToTargetType a lot better.
------- Comment #3 From 2011-05-18 11:52:22 PST -------
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.
------- Comment #4 From 2011-05-18 11:54:43 PST -------
Unfortunately, this has tickled me before, the style failure doesn't recognise a default parameter as sufficient reason to name an argument.
------- Comment #5 From 2011-05-18 11:56:41 PST -------
(From update of attachment 93958 [details])
bad patch.  new upload pending.
------- Comment #6 From 2011-05-18 12:06:01 PST -------
Created an attachment (id=93961) [details]
Patch
------- Comment #7 From 2011-05-18 12:06:41 PST -------
(From update of attachment 93961 [details])
Better patch.  Thank you dominich@chromium.org.
------- Comment #8 From 2011-05-18 12:07:55 PST -------
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 #9 From 2011-05-18 12:30:07 PST -------
(From update of attachment 93961 [details])
Attachment 93961 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/8707547
------- Comment #10 From 2011-05-18 12:33:45 PST -------
Created an attachment (id=93966) [details]
Patch
------- Comment #11 From 2011-05-18 12:34:03 PST -------
(From update of attachment 93966 [details])
Fix non-prefetching builds!
------- Comment #12 From 2011-05-18 12:36:22 PST -------
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 #13 From 2011-05-20 15:27:06 PST -------
(From update of attachment 93966 [details])
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 #14 From 2011-05-21 07:32:45 PST -------
(From update of attachment 93966 [details])
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.
------- Comment #15 From 2011-05-21 12:23:33 PST -------
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 #16 From 2011-05-21 12:25:07 PST -------
(From update of attachment 93966 [details])
Clearing flags on attachment: 93966

Committed r87020: <http://trac.webkit.org/changeset/87020>
------- Comment #17 From 2011-05-21 12:25:14 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #18 From 2011-05-21 13:25:28 PST -------
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.
------- Comment #19 From 2011-05-23 12:27:40 PST -------
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.