Bug 171720 – [preload] Add media and type attribute support.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yoav Weiss]

[preload] Add media and type attribute support.

[Yoav Weiss]

Created attachment 309156 [details]
Patch

[Yoav Weiss]

This is adding media and type support for the Link preload feature.

A few more tests are needed around unsupported types other than images and around Link header support for the media attribute (especially with various <meta viewport>s), but I wanted to kick off the review to get an opinion on the general approach. I'm hoping to add the required tests shortly.

[Build Bot]

Comment on attachment 309156 [details]
Patch

Attachment 309156 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3678443

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 309160 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 309156 [details]
Patch

Attachment 309156 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3678444

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 309162 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 309156 [details]
Patch

Attachment 309156 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3678439

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 309163 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 309156 [details]
Patch

Attachment 309156 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3678463

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 309164 [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.11.6

[youenn fablet]

(In reply to Yoav Weiss from comment #2)
> This is adding media and type support for the Link preload feature.
> 
> A few more tests are needed around unsupported types other than images and
> around Link header support for the media attribute (especially with various
> <meta viewport>s), but I wanted to kick off the review to get an opinion on
> the general approach. I'm hoping to add the required tests shortly.

General approach seems fine.

I am not sure about video preloading since the backend may not reuse the memory cache.
Jer, Eric, any opinion?

[Yoav Weiss]

(In reply to youenn fablet from comment #11)
> (In reply to Yoav Weiss from comment #2)
> > This is adding media and type support for the Link preload feature.
> > 
> > A few more tests are needed around unsupported types other than images and
> > around Link header support for the media attribute (especially with various
> > <meta viewport>s), but I wanted to kick off the review to get an opinion on
> > the general approach. I'm hoping to add the required tests shortly.

I'm failing to test <meta viewport> with http tests :/
I tried `<!-- webkit-test-runner [ useFlexibleViewport=true ] -->` in a php file, but viewport remains 800px despite a `<meta name="viewport" content="width=160">` directive.

Any advice on how that can be accomplished? Otherwise, I can try different expected results for ios and other platforms, but it's uglier...
 
> 
> General approach seems fine.
> 
> I am not sure about video preloading since the backend may not reuse the
> memory cache.
> Jer, Eric, any opinion?

That's a valid concern, but related to preloading videos in general, right? (so not specific to this patch)

From what I've seen videos weren't downloaded twice, but fragment downloads make it hard to verify through tests.

[youenn fablet]

We had a talk with Yoav and agreed to special case media preload for the moment.

[Yoav Weiss]

Created attachment 310249 [details]
Patch

[Yoav Weiss]

Created attachment 310652 [details]
Patch

[Build Bot]

Comment on attachment 310652 [details]
Patch

Attachment 310652 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3776731

New failing tests:
imported/w3c/web-platform-tests/media-source/mediasource-buffered.html

[Build Bot]

Created attachment 310657 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Yoav Weiss]

Added the necessary tests, so ready for review (test failures on mac-debug seem irrelevant).

I'll disable the video/audio preloading in a separate patch.

[youenn fablet]

Comment on attachment 310652 [details]
Patch

Looks almost good to me.
Some questions and suggestions below.
I am not sure about the tests, why they are skipped by default.
I have not checked yet how precisely they are covering the changes but they looked ok.

View in context: https://bugs.webkit.org/attachment.cgi?id=310652&action=review

> Source/WebCore/css/MediaQueryEvaluator.cpp:741
> +    return MediaQueryEvaluator { "screen", document, &document.renderView()->style() }.evaluate(mediaQueries.get());

Do you know why we are refcounting MediaQuerySet? Can we make it not ref countable?
It seems wasteful to allocate memory that we will free just afterwards.

> Source/WebCore/css/MediaQueryEvaluator.h:73
> +    static bool mediaAttributeMatches(Document&, const String& attributeValue);

space here

> Source/WebCore/html/HTMLImageElement.cpp:165
> +            if (!type.isEmpty() && !MIMETypeRegistry::isSupportedImagePrefixedMIMEType(type))

Why do we need !type.isEmpty() here?

> Source/WebCore/loader/LinkLoader.cpp:89
> +void LinkLoader::loadLinksFromHeader(const String& headerValue, const URL& baseURL, Document& document, PreloadLinkType preloadLinkType)

This parameter was not very clear to me at first.
A suggestion to make things clearer:
Might be clearer to have loadLinksFromHeader with the 3 current parameters and loadMediaLinksIfAny() with no parameters.
loadLinksFromHeader would store any useful media header set for loadMediaLinksIfAny

> Source/WebCore/loader/LinkLoader.cpp:171
> +{

Is there a spec defining which preloads we should do or is it browser dependent?

> Source/WebCore/platform/MIMETypeRegistry.cpp:488
> +    return isSupportedImageMIMEType(mimeType) || equalLettersIgnoringASCIICase(mimeType, "image/svg+xml");

Sounds good to have this method. Seems also like this could be used in HTMLImageElement::bestFitSourceFromPictureElement.
I am not sure about its name and how it is expressing the fact that it allows svg in addition to other image types.

> Source/WebCore/platform/MIMETypeRegistry.cpp:522
> +    return equalLettersIgnoringASCIICase(mimeType, "text/css");

There are other similar checks in the code base, maybe related to the Content-Type which may have other parameters though.
Might want to consider consolidate this as some point.

> LayoutTests/TestExpectations:38
> +http/tests/preload/viewport [ Skip ]

Why skipping the tests?

> LayoutTests/http/tests/preload/media-attribute.html:4
> +<script>

I guess it is too late but this would be great tests to do in LayoutTests/http/wpt and be upstreamed to WPT in the future.

[Yoav Weiss]

(In reply to youenn fablet from comment #19)
> Comment on attachment 310652 [details]
> Patch
> 
> Looks almost good to me.
> Some questions and suggestions below.
> I am not sure about the tests, why they are skipped by default.
> I have not checked yet how precisely they are covering the changes but they
> looked ok.

Only the viewport tests are skipped by default, because they rely on `<meta name=viewport>` which I failed to get working on non-ios builds.

> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=310652&action=review
> 
> > Source/WebCore/css/MediaQueryEvaluator.cpp:741
> > +    return MediaQueryEvaluator { "screen", document, &document.renderView()->style() }.evaluate(mediaQueries.get());
> 
> Do you know why we are refcounting MediaQuerySet? Can we make it not ref
> countable?
> It seems wasteful to allocate memory that we will free just afterwards.

We can by making MediaQuerySet's constructor public. This same pattern is used in many other places in the code.

> 
> > Source/WebCore/css/MediaQueryEvaluator.h:73
> > +    static bool mediaAttributeMatches(Document&, const String& attributeValue);
> 
> space here

Will add one

> 
> > Source/WebCore/html/HTMLImageElement.cpp:165
> > +            if (!type.isEmpty() && !MIMETypeRegistry::isSupportedImagePrefixedMIMEType(type))
> 
> Why do we need !type.isEmpty() here?

Good question. The spec doesn't seem to mandate it.
Avoiding it would mean that for `<picture><source srcset="foo" type><img src="bar"></picture>` "bar" would get picked rather than the current "foo".

If we want to change that behavior, I could do that as a separate patch.

> 
> > Source/WebCore/loader/LinkLoader.cpp:89
> > +void LinkLoader::loadLinksFromHeader(const String& headerValue, const URL& baseURL, Document& document, PreloadLinkType preloadLinkType)
> 
> This parameter was not very clear to me at first.
> A suggestion to make things clearer:
> Might be clearer to have loadLinksFromHeader with the 3 current parameters
> and loadMediaLinksIfAny() with no parameters.
> loadLinksFromHeader would store any useful media header set for
> loadMediaLinksIfAny

Fair enough. Will do

> 
> > Source/WebCore/loader/LinkLoader.cpp:171
> > +{
> 
> Is there a spec defining which preloads we should do or is it browser
> dependent?

The supported mime types we should preload are not defined and are browser dependent.

> 
> > Source/WebCore/platform/MIMETypeRegistry.cpp:488
> > +    return isSupportedImageMIMEType(mimeType) || equalLettersIgnoringASCIICase(mimeType, "image/svg+xml");
> 
> Sounds good to have this method. Seems also like this could be used in
> HTMLImageElement::bestFitSourceFromPictureElement.
> I am not sure about its name and how it is expressing the fact that it
> allows svg in addition to other image types.

Would `isSupportedImageOrSVGMimeType` be better?

> 
> > Source/WebCore/platform/MIMETypeRegistry.cpp:522
> > +    return equalLettersIgnoringASCIICase(mimeType, "text/css");
> 
> There are other similar checks in the code base, maybe related to the
> Content-Type which may have other parameters though.
> Might want to consider consolidate this as some point.

OK.

> 
> > LayoutTests/TestExpectations:38
> > +http/tests/preload/viewport [ Skip ]
> 
> Why skipping the tests?

The viewport tests are skipped for most platforms and enabled for ios and ios-simluator, as I failed to get the viewport reacting to the `meta name=viewport` instructions elsewhere.

> 
> > LayoutTests/http/tests/preload/media-attribute.html:4
> > +<script>
> 
> I guess it is too late but this would be great tests to do in
> LayoutTests/http/wpt and be upstreamed to WPT in the future.

I can move those tests there

[Yoav Weiss]

(In reply to Yoav Weiss from comment #20)
> (In reply to youenn fablet from comment #19)
> > Comment on attachment 310652 [details]
> > Patch
> > 
> > 
> > > Source/WebCore/loader/LinkLoader.cpp:89
> > > +void LinkLoader::loadLinksFromHeader(const String& headerValue, const URL& baseURL, Document& document, PreloadLinkType preloadLinkType)
> > 
> > This parameter was not very clear to me at first.
> > A suggestion to make things clearer:
> > Might be clearer to have loadLinksFromHeader with the 3 current parameters
> > and loadMediaLinksIfAny() with no parameters.
> > loadLinksFromHeader would store any useful media header set for
> > loadMediaLinksIfAny
> 
> Fair enough. Will do
> 

On second thought, since loadLinksFromHeader is static, I think it's better to avoid it keeping state. Would changing the parameter name to MediaAttributeCheck with the values of MediaAttributeEmpty and MediaAttributeNotEmpty make it clearer?

[youenn fablet]

> On second thought, since loadLinksFromHeader is static, I think it's better
> to avoid it keeping state. Would changing the parameter name to
> MediaAttributeCheck with the values of MediaAttributeEmpty and
> MediaAttributeNotEmpty make it clearer?

Oh right!
It seems that we are mixing two things with LinkLoader: actually load of a link element and preloading stuff. That is not easy to understand.
Having preloadIfNeeded return a unique_ptr that is used only in one case seems very strange too.

It seems like all the preloading stuff should be done either in Document, CachedResourceLoader or in its own class that the CachedResourceLoader would own.
Last option would make things much easier to reason about.

[Yoav Weiss]

Created attachment 310859 [details]
Patch

[Build Bot]

Comment on attachment 310859 [details]
Patch

Attachment 310859 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3794104

New failing tests:
http/tests/preload/single_download_preload_headers.php
http/tests/preload/download_resources_from_header_iframe.html
http/tests/preload/single_download_preload_headers_charset.php
http/tests/preload/download_resources_from_invalid_headers.html

[Build Bot]

Created attachment 310863 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 310859 [details]
Patch

Attachment 310859 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3794122

New failing tests:
http/tests/preload/single_download_preload_headers.php
http/tests/preload/download_resources_from_header_iframe.html
http/tests/preload/single_download_preload_headers_charset.php
http/tests/preload/download_resources_from_invalid_headers.html

[Build Bot]

Created attachment 310865 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 310859 [details]
Patch

Attachment 310859 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3794110

New failing tests:
http/tests/preload/single_download_preload_headers.php
http/tests/preload/download_resources_from_header_iframe.html
http/tests/preload/single_download_preload_headers_charset.php
http/tests/preload/download_resources_from_invalid_headers.html

[Build Bot]

Created attachment 310866 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 310859 [details]
Patch

Attachment 310859 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3794224

New failing tests:
http/tests/preload/single_download_preload_headers.php
http/tests/preload/single_download_preload_headers_charset.php
http/tests/preload/viewport/meta-viewport-link-headers.php
http/tests/preload/download_resources_from_invalid_headers.html
http/tests/preload/download_resources_from_header_iframe.html
fetch/closing-while-fetching-blob.html

[Build Bot]

Created attachment 310873 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[youenn fablet]

> > > Source/WebCore/html/HTMLImageElement.cpp:165
> > > +            if (!type.isEmpty() && !MIMETypeRegistry::isSupportedImagePrefixedMIMEType(type))
> > 
> > Why do we need !type.isEmpty() here?
> 
> Good question. The spec doesn't seem to mandate it.
> Avoiding it would mean that for `<picture><source srcset="foo" type><img
> src="bar"></picture>` "bar" would get picked rather than the current "foo".
> 
> If we want to change that behavior, I could do that as a separate patch.

OK, we might want to add a test for that case if it is not tested.
I'll r+ the patch once bots will be happy.

Refactoring things in this area would be nice, the two so different uses of preloadIfNeeded is not great for instance.

[Yoav Weiss]

Created attachment 310889 [details]
Patch

[youenn fablet]

Comment on attachment 310889 [details]
Patch

r=me.
I reviewed the tests more thoroughly. Nice to see some wpt ones!
There is a risk for flakiness on these tests and I am wondering whether we could not improve on that and maybe on readability.
Some suggestions below.

View in context: https://bugs.webkit.org/attachment.cgi?id=310889&action=review

> LayoutTests/ChangeLog:7
> +

ChangeLog should include listing TestExpectations changes.
Can you also add the explanations of why we are skipping them by default, either here or in LayoutTests/TestExpectations, since we should be able at some point to pass these tests in all platforms.

> LayoutTests/http/tests/preload/media-attribute.html:5
> +    var t = async_test('Makes sure that preloaded resources are not downloaded when the media attribute is a mismatch.');

Why do we need to create the test here?
Can we do it in the lat script below instead?

> LayoutTests/http/tests/preload/media-attribute.html:7
> +<link rel=preload href="http://127.0.0.1:8000/resources/square100.png?media_all" as=image media="all">

http://127.0.0.1:8000 is probably unnecessary and we would need to remove it if migrating to wpt.
But wait, isn't it the same as LayoutTests/http/wpt/preload/media-attribute.html?

> LayoutTests/http/tests/preload/media-attribute.html:9
> +<script src="http://127.0.0.1:8000/resources/slow-script.pl?delay=200"></script>

I am not sure this will not end up be flaky.
Can we use onload events on preload link elements instead of window.load?
If we put the link that will get preloaded at the end of the list, we can be confident that other links will not be preloaded. We can of course wait a little bit more to ensure that.

> LayoutTests/http/tests/preload/media-attribute.html:19
> +            document.write('<img src="http://127.0.0.1:8000/resources/square100.png?media_not_all">');

Would it be better if we were doing something like 
<img src="/resources/square100.png?media_all" onload="checkWasPreloaded(); loadNextImage();"> 
<img src="/resources/square100.png?media_not_all" onload="checkWasNotPreloaded(); finish();">

We could create one async_test() to ensure we run everything and several synchronous test() to better describe what is going on.

> LayoutTests/http/tests/preload/media-attribute.html:21
> +            }), 100);

Is 100 meaningful? Why not 0?

> LayoutTests/http/wpt/preload/media-attribute-expected.txt:3
> +PASS Makes sure that preloaded resources are not downloaded when the media attribute is a mismatch. 

Nice to see it in wpt!

[Yoav Weiss]

(In reply to youenn fablet from comment #34)
> Comment on attachment 310889 [details]
> Patch
> 
> r=me.
> I reviewed the tests more thoroughly. Nice to see some wpt ones!

Thanks for reviewing! :)

> There is a risk for flakiness on these tests and I am wondering whether we
> could not improve on that and maybe on readability.
> Some suggestions below.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=310889&action=review
> 
> > LayoutTests/ChangeLog:7
> > +
> 
> ChangeLog should include listing TestExpectations changes.
> Can you also add the explanations of why we are skipping them by default,
> either here or in LayoutTests/TestExpectations, since we should be able at
> some point to pass these tests in all platforms.

Added

> 
> > LayoutTests/http/tests/preload/media-attribute.html:5
> > +    var t = async_test('Makes sure that preloaded resources are not downloaded when the media attribute is a mismatch.');
> 
> Why do we need to create the test here?
> Can we do it in the lat script below instead?

No reason. Moved
> 
> > LayoutTests/http/tests/preload/media-attribute.html:7
> > +<link rel=preload href="http://127.0.0.1:8000/resources/square100.png?media_all" as=image media="all">
> 
> http://127.0.0.1:8000 is probably unnecessary and we would need to remove it
> if migrating to wpt.
> But wait, isn't it the same as
> LayoutTests/http/wpt/preload/media-attribute.html?

git rm fail :/
Removed

> 
> > LayoutTests/http/tests/preload/media-attribute.html:9
> > +<script src="http://127.0.0.1:8000/resources/slow-script.pl?delay=200"></script>
> 
> I am not sure this will not end up be flaky.
> Can we use onload events on preload link elements instead of window.load?
> If we put the link that will get preloaded at the end of the list, we can be
> confident that other links will not be preloaded. We can of course wait a
> little bit more to ensure that.
> 
> > LayoutTests/http/tests/preload/media-attribute.html:19
> > +            document.write('<img src="http://127.0.0.1:8000/resources/square100.png?media_not_all">');
> 
> Would it be better if we were doing something like 
> <img src="/resources/square100.png?media_all" onload="checkWasPreloaded();
> loadNextImage();"> 
> <img src="/resources/square100.png?media_not_all"
> onload="checkWasNotPreloaded(); finish();">
> 
> We could create one async_test() to ensure we run everything and several
> synchronous test() to better describe what is going on.

While we could do that for the preloads which are loaded, we cannot do that for the ones that don't. preload with a non-matching media or type gets dropped silently, and doesn't trigger a load event...

> 
> > LayoutTests/http/tests/preload/media-attribute.html:21
> > +            }), 100);
> 
> Is 100 meaningful? Why not 0?

Nope, changed to 0

> 
> > LayoutTests/http/wpt/preload/media-attribute-expected.txt:3
> > +PASS Makes sure that preloaded resources are not downloaded when the media attribute is a mismatch. 
> 
> Nice to see it in wpt!

[Yoav Weiss]

Created attachment 310903 [details]
Patch

[Yoav Weiss]

(In reply to Yoav Weiss from comment #35)
> (In reply to youenn fablet from comment #34)
> > Comment on attachment 310889 [details]
> > Patch
> > 
> > r=me.
> > I reviewed the tests more thoroughly. Nice to see some wpt ones!
> 
> Thanks for reviewing! :)
> 
> > There is a risk for flakiness on these tests and I am wondering whether we
> > could not improve on that and maybe on readability.
> > Some suggestions below.
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=310889&action=review
> > 
> > > LayoutTests/ChangeLog:7
> > > +
> > 
> > ChangeLog should include listing TestExpectations changes.
> > Can you also add the explanations of why we are skipping them by default,
> > either here or in LayoutTests/TestExpectations, since we should be able at
> > some point to pass these tests in all platforms.
> 
> Added
> 
> > 
> > > LayoutTests/http/tests/preload/media-attribute.html:5
> > > +    var t = async_test('Makes sure that preloaded resources are not downloaded when the media attribute is a mismatch.');
> > 
> > Why do we need to create the test here?
> > Can we do it in the lat script below instead?
> 
> No reason. Moved
> > 
> > > LayoutTests/http/tests/preload/media-attribute.html:7
> > > +<link rel=preload href="http://127.0.0.1:8000/resources/square100.png?media_all" as=image media="all">
> > 
> > http://127.0.0.1:8000 is probably unnecessary and we would need to remove it
> > if migrating to wpt.
> > But wait, isn't it the same as
> > LayoutTests/http/wpt/preload/media-attribute.html?
> 
> git rm fail :/
> Removed
> 
> > 
> > > LayoutTests/http/tests/preload/media-attribute.html:9
> > > +<script src="http://127.0.0.1:8000/resources/slow-script.pl?delay=200"></script>
> > 
> > I am not sure this will not end up be flaky.
> > Can we use onload events on preload link elements instead of window.load?
> > If we put the link that will get preloaded at the end of the list, we can be
> > confident that other links will not be preloaded. We can of course wait a
> > little bit more to ensure that.
> > 
> > > LayoutTests/http/tests/preload/media-attribute.html:19
> > > +            document.write('<img src="http://127.0.0.1:8000/resources/square100.png?media_not_all">');
> > 
> > Would it be better if we were doing something like 
> > <img src="/resources/square100.png?media_all" onload="checkWasPreloaded();
> > loadNextImage();"> 
> > <img src="/resources/square100.png?media_not_all"
> > onload="checkWasNotPreloaded(); finish();">
> > 
> > We could create one async_test() to ensure we run everything and several
> > synchronous test() to better describe what is going on.
> 
> While we could do that for the preloads which are loaded, we cannot do that
> for the ones that don't. preload with a non-matching media or type gets
> dropped silently, and doesn't trigger a load event...
> 

What I could do is count the ones that trigger a load event, and once all of them fired, make sure that none of the ones that are not supposed to load have loaded.

[Yoav Weiss]

Created attachment 310905 [details]
Patch

[Yoav Weiss]

(In reply to Yoav Weiss from comment #37)

> 
> What I could do is count the ones that trigger a load event, and once all of
> them fired, make sure that none of the ones that are not supposed to load
> have loaded.

L patch does that and avoids the timeout related flakiness. Landing

[WebKit Commit Bot]

Comment on attachment 310905 [details]
Patch

Clearing flags on attachment: 310905

Committed r217247: <http://trac.webkit.org/changeset/217247>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.