Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ResourceTiming Issue 138: initiatorType test cases #9986

Merged

Conversation

Projects
None yet
5 participants
@nicjansma
Copy link
Contributor

nicjansma commented Mar 13, 2018

Addresses w3c/resource-timing#138

This PR adds test cases for ResourceTiming for all known download initiators to validate their initiatorType.

The test case resource_initiator_types.html has been updated with the following:

  • <img src="...">: Already existed: img
  • <img srcset="...">: Added: img
  • <link rel="stylesheet" href="...">: Already existed: link
  • <link rel="prefetch" href="...">: Added: link
  • <link rel="preload" href="...">: Added: link
  • <link rel="prerender" href="...">: Added: link
  • <link rel="manfiest" href="...">: Added: link
  • <script src="...">: Already existed: script
  • CSS @font-face { src: url(...) }: Already existed: css
  • CSS background: url(...): Already existed: css
  • CSS @import url(...): Already existed: css
  • CSS cursor: url(...): Added: css
  • CSS list-style-image: url(...): Already existed: css
  • <body background=''>: Already existed: body
  • <input src=''>: Already existed: input
  • XMLHttpRequest.open(...): Already existed: xmlhttprequest
  • <iframe src="...">: Already existed: iframe
  • <frame src="...">: Already existed in resource_frame_initiator_type.html, merged into resource_initiator_types.html so everything is in one place: frame
  • <object>: Already existed: object
  • <svg><image xlink:href="...">: Added: image
  • navigator.sendBeacon(...): Added: beacon
  • fetch(...): Added: fetch
  • <video src="...">: Added: video
  • <video poster="...">: Already existed: video
  • <video><source src="..."></video>: Added: source
  • <audio src="...">: Added audio
  • <audio><source src="..."></audio>: Added source
  • <picture><source srcset="..."></picture>: Added source
  • <picture><img src="..."></picture>: Added img
  • <picture><img srcsec="..."></picture>: Added img
  • <track src="...">: Added track
  • <embed src="...">: Already existed: embed
  • favicon.ico: Added link (though browsers don't seem to load this when in an IFRAME)
  • EventSource: Added eventsource (?)

The test was also updated to only validate that when a URL was fetched, that it has the proper initiatorType. In other words, for things like <link rel="prerender">, if it didn't happen (due to lack of browser support), the test will ignore that initiatorType check.

From w3c/resource-timing#132, here are the tests I didn't add:

  • CSS background: url(data:...) (shouldn't be a download)
  • <a ping="..."> (too late, happens on navigate away, unlikely anyone would try to capture this via ResourceTiming)
  • <video src="..."> with Range: requests (couldn't find a way to force this)
  • <audio src="..."> with Range: requests (couldn't find a way to force this)
  • UA extensions
@nicjansma

This comment has been minimized.

Copy link
Contributor Author

nicjansma commented Mar 13, 2018

Failures from all browsers;

Chrome
image

Edge
image

Firefox
image

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Mar 13, 2018

Build PASSED

Started: 2018-03-16 01:31:46
Finished: 2018-03-16 01:44:40

View more information about this build on:

@yoavweiss
Copy link
Contributor

yoavweiss left a comment

LGTM % nits about picture and srcset not testing the fallback image.

</svg>
<picture>
<source srcset="blue.png?id=picture-source" type="image/png" />
<img src="blue.png?id=picture-img" />

This comment has been minimized.

Copy link
@yoavweiss

yoavweiss Mar 13, 2018

Contributor

picture-img won't normally download here, so that part of the test won't actually run anywhere other than legacy browsers. Might be worth while to add another test case where the source's media attribute doesn't match.

This comment has been minimized.

Copy link
@nicjansma

nicjansma Mar 16, 2018

Author Contributor

Great idea, added via d6079f5

<img src="blue.png?id=picture-img" />
</picture>
<picture>
<img src="blue.png?id=picture-img-src"

This comment has been minimized.

Copy link
@yoavweiss

yoavweiss Mar 13, 2018

Contributor

picture-img-src won't normally download here, so that part of the test won't actually run anywhere other than legacy browsers. Might be worth while to add another test case where the srcset descriptors don't match the env (e.g. a 16x descriptor), to make sure the implicit 1x resource, represented in src, is downloaded and tested.

This comment has been minimized.

Copy link
@nicjansma

nicjansma Mar 16, 2018

Author Contributor
@yoavweiss

This comment has been minimized.

Copy link
Contributor

yoavweiss commented Mar 13, 2018

CSS background: url(data:...) (shouldn't be a download)
(too late, happens on navigate away, unlikely anyone would try to capture this via ResourceTiming)

Makes sense we don't need to test those.

with Range: requests (couldn't find a way to force this)
with Range: requests (couldn't find a way to force this)

This is not testable because the media elements don't expose a way to trigger those range requests, right? If so, that means it's not a WPT issue, but (potentially) a media element issue. At the same time, I don't know if we should expose web APIs that are only important for testing.

UA extensions

That seems out of scope to me.

@yoavweiss yoavweiss merged commit 2e8da45 into web-platform-tests:master Mar 16, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
addEntryIfExists(entries, expected_entries, pathname + 'blue.png?id=video-poster', 'video');
addEntryIfExists(entries, expected_entries, '/media/test.mp4?id=video-src', 'video');
addEntryIfExists(entries, expected_entries, '/media/test.mp4?id=video-source', 'source');
addEntryIfExists(entries, expected_entries, '/media/test.ogg?id=video-source', 'source');

This comment has been minimized.

Copy link
@Hexcles

Hexcles Mar 16, 2018

Member

@nicjansma did you mean test.ogv? There is no test.ogg under media.

This comment has been minimized.

Copy link
@Hexcles

Hexcles Mar 16, 2018

Member

Sent #10077 to you. PTAL.

This comment has been minimized.

Copy link
@nicjansma

nicjansma Mar 16, 2018

Author Contributor

Yes! Thank you for finding this

Hexcles added a commit that referenced this pull request Mar 16, 2018

aarongable pushed a commit to chromium/chromium that referenced this pull request Mar 16, 2018

Mark external/wpt/resource-timing/resource_initiator_types.html as flaky
web-platform-tests/wpt#9986 has introduced some
flakiness to the test that is currently blocking WPT imports.

TBR=lukebjerring

Bug: 822757
Change-Id: Iddc3adf65989506b72fba003a2bbbd8d9cbf062f
No-Try: True
Reviewed-on: https://chromium-review.googlesource.com/966604
Reviewed-by: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#543724}

@nicjansma nicjansma deleted the nicjansma:resource-timing-issue-138 branch Mar 16, 2018

gsnedders added a commit that referenced this pull request Mar 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.