r523746 from the blamelist seems related - beccahughes@, could you PTAL?

To run layout tests with --site-per-process use something like:
$ DISPLAY=:20 third_party/WebKit/Tools/Scripts/run-webkit-tests -t <YOUR BUILD DIR - e.g. gn> --no-retry --additional-drt-flag=--site-per-process <TEST TO RUN>

Since this is a WPT test, --site-per-process might not be sufficient for a repro - one might have to isolate WPT-specific origins as shown below (I've confirmed the failure repros locally at r523841)

$ DISPLAY=:20 third_party/WebKit/Tools/Scripts/run-webkit-tests -t dbg --no-retry --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/ --additional-expectations=third_party/WebKit/LayoutTests/FlagExpectations/site-per-process virtual/unified-autoplay/external/wpt/feature-policy/autoplay-allowed-by-feature-policy-attribute.https.sub.html

CL to disable the test / to make the bot green again: https://chromium-review.googlesource.com/c/chromium/src/+/825502

+dpranke@ FYI (the temporary redness of the Site Isolation Linux might affect the ability to turn on --site-per-process layout tests for CQ that you've been working on in  issue 786554 )

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

commit 6240816a6d04ba26e19f6b24aa962d5add8df82f
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Wed Dec 13 19:55:36 2017

Expectations for virtual/unified-autoplay/.../autoplay-allowed-by-feature-policy*

Bug:  794631 
Change-Id: Iacfed87ccd1f96ceafcbf73c70993f79f832fcf0
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/825502
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523851}
[modify] https://crrev.com/6240816a6d04ba26e19f6b24aa962d5add8df82f/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

I believe at this time we shouldn't ship features with potentially broken OOPIF support (OOPIFs started shipping in M56 with the launch of --isolate-extensions and we are preparing to launch Strict Site Isolation to all users soon [aiming for M67]).  Therefore, I think this bug should block issue 779087 (unified autoplay's ship bug) until we investigate more and understand why the test is failing (so far it seems like a legitimate failure around propagation of feature policy).

iclelland@ and lunalu@ - could you please help investigate why the autoplay feature fails the test that checks feature policy propagation into a cross-origin frame?  See the following test from external/wpt/feature-policy/autoplay-allowed-by-feature-policy-attribute.https.sub.html:

  async_test(t => {
    simulateGesture(t, () => {
      test_feature_availability(
          'autoplay', t, cross_origin_src,
          expect_feature_available_default, 'autoplay');
    });
  }, feature_name + ' can be enabled in cross-origin iframe using ' + header);

which fails with the following error message:

  FAIL Feature policy "autoplay" can be enabled in cross-origin iframe using allow="autoplay" attribute assert_true: autoplay expected true got false

FWIW, I've verified that the test fails with OOPIFs at r540963

I think this might have the same underlying issue as  https://crbug.com/690520  -- for a sandboxed frame, the origin as described by the parent is not the same as the origin seen by the frame itself (this is only the case with site-isolation, as the opaque Origin object doesn't survive a round-trip through the browser process)

I'm working on a CL to fix this one, and M57 isn't out of the question.

Thanks!  Updating status per comment 13.  (I assume you meant M67.)  :)

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

I may have jumped too quickly to the conclusion that this was related to  https://crbug.com/690520 .

This test doesn't seem to use sandboxed frames at all, and while it does make use of feature policy, I can't find any other autoplay tests that use --autoplay-policy=document-user-activation-required which *don't* also use feature policy.

I suspect that these failures are an interaction between --autoplay-policy=document-user-activation-required and site isolation, but I haven't confirmed that yet.

