New issue
Advanced search Search tips

Issue 833423 link

Starred by 1 user
Status: Fixed
Owner: ----
Closed: Nov 5
Cc:
wfh@chromium.org
creis@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 417518



Sign in to add a comment

SiteIsolationPolicyBrowserTest.NoPolicyNoTrialsFlags test is incompatible with not_site_per_process_browser_tests trybots step

Project Member Reported by lukasza@chromium.org, Apr 16 2018 
SiteIsolationPolicyBrowserTest.NoPolicyNoTrialsFlags test looks as follows:

    236 IN_PROC_BROWSER_TEST_F(SiteIsolationPolicyBrowserTest, NoPolicyNoTrialsFlags) {
    237   ASSERT_FALSE(base::CommandLine::ForCurrentProcess()->HasSwitch(
    238       switches::kDisableSiteIsolationTrials));
    239 }

This test is incompatible with the not_site_per_process_browser_tests trybots step that will need to be added after making site-per-process the default:

    https://crrev.com/981019:

        'not_site_per_process_browser_tests': {
          'args': [
            '--disable-site-isolation-trials'
          ],
          'swarming': {
            'shards': 10,
          },
          'test': 'browser_tests',
        },

It seems that removing the test altogether might be undesirable (knowing that kDisableSiteIsolationTrials is not present by default is useful), so I plan to disable the test on Linux (which is the only platform where the not_site_per_process_browser_tests step will run).
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 21 2018 

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

commit fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Sat Apr 21 14:55:20 2018

Make --site-per-process the default on ToT via fieldtrial_testing_config

Note that features::kSitePerProcess is a //chrome-layer feature.
Therefore, after this CL:
- site-per-process is turned-on by default in //chrome layer and above
  (i.e. in code that uses ChromeContentBrowserClient).
- site-per-process stays off by default in //content layer

Also note that fieldtrial_testing_config.json only affects how default
Chromium tests are run, but doesn't affect the default behavior of
Chrome builds shipped to end-users - these will continue to have
site-per-process off by default (and controlled via field trials).

In addition to making site-per-process the default, the CL makes the
following supplementary changes:
- site_per_process_browser_tests test step is removed and replaced
  by not_site_per_process_browser_tests (similarily for a few other
  //chrome-layer test suites where I've verified that
  content::AreAllSitesIsolatedForTesting returns true after this CL).
- A few last-minute changes to disable tests that fail with
  --site-per-process - these changes are limited to site-per-process
  and/or specific platforms.

Bug:  824966 , 831078,  833423 , 833429, 833430
Change-Id: Ie87e48658864203d33d1ae647fb2d98a7c488765
Tbr: zhenw@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/981019
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552589}
[modify] https://crrev.com/fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd/chrome/browser/extensions/extension_crash_recovery_browsertest.cc
[modify] https://crrev.com/fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd/chrome/browser/extensions/extension_messages_apitest.cc
[modify] https://crrev.com/fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd/chrome/browser/policy/site_isolation_policy_browsertest.cc
[modify] https://crrev.com/fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd/chrome/browser/resource_coordinator/resource_coordinator_render_process_probe_browsertest.cc
[modify] https://crrev.com/fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd/content/public/browser/site_isolation_policy.cc
[modify] https://crrev.com/fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd/testing/buildbot/chromium.linux.json
[modify] https://crrev.com/fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd/testing/buildbot/client.v8.chromium.json
[modify] https://crrev.com/fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd/testing/buildbot/test_suite_exceptions.pyl
[modify] https://crrev.com/fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd/testing/buildbot/test_suites.pyl
[modify] https://crrev.com/fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd/testing/buildbot/waterfalls.pyl
[modify] https://crrev.com/fb1ccf02ee8ca79e1404abfd3a3a7d540b7d2dbd/testing/variations/fieldtrial_testing_config.json
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 5 

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

commit d99247935d7369ab3013013fb4c9e265e627742b
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Mon Nov 05 19:36:18 2018

Re-enable SiteIsolationPolicyBrowserTest.NoPolicyNoTrialsFlags.

not_site_per_process_browser_tests step has been removed by r581059 back
in August - therefore the original reason for disabling
SiteIsolationPolicyBrowserTest.NoPolicyNoTrialsFlags test is gone and
the test can be re-enabled everywhere.

Bug:  833423 
Change-Id: If4d197202c18c277e80d6e37760c2e0bb6c4c622
Reviewed-on: https://chromium-review.googlesource.com/c/1315359
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605415}
[modify] https://crrev.com/d99247935d7369ab3013013fb4c9e265e627742b/chrome/browser/policy/site_isolation_policy_browsertest.cc

Comment 3 by lukasza@chromium.org, Nov 5

Status: Fixed (was: Untriaged)
►

Sign in to add a comment