New issue
Advanced search Search tips

Issue 819800 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug

Blocking:
issue 477150



Sign in to add a comment

setting-allowpaymentrequest-timing.https.sub.html fails with Site Isolation

Project Member Reported by lukasza@chromium.org, Mar 7 2018

Issue description

REPRO:

1. Build WPT in chromium:
   $ ninja -C out/gn -j 500 -l 25 blink_tests
2. Run setting-allowpaymentrequest-timing.https.sub.html with Site Isolation (the long --additional-driver-flag=--isolate-origins=... argument):

$ DISPLAY=:20 third_party/WebKit/Tools/Scripts/run-webkit-tests -t gn --no-retry --additional-driver-flag=--site-per-process --additional-driver-flag=--isolate-origins=http://www.web-platform.test:8001/,http://www1.web-platform.test:8001/,http://www2.web-platform.test:8001/,http://xn--n8j6ds53lwwkrqhv28a.web-platform.test:8001/,http://xn--lve-6lad.web-platform.test:8001/,http://www.web-platform.test:8081/,http://www1.web-platform.test:8081/,http://www2.web-platform.test:8081/,http://xn--n8j6ds53lwwkrqhv28a.web-platform.test:8081/,http://xn--lve-6lad.web-platform.test:8081/,https://www.web-platform.test:8444/,https://www1.web-platform.test:8444/,https://www2.web-platform.test:8444/,https://xn--n8j6ds53lwwkrqhv28a.web-platform.test:8444/,https://xn--lve-6lad.web-platform.test:8444/ --time-out-ms=30000 external/wpt/payment-request/allowpaymentrequest/setting-allowpaymentrequest-timing.https.sub.html


EXPECTED BEHAVIOR: Test passes with and without Site Isolation

ACTUAL BEHAVIOR: Test fails with Site Isolation
 

Comment 1 by nasko@chromium.org, Mar 7 2018

Labels: Test-Layout
I am a little bit confused by the test.  The test seems to explicitly expect an exception:

  window.onmessage = t.step_func_done((e) => {
    // Some ad-hoc, printf-style debugging from lukasza@:
    testRunner.logToStderr("setting-allowpaymentrequest-timing.https.sub.html:25: e.data.message = " + e.data.message);

    assert_equals(e.data.message, 'Exception');
    assert_array_equals(e.data.details, [true /* ex instanceof DOMException */, DOMException.SECURITY_ERR, 'SecurityError']);
  });

But the test passes without --site-per-process, when there is no exception (e.g. when e.data.message == "Exception" which I would have though violates |assert_equals| above), but fails with --site-per-process when an exception is thrown here:

    #1 blink::DOMException::DOMException(...) third_party/WebKit/Source/core/dom/DOMException.cpp:189:33
    #2 blink::DOMException::Create(...) third_party/WebKit/Source/core/dom/DOMException.cpp:203:14
    #3 blink::V8ThrowDOMException::CreateDOMException(...) third_party/WebKit/Source/bindings/core/v8/V8ThrowDOMException.cpp:62:33
    #4 blink::ExceptionState::ThrowSecurityError(...) third_party/WebKit/Source/bindings/core/v8/ExceptionState.cpp:90:7
    #5 blink::ExceptionState::ThrowSecurityError(...) third_party/WebKit/Source/bindings/core/v8/ExceptionState.cpp:58:3
    #6 blink::PaymentRequest::PaymentRequest(...) third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:0:21
    #7 blink::PaymentRequest::Create(...) third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:806:14
Cc: iclell...@chromium.org
iclelland@ - I wonder if you could please take a quick look?  Do we know of any known feature policy issues that might prevent |iframe.allowPaymentRequest = true| from propagating into an OOPIF?
Cc: zcorpan@gmail.com
+zcorpan@

Not sure if this is relevant for the investigation here, but I don't really understand how the synchronization/timing of the test are supposed to work - I don't see how 1) the timing of |iframe.allowPaymentRequest = true| is synchronized with 2) the timing of 2a) iframe load and 2b) postMessage to the iframe.
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 12 2018

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

commit 961c6c6cc919c92c4a8e4c51656646730c2a344c
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Mon Mar 12 21:24:17 2018

Triaging layout test failures from FlagExpectations/site-per-process

http/tests/devtools/appcache/appcache-iframe-manifests.js:
- This test was associated with crbug.com/678481 and that
  bug is now fixed, and the test-related expectation has
  already been removed from LayoutTests/TestExpectations
  in r527386

http/tests/media/autoplay/document-user-activation-*
- These tests are flaky regardless of site-per-process
  and are already covered by LayoutTests/TestExpectations.
  This CL just extends TestExpectations entry to also cover Win.

external/wpt/payment-request/allowpaymentrequest/setting-allowpaymentrequest-timing.https.sub.html
- New bug has been opened prior to this CL: https://crbug.com/819800

external/wpt/fullscreen/api/element-ready-check*
- New bug has been opened prior to this CL:  https://crbug.com/820617 

external/wpt/css/css-fonts/font-display/font-display.html
- This test is simply slow and often takes more than 5 seconds to
  finish - see https://crbug.com/816026#c7
- The timeout of this test is (since 2018-03-12) covered by the main
  LayoutTests/TestExpectations (see r542490).

Other entries removed by this CL simply seem to have "healed" themselves
and are currently passing on the waterfall and on the tryjobs.

Bug: 477150,  794631 , 678481,  788390 
Bug: 819800,  820617 ,  788390 ,  801992 
Change-Id: If24ec321df9593ab217f50b7f1b39b3496faceef
Reviewed-on: https://chromium-review.googlesource.com/957683
Commit-Queue: Ɓukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542601}
[modify] https://crrev.com/961c6c6cc919c92c4a8e4c51656646730c2a344c/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
[modify] https://crrev.com/961c6c6cc919c92c4a8e4c51656646730c2a344c/third_party/WebKit/LayoutTests/TestExpectations

Comment 6 by zcorpan@gmail.com, Apr 16 2018

The timing in the test is as follows.

T0. The <iframe> is inserted into the document (line 29). The allowpaymentrequest flag is not set. See
https://html.spec.whatwg.org/#the-iframe-element:process-the-iframe-attributes
->
https://html.spec.whatwg.org/#creating-a-new-browsing-context
->
https://html.spec.whatwg.org/#set-the-allow*-flags (step 3, no-op)

T1. The 10ms timeout fires, which adds the allowpaymentrequest attribute, but does not change the flag (because there's no new navigation).

T2. The iframe 'load' fires when the resource has loaded (which is delayed 3 seconds, by `?pipe=trickle(d3)`, see https://wptserve.readthedocs.io/en/latest/pipes.html#trickle ). This sends the postMessage to the framed page (line 21).

T3. The framed page receives the message, and responds with the result of constructing PaymentRequest.

T4. onmessage is invoked, an exception is expected because the allowpaymentrequest flag has not been set since T0.


However, I realize now that the spec also runs "navigate" from inserting the iframe, which eventually reaches https://html.spec.whatwg.org/#read-html 
->
https://html.spec.whatwg.org/#initialise-the-document-object
->
https://html.spec.whatwg.org/#set-the-allow*-flags

That would go between T1 and T2. :-/ Possible spec bug.

Sign in to add a comment