(Also, the link in comment #0 indicates that these tests have been failing since they were introduced to the tree; they may never have passed)

Becca -- could you take a look and see if there's an issue with /virtual/unified-autoplay and site isolation, or with the way it is being tested here?

Confirmed this is unrelated to feature policy. It looks like this does not work because HasBeenActivated() on the frame is not properly replicated with site isolation.

Since the test doesn't use subframes at all, all calls to HasBeenActivated() must be have been made from a single LocalFrame.  There is practically no user gesture info propagation here, so correctly updating another (Local/Remote)Frame is not relevant.

I fell like these might be related to these non-site-isolation failures with auto feature policy:
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/TestExpectations?rcl=70b55043fb5df3465f3e7ed81535a0b90b27794c&l=3185

The test does use subframes which are created by featurepolicy.js. I have a fix out to fix those TestExpectations tests and when I was testing them I ran them without --site-per-process and they worked.

When running them with --site-per-process they failed, after doing some digging it looks like when AutoplayPolicy traverses the frame tree on a cross origin subframe the HasBeenActivated bit is not set for the parent frame where it is when site isolation is disabled.

mustaq@, do you think #20 will be fixed by your work on  issue 780556 ?

Perhaps but not sure because I never looked into how feature policy "propagates" into subframes.

Just verified that a patch of mine remotely related to  Issue 780556  fixes the problem, yayy!  I ran the command on #c4 above in Linux with and w/o this patch:
  https://chromium-review.googlesource.com/c/chromium/src/+/973507/

The root cause is that the second remote frame created through featurepolicy.js doesn't currently get activation update for the (second) activation here:
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/external/wpt/feature-policy/autoplay-allowed-by-feature-policy-attribute.https.sub.html?rcl=1337ce842367e0bd9aec9d919d2e7c08592a0daa&l=26

The cause seems significant enough to me.  Is this the only layout test (for autoplay+featurepolicy) known to fail?

Thanks mustaq@!  This is great news :-)


RE: Is this the only layout test (for autoplay+featurepolicy) known to fail?

I only know of the ones explicitly listed in third_party/WebKit/LayoutTests/FlagExpectations/site-per-process and associated with  https://crbug.com/794631  (and I see that you are already removing these test expectations in https://crrev.com/c/973507 - thanks!).

There are a set of tests that are currently disabled in TestExpectations that may be related:

These ones should be tracked by  https://crbug.com/790549 , although they're tagged with   https://crbug.com/788390  instead:

 crbug.com/788390  [ Linux Win ] http/tests/media/autoplay/document-user-activation-cross-origin-feature-policy-delegation.html [ Failure Pass Timeout ]
 crbug.com/788390  [ Linux Win ] http/tests/media/autoplay/document-user-activation-cross-origin-feature-policy-disabled.html [ Failure Pass Timeout ]
 crbug.com/788390  [ Linux Mac Win ] http/tests/media/autoplay/document-user-activation-cross-origin-feature-policy-gesture.html [ Failure Pass Timeout ]
 crbug.com/788390  [ Linux Win ] http/tests/media/autoplay/document-user-activation-cross-origin-feature-policy-header.html [ Failure Pass Timeout ]
 crbug.com/788390  [ Linux ] http/tests/media/autoplay/document-user-activation-feature-policy-alternating.html [ Failure Pass Timeout ]
 crbug.com/788390  [ Linux ] http/tests/media/autoplay/document-user-activation-feature-policy-iframe-no-gesture.html [ Failure Pass Timeout ]
 crbug.com/788390  [ Linux ] http/tests/media/autoplay/document-user-activation-feature-policy-same-origin.html [ Failure Pass Timeout ]
 crbug.com/788390  [ Linux ] http/tests/media/autoplay/document-user-activation-iframe-delegation.html [ Failure Pass Timeout ]

Also these:

# These tests require Unified Autoplay.
crbug.com/779087 external/wpt/feature-policy/autoplay-allowed-by-feature-policy-attribute-redirect-on-load.https.sub.html [ Skip ]
crbug.com/779087 external/wpt/feature-policy/autoplay-default-feature-policy.https.sub.html [ Skip ]
crbug.com/779087 external/wpt/feature-policy/autoplay-disabled-by-feature-policy.https.sub.html [ Skip ]

(None of those are on the site-per-process expectations, though)

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

