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

Issue 664256 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Layout Test imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects.html is failing

Project Member Reported by dpranke@chromium.org, Nov 10 2016

Issue description

The following layout test is failing on all platforms:

imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects.html

It looks like it was passing yesterday, and then started failing one way when tkent's change to enable wptserve landed in https://codereview.chromium.org/2482793002/, and then started failing a different way when I reverted that change in https://codereview.chromium.org/2495613002/

It's not immediately obvious to me what changed, but it's possible some CL landed while wptserve was enabled that changed the non-wptserve behavior.

I'm filing this bug to track down what that was.

In addition, it looks like the layout-test-results actual results .json file might be busted, because if you click on the "expected failures" checkbox in the results.html UI, you'll see a bunch of "expected failures" that are listed as Pass/Pass and that don't seem to be failing:

https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Linux_Precise/2241/layout-test-results/results.html

this is probably a separate bug, but I'm mentioning it here because it's adding to my confusion :).
 
copy/pasting chat log between qyearsley@ and I from a few minutes ago for the record:

Dirk Pranke
hola.
I'm chromium sheriff today (as you may have seen)
and I've noticed something weird. Can you look at it for me?
10:48
Quinten Yearsley
Sure
10:48
Dirk Pranke
https://sheriff-o-matic.appspot.com/chromium
you can see that imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects.html is failing
and I think it started failing when I reverted the "enable wptserve by default" CL
however, looking at the flakiness dashboard results
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects.html
it looks like it's been failing since the original change landed.
The test is listed as [ Fail Timeout ] in WPTServeExpectations
so I guess I would expect it to fail when the switch was on
but I'm not sure why it's failing now?
10:55
Quinten Yearsley
So, it appeared to start failing when wptserve was turned on by default; but it's still failing after the revert
10:55
Dirk Pranke
that's what I'm seeing, yes
though the test didn't actually show up as failing in the builds because of the WPTServeExpectations entry (i.e., it was an expected failure at that time, now it's an unexpected failure.
11:01
Quinten Yearsley
And also the failure diff has changed, it's failing for a different reason now than when wptserve was enabled
So sometime between enabling wptserve and now, something else changed?
11:01
Dirk Pranke
hmm
let me look again
11:02
Quinten Yearsley
Diff after revert:
https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Linux_Trusty/19788/layout-test-results/imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects-pretty-diff.html

Diff after enabling wptserve before revert:
https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Linux_Precise/2242/layout-test-results/imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects-pretty-diff.html
11:07
Dirk Pranke
hmm
11:07
Quinten Yearsley
Last change to that test was https://codereview.chromium.org/2482213003 (r430880), and enabling wptserve was r431186 -- so it appears there were no changes to that file within the mystery window (between enabling wptserve and reverting it)
11:08
Dirk Pranke
in build 2241, if you look at the layout-test-results with "expected failures" enabled
https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Linux_Precise/2241/layout-test-results/results.html
it says that there are 4653 failures, and lists a lot of tests that are actual pass/expected pass
that seems busted, too?
11:10
Quinten Yearsley
Ah, right, that does seem incorrect
11:15
Dirk Pranke
Okay, this is strange enough that I'm going to file a bug and suppress the failure for now, and you and tkent and dig in and figure out what happened.
11:16
Quinten Yearsley
Sounds good
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 10 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9c9e46be729f430ea0f6f0c50e70d19c8fd8f2a7

commit 9c9e46be729f430ea0f6f0c50e70d19c8fd8f2a7
Author: dpranke <dpranke@chromium.org>
Date: Thu Nov 10 19:32:54 2016

Suppress weird WPT cross-origin-objects.html layout test failure.

TBR=qyearsley@chromium.org, tkent@chromium.org
BUG= 664256 
NOTRY=true

Review-Url: https://codereview.chromium.org/2486373003
Cr-Commit-Position: refs/heads/master@{#431316}

[modify] https://crrev.com/9c9e46be729f430ea0f6f0c50e70d19c8fd8f2a7/third_party/WebKit/LayoutTests/TestExpectations

Cc: littledan@chromium.org
Status: Assigned (was: Untriaged)
On WebKit Linux Precise, build 2248 is the first one that has the new diff:

-FAIL [[SetPrototypeOf]] should throw assert_throws: proto set on cross-origin Window function "function () { C.__proto__ = new Object(); }" did not throw
+FAIL [[SetPrototypeOf]] should throw assert_throws: proto set on cross-origin Location function "function () { C.location.__proto__ = new Object(); }" did not throw

Before:
https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Linux_Precise/2247/layout-test-results/imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects-pretty-diff.html

After:
https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Linux_Precise/2248/layout-test-results/imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects-pretty-diff.html

Blamelist:
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Precise/builds/2248

Most interesting commit in that range is c65700010f3c557867e23b24373499170959f845, but I'm not sure whether that change explains this change in imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects.html yet.

One way to investigate would be to locally run this test with and without a revert of that change; if the test result changes, then that indicates that that change is the cause. Then, the most obvious fix would be to rebaseline the test.
Yup, I expect c6570001 is to blame.
Yep, it's my patch. Reverting it shows that the old expectations are found.

It looks like the change resulted in a test which was previously failing to pass. The assertion on line 131 of html/browsers/origin/cross-origin-objects/cross-origin-objects.html was previously failing, but it now throws as expected, and this leads to the assertion on 132 being reached (and then failing as it would have done before). I think the appropriate thing to do here is to change the test expectations to what is output now.

What I don't understand is why there are so many failing lines in this test. It seems like it would be a good idea for someone to look into that.
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6e4c3b357983e3d687d135297ed9f0f7c44031f5

commit 6e4c3b357983e3d687d135297ed9f0f7c44031f5
Author: littledan <littledan@chromium.org>
Date: Fri Nov 11 04:26:45 2016

Update test expectations for a test which passes slightly more

Freezing the prototype chain of the global object made a line of a test
pass which was checking that, cross-origin, this chain cannot be manipulated.
This patch updates the test expectations for the test doing better.

BUG= 664256 , v8:5149 

Review-Url: https://codereview.chromium.org/2486943006
Cr-Commit-Position: refs/heads/master@{#431496}

[modify] https://crrev.com/6e4c3b357983e3d687d135297ed9f0f7c44031f5/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/6e4c3b357983e3d687d135297ed9f0f7c44031f5/third_party/WebKit/LayoutTests/imported/wpt/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt

Status: Fixed (was: Assigned)
Thanks littledan@!

Sign in to add a comment