Add --{enable,disable}-blink-features flags which set RuntimeEnabledFeatures.

Add --{enable,disable}-blink-features flags which set RuntimeEnabledFeatures.

These provide a straightforward way to toggle RuntimeEnabledFeatures
without recompiling or plumbing a separate flag through for each.

Features requiring more complicated logic will still have to plumb a
separate flag through.

The Blink-side CL is:
  https://codereview.chromium.org/890723004/

BUG=454504
Committed: https://crrev.com/8a522834e604f3683f3d9741181dfeb1a03d4998
Cr-Commit-Position: refs/heads/master@{#314821}

[jbroman, 2015-02-02 21:44:08]

jbroman@chromium.org changed reviewers:
+ jochen@chromium.org

[jbroman, 2015-02-02 21:44:08]

Blink-side CL has LGTM from chrishtr.

Does this look reasonable to you?

[jochen (gone - plz use gerrit), 2015-02-04 12:39:21]

lgtm

https://codereview.chromium.org/898483004/diff/1/content/child/runtime_featur...
File content/child/runtime_features.cc (right):

https://codereview.chromium.org/898483004/diff/1/content/child/runtime_featur...
content/child/runtime_features.cc:200: for (const std::string& feature :
enabled_features)
please add {}

https://codereview.chromium.org/898483004/diff/1/content/child/runtime_featur...
content/child/runtime_features.cc:209: for (const std::string& feature :
disabled_features)
same here

[jbroman, 2015-02-04 15:39:07]

New patchsets have been uploaded after l-g-t-m from jochen@chromium.org

[jbroman, 2015-02-04 15:48:28]

Patchset #3 (id:40001) has been deleted

[jbroman, 2015-02-04 15:49:21]

https://codereview.chromium.org/898483004/diff/1/content/child/runtime_featur...
File content/child/runtime_features.cc (right):

https://codereview.chromium.org/898483004/diff/1/content/child/runtime_featur...
content/child/runtime_features.cc:200: for (const std::string& feature :
enabled_features)
On 2015/02/04 12:39:20, jochen (slow) wrote:
> please add {}

Done.

https://codereview.chromium.org/898483004/diff/1/content/child/runtime_featur...
content/child/runtime_features.cc:209: for (const std::string& feature :
disabled_features)
On 2015/02/04 12:39:20, jochen (slow) wrote:
> same here

Done.

[jbroman, 2015-02-05 15:12:56]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2015-02-05 15:13:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/898483004/60001

[commit-bot: I haz the power, 2015-02-05 16:14:28]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-02-05 16:15:37]

