GitLab

This reverts commit b2457b26.

Reason for revert: suspected of causing assertion failure running merge_web_test_results.py in not_site_per_process_webkit_layout_tests

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Tests%20%28dbg%29%281%29%2832%29/53956

2018-11-02 10:44:57,220 - root: [INFO] Running with isolated arguments
Traceback (most recent call last):
  File "/b/swarming/w/ir/cache/builder/src/third_party/blink/tools/merge_web_test_results.py", line 12, in <module>
    main(sys.argv[1:])
  File "/b/swarming/w/ir/cache/builder/src/third_party/blink/tools/blinkpy/web_tests/merge_results.py", line 775, in main
    assert args.positional
AssertionError


Original change's description:
> Make layout tests use default isolation (e.g. site-per-process on desktop).
> 
> Summary
> =======
> 
> This CL makes layout tests use the default site isolation from the
> platform they are run on (instead of opting out of strict site isolation
> via LayoutTestContentBrowserClient::ShouldEnableStrictSiteIsolation).
> Additionally, on platforms where strict site isolation is enabled,
> layout tests opt into slightly stricter isolation by enabling isolation
> of same-site origins used by Web Platform Tests - this ensures that
> features covered by WPT also get sufficient coverage of out-of-process
> iframes (OOPIFs).
> 
> After this CL, expectations for tests that differ in behavior with and
> without OOPIFs are being moved from
> LayoutTests/FlagExpectations/site-per-process to:
> - LayoutTests/VirtualTestSuites (virtual/not-site-per-process suite)
> - LayoutTests/virtual/not-site-per-process/README.md
> - LayoutTests/TestExpectations and LayoutTests/NeverFixTests
>   ("Site Isolation failures" section)
> 
> 
> Desirability
> ============
> 
> The CL helps with the following:
> 
> - Focusing on testing the mode that is actually shipped to end users
> - Helping ensure that newly developed features get site-per-process
>   coverage without having to set up a separate step (i.e. it is
>   sufficient to set-up a bot that runs layout tests with
>   --enable-features=NewFeature without also having to have a separate
>   test step for runing layout tests with *both*
>   --enable-features=NewFeature *and* --site-per-process
> 
> This CL does *not* help with reducing requirements for CQ capacity,
> because we need to maintain a separate
> not_site_per_process_webkit_layout_tests step to make sure that tests
> pass without isolation (which is the mode Chrome ships on Android).
> Also note that layout test coverage on Android is very sparse - see
> https://groups.google.com/a/chromium.org/d/topic/blink-dev/SOXhTYysYkE/discussion
> 
> 
> Preserving test coverage
> ========================
> 
> Most tests
> ----------
> 
> The CL preserves covering most layout tests with and without OOPIFs, by
> relying on the fact that CQ/waterfall run layout tests on both kinds of
> platforms - ones that default to strict site isolation (desktop
> platforms) and ones that default to no site isolation (Android).
> 
> 
> Tests that used to be excluded FlagExpectations/site-per-process
> ----------------------------------------------------------------
> 
> Around 40 tests fail when run in presence of OOPIFs.  Such tests are
> disabled by this CL by moving test expectations from
> FlagExpectations/site-per-process into the main TestExpectations file.
> The CL preserves non-OOPIF test coverage provided by the disabled tests
> by introducing virtual/not-site-per-process directory which runs all
> such tests with site isolation disabled.  Using a virtual test suites
> for preserving the test coverage relies on the ability to have separate
> test expectations for these tests (i.e. relying on the fact that
> disabling these tests in TestExpectations doesn't disable their
> virtual/not-site-per-process equivalents).
> 
> Note that the CL keeps isolating "oopif.test" site even in
> virtual/not-site-per-process suite.  This site should only be used by
> tests that require an OOPIF.
> 
> 
> Preserving site-per-process-specific test expectations
> ------------------------------------------------------
> 
> Some tests have site-per-process-specific expectations:
> - http/tests/inspector-protocol/network/security-info-on-response.js
> - http/tests/inspector-protocol/network/raw-headers-for-protected-document.js
> 
> The tests above highlight that cross-origin cookies are not displayed in
> site-per-process mode (a known regression tracked by
> https://crbug.com/849483).  This CL preserves expectations and coverage
> by shuffling things around:
> - old, main expectation -> android expectation
> - old, site-per-process expectation -> main expectation
> 
> 
> There is one additional test with site-per-process-specific
> expectations:
> - external/wpt/dom/events/EventListener-addEventListener.sub.window.js
> 
> Unlike the other 2 tests, it seems less important to preserve exact test
> expectations for the case when the test fails with Site Isolation.
> Therefore this test is covered by virtual/not-site-per-process test
> suite and an expectation for this test is added to the main
> TestExpectations.
> 
> 
> Lost test coverage
> ------------------
> 
> Even with extra caution described above, some test coverage may be lost:
> 
> - Features covered by tests only on one kind of platform (e.g. disabled
>   on Android) are at risk of losing OOPIF or non-OOPIF coverage.
> 
> - Before this CL, site-per-process was also applied to all other
>   `virtual/...` test suites.  After this CL,
>   `virtual/not-site-per-process` will not provide such coverage.
> 
> 
> Cleaned up test expectations
> ----------------------------
> 
> Some additional test expectations clean-up is done, while preserving
> test coverage:
> 
> - The http/tests/perf/large-inlined-script.html test has been already
>   present in `SlowTests` and therefore I didn't include this test in the
>   new `virtual/not-site-per-process` suite.
> 
> - The http/tests/devtools/network/network-datareceived.js test was
>   already marked as expecting a `[Failure]` in TestExpectations and
>   therefore I didn't include this test in the new
>   `virtual/not-site-per-process` suite.
> 
> - The http/tests/devtools/console-cross-origin-iframe-logging.js test
>   was already marked as `[Timeout]` in the old TestExpectations but for
>   Win only.  Since I can repro a timeout on Linux (with and without site
>   isolation), I just extended the old expectation to all platforms and I
>   didn't include this test in the new `virtual/not-site-per-process`
>   suite.  I also removed the test from SlowTests (since timeouts in the
>   test are not expected everywhere).
> 
> 
> Bug: 870761
> Change-Id: I74d8ac4ebee8f0402d449fda2cec46ba3b49cf64
> Reviewed-on: https://chromium-review.googlesource.com/c/1302662
> Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#604955}

TBR=dpranke@chromium.org,alexmos@chromium.org,lukasza@chromium.org

Change-Id: I54b6a36922bebfbc8a3f1467bc3d4807c2322604
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 870761
Reviewed-on: https://chromium-review.googlesource.com/c/1315960Reviewed-by: Mike Wittman <wittman@chromium.org>
Commit-Queue: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605039}