commit e3b44520f3132325defac71697871d328e28b890
Author: Becca Hughes <beccahughes@chromium.org>
Date: Mon Mar 26 15:53:20 2018

Autoplay: Harden and switch layout tests back on

Harden feature policy layout tests and switch them back on.

These tests are still disabled with site isolation until
HasBeenActivated is replicated.

BUG= 794631 , 790549 

Change-Id: I8fd2be1f9222c71288d5e39e1639c4d990df995b
Reviewed-on: https://chromium-review.googlesource.com/966069
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545804}
[modify] https://crrev.com/e3b44520f3132325defac71697871d328e28b890/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
[modify] https://crrev.com/e3b44520f3132325defac71697871d328e28b890/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/e3b44520f3132325defac71697871d328e28b890/third_party/WebKit/LayoutTests/http/tests/media/autoplay/document-user-activation-cross-origin-feature-policy-delegation.html
[modify] https://crrev.com/e3b44520f3132325defac71697871d328e28b890/third_party/WebKit/LayoutTests/http/tests/media/autoplay/document-user-activation-cross-origin-feature-policy-disabled.html
[modify] https://crrev.com/e3b44520f3132325defac71697871d328e28b890/third_party/WebKit/LayoutTests/http/tests/media/autoplay/document-user-activation-cross-origin-feature-policy-gesture.html
[modify] https://crrev.com/e3b44520f3132325defac71697871d328e28b890/third_party/WebKit/LayoutTests/http/tests/media/autoplay/document-user-activation-cross-origin-feature-policy-header.html
[modify] https://crrev.com/e3b44520f3132325defac71697871d328e28b890/third_party/WebKit/LayoutTests/http/tests/media/autoplay/document-user-activation-feature-policy-alternating.html
[modify] https://crrev.com/e3b44520f3132325defac71697871d328e28b890/third_party/WebKit/LayoutTests/http/tests/media/autoplay/document-user-activation-feature-policy-iframe-no-gesture.html
[modify] https://crrev.com/e3b44520f3132325defac71697871d328e28b890/third_party/WebKit/LayoutTests/http/tests/media/autoplay/document-user-activation-feature-policy-same-origin.html
[modify] https://crrev.com/e3b44520f3132325defac71697871d328e28b890/third_party/WebKit/LayoutTests/http/tests/media/autoplay/document-user-activation-iframe-delegation.html
[modify] https://crrev.com/e3b44520f3132325defac71697871d328e28b890/third_party/WebKit/LayoutTests/http/tests/media/autoplay/resources/autoplay-test.js
[modify] https://crrev.com/e3b44520f3132325defac71697871d328e28b890/third_party/WebKit/LayoutTests/http/tests/media/autoplay/resources/test-autoplay-post-navigation.html

