While investigating test failures on https://bugs.webkit.org/show_bug.cgi?id=181657 I've seen that the ios-simulator build does not get the Link header values when called loadLinksFromHeader is called for subresources. I'm not sure regarding the cause, but the test seems to pass in all other platform that preconnect is working on.
Looking further into this, this is not ios-simulator specific.
Created attachment 332131 [details] Patch
Comment on attachment 332131 [details] Patch Attachment 332131 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/6192834 New failing tests: http/tests/preload/single_download_preload_headers.php http/tests/preload/preload-encoding.php http/tests/preload/download_resources_from_invalid_headers.html http/tests/preload/download_resources_from_header_iframe.html
Created attachment 332139 [details] Archive of layout-test-results from ews101 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 332131 [details] Patch Attachment 332131 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/6192847 New failing tests: http/tests/preload/single_download_preload_headers.php http/tests/preload/download_resources_from_header_iframe.html http/tests/preload/download_resources_from_invalid_headers.html
Created attachment 332140 [details] Archive of layout-test-results from ews105 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 332131 [details] Patch Attachment 332131 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/6192908 New failing tests: http/tests/preload/single_download_preload_headers.php http/tests/preload/preload-encoding.php http/tests/preload/download_resources_from_invalid_headers.html http/tests/preload/download_resources_from_header_iframe.html
Created attachment 332142 [details] Archive of layout-test-results from ews113 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 332131 [details] Patch Attachment 332131 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/6192927 New failing tests: http/tests/preload/single_download_preload_headers.php http/tests/preload/download_resources_from_header_iframe.html http/tests/preload/download_resources_from_invalid_headers.html
Created attachment 332143 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 332131 [details] Patch Attachment 332131 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/6193585 New failing tests: http/tests/preload/download_resources_from_header_iframe.html http/tests/preload/single_download_preload_headers.php http/tests/preload/preload-encoding.php http/tests/preload/download_resources_from_invalid_headers.html http/tests/misc/bubble-drag-events.html
Created attachment 332145 [details] Archive of layout-test-results from ews206 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Created attachment 334681 [details] Patch
Created attachment 334684 [details] Patch
Finally found the time to get back to this issue. Alex, Chris or Youenn, care to take a look?
Comment on attachment 334684 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334684&action=review > Source/WebCore/ChangeLog:8 > + Triggers Link header processing when the Link headers arrive on a subresource. Sounds fine to me. Some suggestions below. We might want at some point to move some important parts of loading code in NetworkProcess. Preload would also benefit from that since that would make it potentially faster. Current implementation of preloads might not be easy to move since it is placed in various bits. I wonder what your thoughts are there. > Source/WebCore/loader/SubresourceLoader.cpp:363 > + if (m_documentLoader->url() != request().url()) This cheks whether it is a subresource or a document load, right? I would do the check based on options.mode != Mode::Navigation. > Source/WebCore/loader/SubresourceLoader.cpp:364 > + LinkLoader::loadLinksFromHeader(response.httpHeaderField(HTTPHeaderName::Link), m_documentLoader->url(), *m_frame->document(), LinkLoader::MediaAttributeCheck::SkipMediaAttributeCheck); I am a little unclear on why we need SkipMediaAttributeCheck. Is there a spec that requires that somewhere? > LayoutTests/http/tests/preload/link-header-on-subresource.html:10 > +<script src="http://127.0.0.1:8000/resources/slow-script.pl?delay=400"></script> Is there a potential flakiness issue there? Shouldn't we try several times to check for internals.isPreloaded until it succeeds? Ideally, we would want these tests in WPT, right? We could try to use WPT server and doing preloads on URLs served by python scripts that would save some state. We would then get the state making another WPT server request, to check that all preloads are done/not done.
(In reply to youenn fablet from comment #16) > Comment on attachment 334684 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=334684&action=review > > > Source/WebCore/ChangeLog:8 > > + Triggers Link header processing when the Link headers arrive on a subresource. > > Sounds fine to me. Some suggestions below. > > We might want at some point to move some important parts of loading code in > NetworkProcess. > Preload would also benefit from that since that would make it potentially > faster. > Current implementation of preloads might not be easy to move since it is > placed in various bits. > I wonder what your thoughts are there. Faster sounds better :) Is there a design doc you can share on that move? > > > Source/WebCore/loader/SubresourceLoader.cpp:363 > > + if (m_documentLoader->url() != request().url()) > > This cheks whether it is a subresource or a document load, right? > I would do the check based on options.mode != Mode::Navigation. Yeah, that's significantly better :) > > > Source/WebCore/loader/SubresourceLoader.cpp:364 > > + LinkLoader::loadLinksFromHeader(response.httpHeaderField(HTTPHeaderName::Link), m_documentLoader->url(), *m_frame->document(), LinkLoader::MediaAttributeCheck::SkipMediaAttributeCheck); > > I am a little unclear on why we need SkipMediaAttributeCheck. Is there a > spec that requires that somewhere? This is an implementation issue. Link headers for navigations are handled in two separate location depending if they have a media attribute or not. (without media attribute they are handled before the PreloadScanner, and with it they are handled after it, once we know the viewport) For subresources, there's no need for that check, as the viewport is already known. > > > LayoutTests/http/tests/preload/link-header-on-subresource.html:10 > > +<script src="http://127.0.0.1:8000/resources/slow-script.pl?delay=400"></script> > > Is there a potential flakiness issue there? > Shouldn't we try several times to check for internals.isPreloaded until it > succeeds? I can change the test to do that. > > Ideally, we would want these tests in WPT, right? > We could try to use WPT server and doing preloads on URLs served by python > scripts that would save some state. > We would then get the state making another WPT server request, to check that > all preloads are done/not done. There's already a similar test on WPT: https://github.com/w3c/web-platform-tests/blob/master/preload/link-header-on-subresource.html I should probably take some time soon to unite these test suites. I guess I should put some time to unite those two test suites
Created attachment 334919 [details] Patch
> > We might want at some point to move some important parts of loading code in > > NetworkProcess. > > Preload would also benefit from that since that would make it potentially > > faster. > > Current implementation of preloads might not be easy to move since it is > > placed in various bits. > > I wonder what your thoughts are there. > > Faster sounds better :) > Is there a design doc you can share on that move? Not yet unfortunately. We would like to handle all redirections in network process with potential interactions with UIProcess but no interaction with WebProcess.
Comment on attachment 334919 [details] Patch Clearing flags on attachment: 334919 Committed r229196: <https://trac.webkit.org/changeset/229196>
All reviewed patches have been landed. Closing bug.
<rdar://problem/38097208>