pastarmovj@ - does this proposal sound good from Enterprise Policy perspective?  Is it okay to show the "stability and security will suffer" butter bar warning to enterprise users if their enterprise admin opts out of Site Isolation?

Today, Site Isolation can still be disabled via Enterprise Policy, if the admin explicitly sets site-per-process enterprise policy to false, or sets isolate-origin enterprise policy to an empty list.  See the following code in chrome/browser/chrome_browser_main.cc:

  // The admin should also be able to use these policies to override trials that
  // will try to turn site isolation on per default.
  // Note that disabling either SitePerProcess or IsolateOrigins via policy will
  // disable both types of field trials.
  if ((local_state->IsManagedPreference(prefs::kSitePerProcess) &&
       !local_state->GetBoolean(prefs::kSitePerProcess)) ||
      (local_state->IsManagedPreference(prefs::kIsolateOrigins) &&
       local_state->GetString(prefs::kIsolateOrigins).empty())) {
    base::CommandLine::ForCurrentProcess()->AppendSwitch(
        switches::kDisableSiteIsolationTrials);
  }

Note that the code above will disable not just the *trials* of site isolation, but also the //chrome-layer default that enables site isolation on desktop.  The switches::kDisableSiteIsolationTrials is not very accurately named at this point :-(  (should probably be renamed to switches::kDisableSiteIsolation, but this is problematic for chrome://flags backcompatiblity).

Showing this warning to the _user_ when their administrator has disabled site isolation sounds wrong to me. We could file a FR to show in in the Administrator console instead?

Yes, definitely should not show this butter bar if site isolation is disabled by policy.

Hmmm... we want to show the "stability and security will suffer" when
1. The individual user uses chrome://flags/#site-isolation-trial-opt-out
2. The individual user uses --disable-site-isolation-trials switch
3. The enterprise admin disables Site Isolation by setting SitePerProcess to false or setting IsolateOrigins enterprise policy to an empty list.  Under the cover this injects --disable-site-isolation-trials switch.

It is not possible to distinguish at //content-level between #2 and #3 (both look the same - the --disable-site-isolation-trials switch is present) :-(

RE: We could file a FR to show in in the Administrator console instead?

How to proceed with this?  If the warning has been shown in the Administrator's console for a milestone (say, in M72), then is it okay to proceed with the individual user warning (either in the same milestone or 1 milestone later)?
 

I agree with pmarko@ and atwilson@ that we should not push this message to users when it's set as an enterprise policy (even after a delay).  We can use a different flag for it in the content layer if needed.

Showing a warning in the Administrator console sounds good to me.

If we want to discourage it further than that, we should look into solving any remaining issues that are causing enterprises to disable the policy (assuming there are any) and then possibly remove the ability to disable it via enterprise policy.  I don't think showing the warning to users is the right approach.

Sorry for the delayed reply here. +1 to not showing the bar. If the admin is given the choice to make a configuration we should not second guess them. If we feel they should not have this choice we can deprecate the policy and remove it. I guess for now we are out f this option due to fears of stability and/or performance implication fears.