The http/tests/media/autoplay/* tests were flaky so hopefully I have fixed those. However, they definitely do not work with site-per-process enabled. I will see if mustaq's patch fixes those.

Yikes, sorry, my patch doesn't fix these tests!  My local runs missed something!

beccahughes@chromium.org: feel free to grab this bug back from me.  Otherwise I will take a closer look into the tests tomorrow.

Here is what my attempt reveals so far: FrameReplicationState is not propagating correctly to new RemoteFrames.

[Details]
When the layout test creates a remote subframe B (right after the second "click") under the root frame A:
- RenderFrameHostManager::InitRenderView correctly is called once and has
  true == frame_tree_node_->current_replication_state().has_received_user_gesture
  which looks correct.

- Then RenderFrameProxy::CreateFrameProxy is called twice,
  which seems correct too (once for each of A and B).

- But each of those two calls has false == FrameReplicationState.has_received_user_gesture
  which shouldn't be the case.

I now suspect two possibilities:
1. Something to do with passing the struct through a mojo channel?
2. The navigation is resetting the bit?  I know /some/ navigations do it but site-isolation shouldn't change the behavior I believe.

Thanks for the analysis, mustaq@!  Skimming the code quickly, I noticed that has_received_user_gesture is missing from the IPC_STRUCT_TRAITS for FrameReplicationState.  Putting it there makes both of the autoplay tests pass.  I can take this one over and re-enable the tests; I think we'll want to merge this to M66 as well, given that it'll be a short fix.

This explains why has_received_user_gesture wouldn't be propagated properly through IPCs.  Can someone from the autoplay side comment on the impact/scope of this?  Is the autoplay code that depends on this launched in M65?  If so, it'll probably be broken with OOPIFs.

We are launching in M66 so we should make sure we get this merged.

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

commit cac9faa992309767ed7f2a8c873fe9cc26e36e98
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Tue Mar 27 03:29:06 2018

Add missing members to FrameReplicationState struct traits.

Not having FrameReplicationState::has_received_user_gesture in the
struct traits caused it to not propagate through IPCs properly, and
was caught by a couple of autoplay layout test failures, which had
been failing with site isolation enabled.  Thanks to mustaq@ for the
analysis that led to the discovery of this bug.

FrameReplicationState::active_sandbox_flags was similarly missing from
the struct traits, so that member is fixed as well.

Bug:  794631 
Change-Id: I3ebdb51ff887b79c18c4a7c593fac33bb3333a0d
Reviewed-on: https://chromium-review.googlesource.com/981306
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545969}
[modify] https://crrev.com/cac9faa992309767ed7f2a8c873fe9cc26e36e98/content/common/frame_messages.h
[modify] https://crrev.com/cac9faa992309767ed7f2a8c873fe9cc26e36e98/content/common/frame_replication_state.h
[modify] https://crrev.com/cac9faa992309767ed7f2a8c873fe9cc26e36e98/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

Adding blocker label per comment #31.  Fix has already landed; I'll verify and request a merge.

#32 hasn't been out in canary yet, but I also don't know how to verify the fix there given that it only manifested in the layout tests.  The tests are passing and not flaky (checked the flakiness dashboard), and given that the fix is really simple, I'll go ahead and request a merge to M66.  It should be a very safe merge.

Approving merge to M66. Branch:3359

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

commit cf541a8ab70bdeea22185b5d839f311ff6a4e013
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Tue Mar 27 20:40:38 2018

Add missing members to FrameReplicationState struct traits (Merge to M66)

Not having FrameReplicationState::has_received_user_gesture in the
struct traits caused it to not propagate through IPCs properly, and
was caught by a couple of autoplay layout test failures, which had
been failing with site isolation enabled.  Thanks to mustaq@ for the
analysis that led to the discovery of this bug.

FrameReplicationState::active_sandbox_flags was similarly missing from
the struct traits, so that member is fixed as well.

TBR=alexmos@chromium.org

(cherry picked from commit cac9faa992309767ed7f2a8c873fe9cc26e36e98)

Bug:  794631 
Change-Id: I3ebdb51ff887b79c18c4a7c593fac33bb3333a0d
Reviewed-on: https://chromium-review.googlesource.com/981306
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#545969}
Reviewed-on: https://chromium-review.googlesource.com/982510
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#471}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/cf541a8ab70bdeea22185b5d839f311ff6a4e013/content/common/frame_messages.h
[modify] https://crrev.com/cf541a8ab70bdeea22185b5d839f311ff6a4e013/content/common/frame_replication_state.h
[modify] https://crrev.com/cf541a8ab70bdeea22185b5d839f311ff6a4e013/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

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

commit c3a5368ac3723b03e0ac12b1fc0b5520c5bcefc8
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Aug 10 00:25:18 2018

Remove test expectations for tests that have been already fixed.

Some test expectations have been already removed in r545969, but have
been in the meantime readded in r550463.  Some test expectations were
missed and were left in place in r545969.  Some tests are still failing.

FWIW, the affected tests pass when run 20 times locally.

Bug:  794631 
Change-Id: I22ae64eab4a82c92542b65d6d336fa3832742091
Reviewed-on: https://chromium-review.googlesource.com/1169916
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581979}
[modify] https://crrev.com/c3a5368ac3723b03e0ac12b1fc0b5520c5bcefc8/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process