New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 879633 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 31
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-09-04
OS: Linux , Windows , Chrome , Mac , Fuchsia
Pri: 2
Type: Bug



Sign in to add a comment

Reduce confusion for chrome://flags/#enable-site-per-process

Project Member Reported by creis@chromium.org, Aug 31

Issue description

Chrome Version: 70.0.3538.0
OS: Win/Mac/Linux/ChromeOS

chrome://flags/#enable-site-per-process was added long ago as a way to enable strict Site Isolation when it was still an experimental feature, with the default SINGLE_VALUE_TYPE options of "Enabled" and "Disabled."  It controls whether the --site-per-process flag is passed.

Now that the feature is enabled by default on Windows, Mac, Linux, and ChromeOS, the default "Disabled" value is confusing and causing users to think the feature is not enabled, even though it's enabled via other means (e.g., Finch in M67+, and by default in the code in M70).  chrome://process-internals in M68+ is a more accurate way to check whether the mode is enabled.

The flag is still useful on Android (and in other embedders who haven't enabled the feature by default), and we're hesitant to change the value labels to "Enabled" and "Default" because that would reset the flag for any users who currently have it enabled (e.g., on Android).

For now, let's clarify the meaning in the description, and we can look into removing the flag from desktop platforms (where it's basically a no-op) after checking on the impact to other embedders.
 
Note: We can follow up on potentially removing the flag in issue 874998.
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 31

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

commit f44fe2984e771e90d777dadac4f6431cbae11b81
Author: Charlie Reis <creis@chromium.org>
Date: Fri Aug 31 18:38:47 2018

Update flag description for chrome://flags/#enable-site-per-process.

The "Disabled" value is legacy and confusing, since it actually means
"Default" (which is currently "Enabled" on desktop platforms).
Unfortunately, updating the label to "Default" would reset users'
preferences, which is undesirable (especially on Android).

Instead, update the description to clarify the meaning of the settings
and to point to chrome://process-internals for checking status.

Bug:  879633 
Change-Id: I672f5ebe4ef75482493f0668b64d6d20144d15ed
Reviewed-on: https://chromium-review.googlesource.com/1198011
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588098}
[modify] https://crrev.com/f44fe2984e771e90d777dadac4f6431cbae11b81/chrome/browser/flag_descriptions.cc

Cc: abdulsyed@chromium.org
Labels: Target-70
NextAction: 2018-09-04
Status: Fixed (was: Started)
The description update in r588098 should help.  Once we verify that it looks correct in the next Canary, this will probably be worth merging to M70 (where Site Isolation is enabled at 100% and not just 99%).  Setting NextAction date to request merge.
Cc: jmukthavaram@chromium.org
Labels: Needs-Feedback
Tested this issue on Windows 10, Mac 10.13.6 & debian Rodate using chrome-70.0.3538.0,70.0.3539.0 & 71.0.3541.0 as per c#0.

Steps:
1. Launched chrome
2. Go to chrome://flags & search for 'site-per-process' 
3. Observed #enable-site-per-process is in 'Disabled' mode & '#site-isolation-trial-opt-out' is in 'default' mode.

creis@, Please find the attached screencast for reference & confirm on the fix.
Thanks..!





879633-on Win-70.0.3538.0-Reported.mp4
925 KB View Download
879633-Win-71.0.3539.0-Fixed version .mp4
474 KB View Download
The NextAction date has arrived: 2018-09-04
Labels: -Needs-Feedback
Comment 4: Yes, that's the expected behavior.  Thanks!

Also, I found a typo in the updated description (a missing space in an important place), which I'm fixing in https://chromium-review.googlesource.com/c/chromium/src/+/1204732.  I'll request merge once that's in.
Labels: Merge-Request-70
The typo fix landed in r588601.  Requesting merge for the combo of that and r588098 to M70, to help avoid confusion about the flag now that Site Isolation is on by default in M70 (even when that page says "disabled").
Labels: Needs-Feedback
Tested this issue on windows 10,Mac 10.13.6 & debian using chrome canary-71.0.3543.0 as per C#6 & C#7 understood that enable-site-per-=process flag is on by default in both M70 and M71 even it shown as 'disabled' in chrome://flags.Find the attached screencast for reference.

Please let us know the typo fixed in new Canary so that we will test the same & update the thread.
Thanks..!



