New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 758815 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

web-platform-tests imageset.https.sub.html, regressed by other browser vendors.

Reported by allstars...@gmail.com, Aug 25 2017

Issue description

Chrome team added a wpt for image preloading in https://github.com/w3c/web-platform-tests/blob/master/mixed-content/imageset.https.sub.html
, it was done in 

https://chromium.googlesource.com/chromium/src/+/babb7e6d7db134dab30bfa1419fc0b45744a2645%5E%21/

Later, Firefox teams fixed another wpt, but also modified the helper function, preload_helper.js, in 
https://hg.mozilla.org/mozilla-central/diff/ce6451b2ea64/testing/web-platform/tests/preload/resources/preload_helper.js

However, this modification regressed the original wpt, imageset.https.sub.html,

In the original version of preload_helper.js,
https://github.com/w3c/web-platform-tests/commit/82ed307bfbcfd00a81bc3376c9588b847a88bf36#diff-d3efa803e201ddd27b3c9fde4d16e3cc

It used performance.getEntriesByName.

However later it was changed to 
https://github.com/w3c/web-platform-tests/commit/e7327c5ba23c244c1e98358f9622cd1e84202ba2#diff-d3efa803e201ddd27b3c9fde4d16e3cc

Now it uses 'entry.transferSize'
And from the spec of Timing API, getting 'transferSize' information needs to be same origin for Security Reason.
https://www.w3.org/TR/resource-timing-1/#timing-allow-origin

And the test 'imageset.https.sub.html' now failed because the tese itself is for testing mixed-content-blocker and preloading, it'll load http: images from https: document, they will NOT be same origin and hence failed.

The fix is already pushed in Gecko,
https://hg.mozilla.org/mozilla-central/rev/9f782467df88

Firefox didn't notice this regression since it was disabled in Firefox in the first place.
But I'm surprised Chrome team didn't notice this problem, originally imageset.https.sub.html was pass in the beginning, but Firefox team changed some other wpt code, now it is failed in Chrome.

I am wondering perhaps there should be some kind of working flow to prevent this again (Browser A regressed some wpt in Browser B).

Thanks



 
Cc: y...@yoav.ws mkwst@chromium.org
Components: Blink>PerformanceAPIs Blink>SecurityFeature
Labels: -OS-Linux Hotlist-Interop
Owner: y...@yoav.ws
Yoav mind triaging this bug?

Comment 3 by mkwst@chromium.org, Aug 28 2017

Cc: jeffcarp@chromium.org foolip@chromium.org
+foolip@ and jeffcarp@ for the wider question about regressions: are we just blindly overwriting test expectations?
Cc: robertma@chromium.org qyears...@chromium.org
Adding qyearsley & robertma since expectation updating happens on the importer side.
Sorry for the late reply. Regarding comment 3, I think generally yes. If a test is changed upstream, we will get new baselines for the test during the import. (Not 100% familiar with the expectation updating process, qyearsley@ can confirm.)

I can't think of any obvious ways to improve human awareness of this scenario. The key question is, how can a bot identify regression from the new baselines caused by upstream changes?
The general situation is for web-platform-tests is that we want to maintain and up-to-date copy of all relevant tests at all times, including tests for which Chrome doesn't pass. In order to do this, we need to automatically update expectations when tests get updated.

So in response to mkwst about regressions: We are indeed automatically overwriting test expectations whenever upstream changes cause test results to change.

Currently, OWNERS of imported directories are added to the CC list of import CLs, and the hope is that this will help people discover cases where the results changed in ways that they didn't expect. But currently, most people don't closely inspect import CLs.

I think that if all import CLs are small, then it might be reasonable to expect OWNERS to inspect and review all imports that affect their owned directories. Do you think that would help?
We definitely need a better answer for how Blink developers are to keep their parts of wpt in good shape over time. In https://groups.google.com/a/chromium.org/d/msg/blink-dev/L4-7ZIfMK3Q/stbnOJppAAAJ, robertma@ is asking about this, please weigh in there!
Has the problem with the test itself been fixed?
foolip@: yes, the fix in Gecko has been exported to GitHub and imported to Chromium at https://chromium-review.googlesource.com/c/chromium/src/+/646454

The test in question now passes on Chromium.
Labels: Needs-Milestone TE-NeedsTriageHelp
Unable to triage this issue from TE-End, hence adding "TE-NeedsTriageHelp" for further triage
Status: Fixed (was: Unconfirmed)
Closing per #9.

Sign in to add a comment