Thanks for the feedback (and for the suggestion in #c6 to use a different flag/switch when Site Isolation is disabled via Enterprise Policy).  I've put together a CL that starts showing the warning for chrome://flags/#site-isolation-trial-opt-out and --disable-site-isolation-trials, but doesn't show the warning when Site Isolation is disabled via Enterprise Policy.  See: https://crrev.com/c/1318443.

I've opened b/119049821 to track the work of adding a similar warning in the Administrator console (for cases when Site Isolation is disabled via Enterprise Policy).

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

commit 8727349f7bd30918e9ed246567fc5fcd94583b6f
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Tue Dec 04 22:28:57 2018

Show "security will suffer" warning for switches::kDisableSiteIsolation.

After this CL, a "stability and security will suffer" infobar warning
will be shown if an individual user:
1. Uses --disable-site-isolation-trials (aka
   switches::kDisableSiteIsolation) cmdline switch
2. Uses chrome://flags/#site-isolation-trial-opt-out

To avoid showing the warning if Site Isolation is opted out via
Enterprise Policy, the CL introduces a separate API for disabling
Site Isolation and switches Enterprise Policy scenarios to use
the new API .  This avoids showing the new warning in Enterprise
Policy scenarios.

Bug:  900998 
Change-Id: I93bf3d9233c09072c85deaceda0659b9e25a2075
Reviewed-on: https://chromium-review.googlesource.com/c/1318443
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613727}
[modify] https://crrev.com/8727349f7bd30918e9ed246567fc5fcd94583b6f/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/8727349f7bd30918e9ed246567fc5fcd94583b6f/chrome/browser/chromeos/login/session/user_session_manager.cc
[modify] https://crrev.com/8727349f7bd30918e9ed246567fc5fcd94583b6f/chrome/browser/policy/site_isolation_policy_browsertest.cc
[modify] https://crrev.com/8727349f7bd30918e9ed246567fc5fcd94583b6f/chrome/browser/ui/startup/bad_flags_prompt.cc
[modify] https://crrev.com/8727349f7bd30918e9ed246567fc5fcd94583b6f/content/browser/isolated_origin_browsertest.cc
[modify] https://crrev.com/8727349f7bd30918e9ed246567fc5fcd94583b6f/content/public/browser/site_isolation_policy.cc
[modify] https://crrev.com/8727349f7bd30918e9ed246567fc5fcd94583b6f/content/public/common/content_switches.cc
[modify] https://crrev.com/8727349f7bd30918e9ed246567fc5fcd94583b6f/content/public/common/content_switches.h

xiyuan@, I wonder if you could please help with switching to using the new cmdline switch in //chromiumos/platform2/login_manager/device_policy_service.cc on the CrOS side (i.e. starting to use the new --disable-site-isolation-for-policy cmdline flag instead of the --disable-site-isolation-trials old one)?  

I've tried to follow https://chromium.googlesource.com/chromiumos/docs/+/master/developer_guide.md, but I couldn't figure out how to get CrOS source code :-/

$ repo init -u https://chromium.googlesource.com/chromiumos/manifest.git --repo-url https://chromium.googlesource.com/external/repo.git
fatal: Cannot get https://chromium.googlesource.com/external/repo.git/clone.bundle
fatal: error no host given

CL: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1365815

Re #10, the problem looks similar to 

https://groups.google.com/a/chromium.org/forum/#!topic/chromium-os-dev/uP-OF11sjkc

That was due to a bad "repo" in depot_tools. I wonder which "repo" you are running.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/4724025950d3b42ff17fe4eb806d695c81643aa0

commit 4724025950d3b42ff17fe4eb806d695c81643aa0
Author: Xiyuan Xia <xiyuan@google.com>
Date: Fri Dec 07 06:06:36 2018

login: Update site isolation disabling switch

Use the new switch to disable site isolation from policy.
  disable-site-isolation-trials => disable-site-isolation-for-policy

BUG= chromium:900998 
TEST=DevicePolicyServiceTest.*

Change-Id: I6d409dca206a53f624baac97c990d6481190cbdd
Reviewed-on: https://chromium-review.googlesource.com/1365815
Commit-Ready: Xiyuan Xia <xiyuan@chromium.org>
Tested-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>

[modify] https://crrev.com/4724025950d3b42ff17fe4eb806d695c81643aa0/login_manager/device_policy_service.cc
[modify] https://crrev.com/4724025950d3b42ff17fe4eb806d695c81643aa0/login_manager/device_policy_service_test.cc

Just wanted to thank you for stepping up and helping with that Xiyuan! Kudos for being a great teammate!

Thank you xiyuan@!  I'll mark this bug as fixed.  Note that as a follow-up we have issue 910273 to try to disallow disabling Site Isolation via desktop Enterprise Policies.

Verified on Chrome OS, "Stability and security will suffer" warning is shown when chrome://flags/#site-isolation-trial-opt-out is disabled (see attached screenshot).

Google Chrome	73.0.3637.0 (Official Build) unknown (64-bit)
Platform	11386.0.0 (Official Build) dev-channel nautilus test

Deleted: Screenshot 2018-12-11 at 2.51.38 PM.png 219 KB

	Screenshot 2018-12-11 at 2.51.38 PM.png 219 KB View Download