Patchset 3 (id:??) landed as
https://crrev.com/8a522834e604f3683f3d9741181dfeb1a03d4998
Cr-Commit-Position: refs/heads/master@{#314821}

[rjvbertin, 2015-09-28 07:49:58]

Is this why I am getting an unasked-for "You're using an unsupported 
feature, --disable-blink-features" whenever I launch GChrome nowadays?

And more importantly: is that error the reason why session restore NO 
LONGER WORKS after an improper shutdown (chrome, login session or even host 
crash)?

On Monday, 2 February 2015 22:44:10 UTC+1, jbr...@chromium.org wrote:
>
> Reviewers: jochen (slow), 
>
> Message: 
> Blink-side CL has LGTM from chrishtr. 
>
> Does this look reasonable to you? 
>
> Description: 
> Add --{enable,disable}-blink-features flags which set   
> RuntimeEnabledFeatures. 
>
> These provide a straightforward way to toggle RuntimeEnabledFeatures 
> without recompiling or plumbing a separate flag through for each. 
>
> Features requiring more complicated logic will still have to plumb a 
> separate flag through. 
>
> The Blink-side CL is: 
>    https://codereview.chromium.org/890723004/ 
>
> BUG=454504 
>
> Please review this at https://codereview.chromium.org/898483004/ 
>
> Base URL: https://chromium.googlesource.com/chromium/src.git@master 
>
> Affected files (+42, -0 lines): 
>    M chrome/browser/chromeos/login/chrome_restart_request.cc 
>    M content/browser/renderer_host/render_process_host_impl.cc 
>    M content/child/runtime_features.cc 
>    M content/public/common/content_switches.h 
>    M content/public/common/content_switches.cc 
>
>
> Index: chrome/browser/chromeos/login/chrome_restart_request.cc 
> diff --git a/chrome/browser/chromeos/login/chrome_restart_request.cc   
> b/chrome/browser/chromeos/login/chrome_restart_request.cc 
> index   
>
c7fe45b7d4b05eae6cb1e38ace803af074d2f1a0..894bf7600fce4c715fbeb29758160c5f7431c547

>   
> 100644 
> --- a/chrome/browser/chromeos/login/chrome_restart_request.cc 
> +++ b/chrome/browser/chromeos/login/chrome_restart_request.cc 
> @@ -72,6 +72,7 @@ std::string DeriveCommandLine(const GURL& start_url, 
>       ::switches::kDisableAccelerated2dCanvas, 
>       ::switches::kDisableAcceleratedJpegDecoding, 
>       ::switches::kDisableAcceleratedVideoDecode, 
> +    ::switches::kDisableBlinkFeatures, 
>       ::switches::kDisableCastStreamingHWEncoding, 
>       ::switches::kDisableDelegatedRenderer, 
>       ::switches::kDisableDistanceFieldText, 
> @@ -94,6 +95,7 @@ std::string DeriveCommandLine(const GURL& start_url, 
>       ::switches::kDisableTouchDragDrop, 
>       ::switches::kDisableTouchEditing, 
>       ::switches::kEnableBeginFrameScheduling, 
> +    ::switches::kEnableBlinkFeatures, 
>       ::switches::kEnablePreferCompositingToLCDText, 
>       ::switches::kEnableDelegatedRenderer, 
>       ::switches::kDisableDisplayList2dCanvas, 
> Index: content/browser/renderer_host/render_process_host_impl.cc 
> diff --git a/content/browser/renderer_host/render_process_host_impl.cc   
> b/content/browser/renderer_host/render_process_host_impl.cc 
> index   
>
2fc424be26caadd219977a1bff02a3103eef9144..172e52c394d9890235e725254dd9bfc37a67ff7f

>   
> 100644 
> --- a/content/browser/renderer_host/render_process_host_impl.cc 
> +++ b/content/browser/renderer_host/render_process_host_impl.cc 
> @@ -1194,6 +1194,7 @@ void   
> RenderProcessHostImpl::PropagateBrowserCommandLineToRenderer( 
>       switches::kDisableAcceleratedJpegDecoding, 
>       switches::kDisableAcceleratedVideoDecode, 
>       switches::kDisableBlinkScheduler, 
> +    switches::kDisableBlinkFeatures, 
>       switches::kDisableBreakpad, 
>       switches::kDisablePreferCompositingToLCDText, 
>       switches::kDisableDatabases, 
> @@ -1225,6 +1226,7 @@ void   
> RenderProcessHostImpl::PropagateBrowserCommandLineToRenderer( 
>       switches::kDomAutomationController, 
>       switches::kEnableBeginFrameScheduling, 
>       switches::kEnableBleedingEdgeRenderingFastPaths, 
> +    switches::kEnableBlinkFeatures, 
>       switches::kEnableBrowserSideNavigation, 
>       switches::kEnablePreferCompositingToLCDText, 
>       switches::kEnableCredentialManagerAPI, 
> Index: content/child/runtime_features.cc 
> diff --git a/content/child/runtime_features.cc   
> b/content/child/runtime_features.cc 
> index   
>
79b0f244b47fada2de35d0bf87c4ceb00b426adc..73aa0d1b7ce6b44c9567e0bf4d8fd6ba89d736f4

>   
> 100644 
> --- a/content/child/runtime_features.cc 
> +++ b/content/child/runtime_features.cc 
> @@ -4,8 +4,11 @@ 
>
>   #include "content/child/runtime_features.h" 
>
> +#include <vector> 
> + 
>   #include "base/command_line.h" 
>   #include "base/metrics/field_trial.h" 
> +#include "base/strings/string_split.h" 
>   #include "content/common/content_switches_internal.h" 
>   #include "content/public/common/content_switches.h" 
>   #include "third_party/WebKit/public/web/WebRuntimeFeatures.h" 
> @@ -186,6 +189,27 @@ void SetRuntimeFeaturesDefaultsAndUpdateFromArgs( 
>       WebRuntimeFeatures::enableV8IdleTasks(false); 
>     else 
>       WebRuntimeFeatures::enableV8IdleTasks(true); 
> + 
> +  // Enable explicitly enabled features, and then disable explicitly   
> disabled 
> +  // ones. 
> +  if (command_line.HasSwitch(switches::kEnableBlinkFeatures)) { 
> +    std::vector<std::string> enabled_features; 
> +    base::SplitString( 
> +         
> command_line.GetSwitchValueASCII(switches::kEnableBlinkFeatures), ',', 
> +        &enabled_features); 
> +    for (const std::string& feature : enabled_features) 
> +      WebRuntimeFeatures::enableFeatureFromString( 
> +          blink::WebString::fromLatin1(feature), true); 
> +  } 
> +  if (command_line.HasSwitch(switches::kDisableBlinkFeatures)) { 
> +    std::vector<std::string> disabled_features; 
> +    base::SplitString( 
> +         
> command_line.GetSwitchValueASCII(switches::kDisableBlinkFeatures), ',', 
> +        &disabled_features); 
> +    for (const std::string& feature : disabled_features) 
> +      WebRuntimeFeatures::enableFeatureFromString( 
> +          blink::WebString::fromLatin1(feature), false); 
> +  } 
>   } 
>
>   }  // namespace content 
> Index: content/public/common/content_switches.cc 
> diff --git a/content/public/common/content_switches.cc   
> b/content/public/common/content_switches.cc 
> index   
>
5ae4367758474a7ee2b815b4a9cafd6a3e8d3d87..b59e5cd0f8d5ba822d2f0c1174145b84dfae1013

>   
> 100644 
> --- a/content/public/common/content_switches.cc 
> +++ b/content/public/common/content_switches.cc 
> @@ -84,6 +84,12 @@ const char kDisableAcceleratedVideoDecode[] = 
>   // users with many windows/tabs and lots of memory. 
>   const char kDisableBackingStoreLimit[]       
> = "disable-backing-store-limit"; 
>
> +// Disable one or more Blink runtime-enabled features. 
> +// Use names from RuntimeEnabledFeatures.in, separated by commas. 
> +// Applied after kEnableBlinkFeatures, and after other flags that change 
>   
> these 
> +// features. 
> +const char kDisableBlinkFeatures[]          = "disable-blink-features"; 
> + 
>   // Disable the Blink Scheduler. Ensures there's no reordering of blink   
> tasks. 
>   // This switch is intended only for performance tests. 
>   const char kDisableBlinkScheduler[]         = "disable-blink-scheduler"; 
> @@ -285,6 +291,12 @@ const char kEnableBeginFrameScheduling[]     
> = "enable-begin-frame-scheduling"; 
>   const char kEnablePreferCompositingToLCDText[] = 
>       "enable-prefer-compositing-to-lcd-text"; 
>
> +// Disable one or more Blink runtime-enabled features. 
> +// Use names from RuntimeEnabledFeatures.in, separated by commas. 
> +// Applied before kDisableBlinkFeatures, and after other flags that 
> change   
> these 
> +// features. 
> +const char kEnableBlinkFeatures[]           = "enable-blink-features"; 
> + 
>   // PlzNavigate: Use the experimental browser-side navigation path. 
>   const char kEnableBrowserSideNavigation[]     
> = "enable-browser-side-navigation"; 
>
> Index: content/public/common/content_switches.h 
> diff --git a/content/public/common/content_switches.h   
> b/content/public/common/content_switches.h 
> index   
>
916501ea4b1c180fc59bd8a97d30955cafd2f66f..f727047b99833bcae465a4fb309e5592362b38f8

>   
> 100644 
> --- a/content/public/common/content_switches.h 
> +++ b/content/public/common/content_switches.h 
> @@ -31,6 +31,7 @@ CONTENT_EXPORT extern const char kDefaultTileWidth[]; 
>   CONTENT_EXPORT extern const char kDefaultTileHeight[]; 
>   CONTENT_EXPORT extern const char kDisable2dCanvasAntialiasing[]; 
>   CONTENT_EXPORT extern const char kDisable3DAPIs[]; 
> +CONTENT_EXPORT extern const char kDisableBlinkFeatures[]; 
>   CONTENT_EXPORT extern const char kDisableAccelerated2dCanvas[]; 
>   CONTENT_EXPORT extern const char kDisableAcceleratedJpegDecoding[]; 
>   CONTENT_EXPORT extern const char kDisableAcceleratedVideoDecode[]; 
> @@ -90,6 +91,7 @@ CONTENT_EXPORT extern const char   
> kEnableBleedingEdgeRenderingFastPaths[]; 
>   CONTENT_EXPORT extern const char kEnableCredentialManagerAPI[]; 
>   CONTENT_EXPORT extern const char kEnableBeginFrameScheduling[]; 
>   CONTENT_EXPORT extern const char kEnablePreferCompositingToLCDText[]; 
> +CONTENT_EXPORT extern const char kEnableBlinkFeatures[]; 
>   CONTENT_EXPORT extern const char kEnableBrowserSideNavigation[]; 
>   CONTENT_EXPORT extern const char kEnableDeferredImageDecoding[]; 
>   CONTENT_EXPORT extern const char kEnableDelayAgnosticAec[]; 
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jbroman, 2015-09-28 14:10:40]

On 2015/09/28 at 07:49:58, rjvbertin wrote:
> Is this why I am getting an unasked-for "You're using an unsupported 
> feature, --disable-blink-features" whenever I launch GChrome nowadays?

Please do not use the --disable-blink-features flag except for developing
Chromium.
As the message says, doing so is unsupported.

> And more importantly: is that error the reason why session restore NO 
> LONGER WORKS after an improper shutdown (chrome, login session or even host 
> crash)?

Probably not. If the issue continues to occur with a supported set of flags,
file a bug at crbug.com/new.

[rjvbertin, 2015-09-28 15:37:26]

On 2015/09/28 14:10:40, jbroman wrote:

> Please do not use the --disable-blink-features flag except for developing
> Chromium.
> As the message says, doing so is unsupported.

The thing is that *I* am not using the option. I don't see it in chrome://flags,
and cannot find any occurrence of the string in my settings files and
startup-scripts (which are stock; I use Google Chrome's official packages for
Debuntu, stable channel).

> > And more importantly: is that error the reason why session restore NO 
> > LONGER WORKS after an improper shutdown (chrome, login session or even host 
> > crash)?
> 
> Probably not. If the issue continues to occur with a supported set of flags,
> file a bug at crbug.com/new.

Thing is that I can't seem to remove the disable-blink setting, and thus cannot
test without it... (and as long as that's the case I cannot assume the issues
are NOT related)

[jbroman, 2015-09-28 17:48:01]

On 2015/09/28 at 15:37:26, rjvbertin wrote:
> On 2015/09/28 14:10:40, jbroman wrote:
> 
> > Please do not use the --disable-blink-features flag except for developing
> > Chromium.
> > As the message says, doing so is unsupported.
> 
> The thing is that *I* am not using the option. I don't see it in
chrome://flags, and cannot find any occurrence of the string in my settings
files and startup-scripts (which are stock; I use Google Chrome's official
packages for Debuntu, stable channel).

Could you please take a look at chrome://version and tell me where
--disable-blink-features appears. If it's between --flag-switches-begin and
--flag-switches-end, then I think you've discovered an issue we should fix. If
so, could you please try going to chrome://flags/#scroll-top-left-interop and
switching it to Default, then restarting Chrome? I have a hunch about what's
going on here.

> > > And more importantly: is that error the reason why session restore NO 
> > > LONGER WORKS after an improper shutdown (chrome, login session or even
host 
> > > crash)?
> > 
> > Probably not. If the issue continues to occur with a supported set of flags,
> > file a bug at crbug.com/new.
> 
> Thing is that I can't seem to remove the disable-blink setting, and thus
cannot test without it... (and as long as that's the case I cannot assume the
issues are NOT related)

[rjvbertin, 2015-09-28 21:31:31]

On 2015/09/28 17:48:01, jbroman wrote:
> Could you please take a look at chrome://version and tell me where
> --disable-blink-features appears. If it's between --flag-switches-begin and
> --flag-switches-end, then I think you've discovered an issue we should fix. If

Yep. It's the last or among the last options before --flag-switches-end .

> so, could you please try going to chrome://flags/#scroll-top-left-interop and
> switching it to Default, then restarting Chrome? I have a hunch about what's
> going on here.

I did that (switching back from "Disabled" which I had selected not long ago in
an attempt to get back normal scrolling behaviour in Google search results).
That seems to have fixed (removed) the warning message during a normal startup.
I haven't yet tested to crash Chrome (or my computer) deliberately.

R.

[jbroman, 2015-09-28 21:50:25]

On 2015/09/28 at 21:31:31, rjvbertin wrote:
> On 2015/09/28 17:48:01, jbroman wrote:
> > Could you please take a look at chrome://version and tell me where
> > --disable-blink-features appears. If it's between --flag-switches-begin and
> > --flag-switches-end, then I think you've discovered an issue we should fix.
If
> 
> Yep. It's the last or among the last options before --flag-switches-end .
> 
> > so, could you please try going to chrome://flags/#scroll-top-left-interop
and
> > switching it to Default, then restarting Chrome? I have a hunch about what's
> > going on here.
> 
> I did that (switching back from "Disabled" which I had selected not long ago
in an attempt to get back normal scrolling behaviour in Google search results).
That seems to have fixed (removed) the warning message during a normal startup.

I've filed https://code.google.com/p/chromium/issues/detail?id=536913 to resolve
this showing an infobar.

If this feature is breaking Google search, please do file a bug.

> I haven't yet tested to crash Chrome (or my computer) deliberately.
> 
> R.

And if you get crashes, please file a bug. (But please note that we generally
don't expect users to fiddle with chrome://flags, so some combinations of flags
from there may simply be broken.)

[rjvbertin, 2015-09-29 13:26:15]

> If this feature is breaking Google search, please do file a bug.

Nope. I was looking for a way to get rid of the annoying blue cursor in Google
search results and get back "normal" scrolling. Turns out that cannot be done on
the browser level (which is surprising because Chrome is the only browser I
tried that supports that cursor).

> And if you get crashes, please file a bug. (But please note that we generally

I think the rare Chrome crashes I see are mostly related to out-of-memory
conditions or Flash issues. I tend to push my poor old netbook too hard, despite
its 8Gb of RAM (and beginning with the fact I'm running off a ZFS root).
But if I run into a reproduceable bug, sure, I'll report it.