879633-win-71.0.3543.0.mp4
1.0 MB View Download
Labels: -Needs-Feedback
Comment 8: Thanks for checking.  The typo was near the end:

#site-isolation-trial-opt-outfor

was updated to:

#site-isolation-trial-opt-out for

That's corrected in 71.0.3543.0, so I think we should be ready to merge to M70.
Labels: -Merge-Request-70 Merge-Approved-70
Approved - branch:3538
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 5

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

commit 30e3903274e9d3fa18561e1caf087ea320edd7b7
Author: Charlie Reis <creis@chromium.org>
Date: Tue Sep 04 18:58:18 2018

Fix typo in enable-site-per-process flag description.

BUG= 879633 

Change-Id: Ia8d706326522a26505be7848b78fae396de99755
Reviewed-on: https://chromium-review.googlesource.com/1204732
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588601}
[modify] https://crrev.com/30e3903274e9d3fa18561e1caf087ea320edd7b7/chrome/browser/flag_descriptions.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Sep 5

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5ba1486f0091e8de36fa0bf6fc90868bf9525a9f

commit 5ba1486f0091e8de36fa0bf6fc90868bf9525a9f
Author: Charlie Reis <creis@chromium.org>
Date: Wed Sep 05 23:40:15 2018

Merge to M70: Update flag description for chrome://flags/#enable-site-per-process.

The "Disabled" value is legacy and confusing, since it actually means
"Default" (which is currently "Enabled" on desktop platforms).
Unfortunately, updating the label to "Default" would reset users'
preferences, which is undesirable (especially on Android).

Instead, update the description to clarify the meaning of the settings
and to point to chrome://process-internals for checking status.

Bug:  879633 
Change-Id: I672f5ebe4ef75482493f0668b64d6d20144d15ed
Reviewed-on: https://chromium-review.googlesource.com/1198011
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588098}

Also merges r588601:
Fix typo in enable-site-per-process flag description.

Change-Id: Ia8d706326522a26505be7848b78fae396de99755
Reviewed-on: https://chromium-review.googlesource.com/1204732
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#588601}
Reviewed-on: https://chromium-review.googlesource.com/1208950
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#74}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/5ba1486f0091e8de36fa0bf6fc90868bf9525a9f/chrome/browser/flag_descriptions.cc

Labels: Needs-Feedback
Tested this issue on windows 10,Mac 10.13.6 & debian using chrome canary-71.0.3544.0 & #70.0.3538.9 as per C#11 & C#12.As per C#9, typo issue is not seen.
Please find the attached screencast for reference & confirm both M70 & M71 are working as intended as per the fix in C#11 & 12.
Thanks..!
879633-Win-70.0.3538.9.mp4
408 KB View Download
879633-WIN-71.0.3544.0.mp4
350 KB View Download
Labels: -Needs-Feedback
Comment 13: Looks like my merge didn't make 70.0.3538.9.  It's in 70.0.3538.10, though, so we'll see it in next week's build:
https://chromium.googlesource.com/chromium/src/+log/2f9354c40d473ed61ab5f9613df98ad2d6a7b0da
Labels: TE-Verified-M70 TE-Verified-70.0.3538.16
Thanks @Creis,
Tested this issue on windows 10,Mac 10.13.6 & debian using chrome reported version-70.0.3538.0 and latest dev-70.0.3538.16 as per C#6,9 & 14.

Steps:
-----
1. Launch chrome
2. Open chrome://flags & search for 'site-per-process'
3. Observe 2 flags

Reported version:
--------------
1. Flag "Strict site isolation"  content in chrome://flags is less & its state is in 'Disabled'.No sentence with 'site-isolation-trial-opt-out' in the content.
2. Flag "Site isolation trial opt-out" is in 'Default' mode in chrome://flags

Fixed version:
------------
1. Flag "Strict site isolation" in chrome://flags describes more about the flag and observed space before 'for' and after'#site-isolation-trial-opt-out' in the content & its state is in 'Disabled'.  .
2. Flag "Site isolation trial opt-out" is in 'Default' mode in chrome://flags

Seems issue got fixed now in #70.0.3538.16 build, hence adding TE Verified labels.Please find the attached screenshots for reference.


879633-Reported version.PNG
96.0 KB View Download
879633-WAI on Win-70.0.3538.16.PNG
108 KB View Download

Sign in to add a comment