Replace kPreferHtmlOverPlugins feature checks with PluginUtils::ShouldPreferHtmlOverPlugi…

Replace kPreferHtmlOverPlugins feature checks with PluginUtils::ShouldPreferHtmlOverPlugins

This replaces occurences of base::FeatureList::IsEnabled(features::kPreferHtmlOverPlugins)
with PluginUtils::ShouldPreferHtmlOverPlugins which includes extra checks to
determine whether the feature should be enabled. PreferHtmlOverPlugins should
not be enabled if an enterprise setting is set because it currently doesn't
work correctly when the enterprise setting is set to ASK. The Profile has been
plumbed through to places where it's needed in order to make checking this
possible.

BUG=654072
Committed: https://crrev.com/b512db8bfb5266fb346540e24bca1416eb9260a7
Cr-Commit-Position: refs/heads/master@{#424583}

[raymes, 2016-10-10 04:25:37]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 04:25:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[raymes, 2016-10-10 04:36:20]

raymes@chromium.org changed reviewers:
+ groby@chromium.org

[raymes, 2016-10-10 04:37:54]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 04:38:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-10 04:54:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-10 04:54:16]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[raymes, 2016-10-10 05:00:22]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 05:00:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[raymes, 2016-10-10 05:09:45]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 05:09:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-10 05:27:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-10 05:27:28]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[raymes, 2016-10-10 05:31:31]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 05:31:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[groby-ooo-7-16, 2016-10-10 05:50:07]

LGTM (with tiny style nits)

https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
(right):

https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:88:
forProfile:(Profile*)profile;
Tiny nit: I'd call this profile: instead of forProfile:

(Since it's the 5th param - usually forXYZ only is the name of the first)

https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:100:
forProfile:(Profile*)profile {
nit: see above

https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
File
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h
(right):

https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h:37:

nit: kill the blank lines

https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h:109:
forProfile:(Profile*)profile
nit: forProfile

[groby-ooo-7-16, 2016-10-10 05:51:47]

On 2016/10/10 05:50:07, groby wrote:
> LGTM (with tiny style nits)
> 
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> File chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
> (right):
> 
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:88:
> forProfile:(Profile*)profile;
> Tiny nit: I'd call this profile: instead of forProfile:
> 
> (Since it's the 5th param - usually forXYZ only is the name of the first)
> 
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:100:
> forProfile:(Profile*)profile {
> nit: see above
> 
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> File
> chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h
> (right):
> 
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
>
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h:37:
> 
> nit: kill the blank lines
> 
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
>
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h:109:
> forProfile:(Profile*)profile
> nit: forProfile

Ah - and it looks like you'll need to fix up WebsiteSettingsBubbleUnittest

[raymes, 2016-10-10 05:53:05]

On 2016/10/10 05:51:47, groby wrote:
> On 2016/10/10 05:50:07, groby wrote:
> > LGTM (with tiny style nits)
> > 
> >
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> > File
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
> > (right):
> > 
> >
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> > chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:88:
> > forProfile:(Profile*)profile;
> > Tiny nit: I'd call this profile: instead of forProfile:
> > 
> > (Since it's the 5th param - usually forXYZ only is the name of the first)
> > 
> >
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> >
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:100:
> > forProfile:(Profile*)profile {
> > nit: see above
> > 
> >
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> > File
> >
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h
> > (right):
> > 
> >
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> >
>
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h:37:
> > 
> > nit: kill the blank lines
> > 
> >
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> >
>
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h:109:
> > forProfile:(Profile*)profile
> > nit: forProfile
> 
> Ah - and it looks like you'll need to fix up WebsiteSettingsBubbleUnittest

Yeah - I'm still debugging a few unittests/mac build failures - thought I'd
upload it for overall review. Thanks for the feedback :)

[commit-bot: I haz the power, 2016-10-10 05:53:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-10 05:54:00]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[groby-ooo-7-16, 2016-10-10 05:55:10]

Description was changed from

==========
Replace remaining kPreferHtmlOverPlugins feature checks with
PluginUtils::ShouldPreferHtmlOverPlugins

This replaces the remaining occurences of
base::FeatureList::IsEnabled(features::kPreferHtmlOverPlugins)
with PluginUtils::ShouldPreferHtmlOverPlugins which includes extra checks to
determine whether the feature should be enabled. The Profile has been plumbed
through to places where it's needed in order to make this possible.

BUG=654072
==========

to

==========
Replace remaining kPreferHtmlOverPlugins feature checks with
PluginUtils::ShouldPreferHtmlOverPlugins

This replaces the remaining occurences of
base::FeatureList::IsEnabled(features::kPreferHtmlOverPlugins)
with PluginUtils::ShouldPreferHtmlOverPlugins which includes extra checks to
determine whether the feature should be enabled. The Profile has been plumbed
through to places where it's needed in order to make this possible.

BUG=654072
==========

[groby-ooo-7-16, 2016-10-10 05:55:10]

groby@chromium.org changed reviewers:
+ bauerb@chromium.org, dbeam@chromium.org

[groby-ooo-7-16, 2016-10-10 05:56:16]

On 2016/10/10 05:53:05, raymes wrote:
> On 2016/10/10 05:51:47, groby wrote:
> > On 2016/10/10 05:50:07, groby wrote:
> > > LGTM (with tiny style nits)
> > > 
> > >
> >
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> > > File
> chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> > >
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:88:
> > > forProfile:(Profile*)profile;
> > > Tiny nit: I'd call this profile: instead of forProfile:
> > > 
> > > (Since it's the 5th param - usually forXYZ only is the name of the first)
> > > 
> > >
> >
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> > >
> chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:100:
> > > forProfile:(Profile*)profile {
> > > nit: see above
> > > 
> > >
> >
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> > > File
> > >
> chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> > >
> >
>
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h:37:
> > > 
> > > nit: kill the blank lines
> > > 
> > >
> >
>
https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
> > >
> >
>
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h:109:
> > > forProfile:(Profile*)profile
> > > nit: forProfile
> > 
> > Ah - and it looks like you'll need to fix up WebsiteSettingsBubbleUnittest
> 
> Yeah - I'm still debugging a few unittests/mac build failures - thought I'd
> upload it for overall review. Thanks for the feedback :)

Also, I just added bauerb@ for plugins and dbeam@ for webui/options.

Dan: It's not a functionality change for the old settings, just looping through
an additional needed parameter. RS would be appreciated :)

[raymes, 2016-10-10 06:02:01]

Description was changed from

==========
Replace remaining kPreferHtmlOverPlugins feature checks with
PluginUtils::ShouldPreferHtmlOverPlugins

This replaces the remaining occurences of
base::FeatureList::IsEnabled(features::kPreferHtmlOverPlugins)
with PluginUtils::ShouldPreferHtmlOverPlugins which includes extra checks to
determine whether the feature should be enabled. The Profile has been plumbed
through to places where it's needed in order to make this possible.

BUG=654072
==========

to

==========
Replace kPreferHtmlOverPlugins feature checks with
PluginUtils::ShouldPreferHtmlOverPlugins

This replaces occurences of
base::FeatureList::IsEnabled(features::kPreferHtmlOverPlugins)
with PluginUtils::ShouldPreferHtmlOverPlugins which includes extra checks to
determine whether the feature should be enabled. PreferHtmlOverPlugins should
not be enabled if the enterprise setting is set to ASK because it currently
doesn't work correctly. The Profile has been plumbed through to places where
it's needed in order to make checking this possible.

BUG=654072
==========

[raymes, 2016-10-10 06:02:18]

https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
(right):

https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:88:
forProfile:(Profile*)profile;
On 2016/10/10 05:50:07, groby wrote:
> Tiny nit: I'd call this profile: instead of forProfile:
> 
> (Since it's the 5th param - usually forXYZ only is the name of the first)

Done. Thanks I'm not familiar with objective C style.

https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:100:
forProfile:(Profile*)profile {
On 2016/10/10 05:50:07, groby wrote:
> nit: see above

Done.

https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
File
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h
(right):

https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h:37:

On 2016/10/10 05:50:07, groby wrote:
> nit: kill the blank lines

Done.

https://codereview.chromium.org/2408613002/diff/90001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h:109:
forProfile:(Profile*)profile
On 2016/10/10 05:50:07, groby wrote:
> nit: forProfile

Done.

[raymes, 2016-10-10 06:02:41]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 06:02:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[raymes, 2016-10-10 06:12:48]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 06:12:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[raymes, 2016-10-10 06:22:24]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 06:22:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-10 06:49:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-10 06:49:18]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[raymes, 2016-10-10 07:11:06]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 07:11:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-10 08:11:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-10 08:11:53]

Dry run: This issue passed the CQ dry run.

[Bernhard Bauer, 2016-10-10 14:03:25]

lgtm

https://codereview.chromium.org/2408613002/diff/190001/chrome/browser/plugins...
File chrome/browser/plugins/plugin_utils.cc (right):

https://codereview.chromium.org/2408613002/diff/190001/chrome/browser/plugins...
chrome/browser/plugins/plugin_utils.cc:123: // policy for the plugin default
setting. crbug.com/654072.
This doesn't quite match the description -- do we want to disallow PreferHtml if
_any_ default is set, or just if it's set to ASK?

[raymes, 2016-10-10 23:56:42]

https://codereview.chromium.org/2408613002/diff/190001/chrome/browser/plugins...
File chrome/browser/plugins/plugin_utils.cc (right):

https://codereview.chromium.org/2408613002/diff/190001/chrome/browser/plugins...
chrome/browser/plugins/plugin_utils.cc:123: // policy for the plugin default
setting. crbug.com/654072.
On 2016/10/10 14:03:25, Bernhard Bauer wrote:
> This doesn't quite match the description -- do we want to disallow PreferHtml
if
> _any_ default is set, or just if it's set to ASK?

Good point. I've updated it.

[raymes, 2016-10-10 23:56:50]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 23:57:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[raymes, 2016-10-10 23:58:19]

raymes@chromium.org changed reviewers:
+ tapted@chromium.org

[raymes, 2016-10-10 23:58:19]

+OWNERS

tapted for chrome/browser/ui/cocoa/website_settings/*

dbeam for: 
chrome/browser/ui/webui/options/content_settings_handler.cc
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

[raymes, 2016-10-11 00:20:11]

Description was changed from

==========
Replace kPreferHtmlOverPlugins feature checks with
PluginUtils::ShouldPreferHtmlOverPlugins

This replaces occurences of
base::FeatureList::IsEnabled(features::kPreferHtmlOverPlugins)
with PluginUtils::ShouldPreferHtmlOverPlugins which includes extra checks to
determine whether the feature should be enabled. PreferHtmlOverPlugins should
not be enabled if the enterprise setting is set to ASK because it currently
doesn't work correctly. The Profile has been plumbed through to places where
it's needed in order to make checking this possible.

BUG=654072
==========

to

==========
Replace kPreferHtmlOverPlugins feature checks with
PluginUtils::ShouldPreferHtmlOverPlugins

This replaces occurences of
base::FeatureList::IsEnabled(features::kPreferHtmlOverPlugins)
with PluginUtils::ShouldPreferHtmlOverPlugins which includes extra checks to
determine whether the feature should be enabled. PreferHtmlOverPlugins should
not be enabled if an enterprise setting is set because it currently doesn't
work correctly when the enterprise setting is set to ASK. The Profile has been
plumbed through to places where it's needed in order to make checking this
possible.

BUG=654072
==========

[tapted, 2016-10-11 00:24:50]

https://codereview.chromium.org/2408613002/diff/230001/chrome/browser/ui/coco...
File
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h
(right):

https://codereview.chromium.org/2408613002/diff/230001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h:36:
Profile* profile_;
It's usually nice to avoid unnecessary weak pointers. Can you use a helper
method like 

- (Profile*)profile {
  return profile::FromBrowserContext(webContents_->GetBrowserContext());
}

and just pass [self profile] when needed?

[raymes, 2016-10-11 00:52:34]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 00:53:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[raymes, 2016-10-11 00:54:30]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 00:54:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-11 01:22:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-11 01:22:53]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[raymes, 2016-10-11 02:04:38]

https://codereview.chromium.org/2408613002/diff/230001/chrome/browser/ui/coco...
File
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h
(right):

https://codereview.chromium.org/2408613002/diff/230001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h:36:
Profile* profile_;
On 2016/10/11 00:24:50, tapted wrote:
> It's usually nice to avoid unnecessary weak pointers. Can you use a helper
> method like 
> 
> - (Profile*)profile {
>   return profile::FromBrowserContext(webContents_->GetBrowserContext());
> }
> 
> and just pass [self profile] when needed?

Done.

[raymes, 2016-10-11 02:04:57]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 02:05:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2016-10-11 02:22:16]

chrome/browser/ui/cocoa/website_settings/* lgtm

https://codereview.chromium.org/2408613002/diff/290001/chrome/browser/ui/coco...
File
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h
(right):

https://codereview.chromium.org/2408613002/diff/290001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h:109:
- (Profile*)profile;
nit: I think this can be "private". Which you can achieve just be deleting this
line. (we usually also put the declaration into a private category to get better
compile errors, but WebsiteSettingsBubbleController has a ton of stuff that
doesn't do this currently.)

https://codereview.chromium.org/2408613002/diff/290001/chrome/browser/ui/coco...
File
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.mm
(right):

https://codereview.chromium.org/2408613002/diff/290001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.mm:182:
- (Profile*)profile {
nit: move after the initializer (line ~242)

[commit-bot: I haz the power, 2016-10-11 02:38:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-11 02:38:13]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[raymes, 2016-10-11 03:47:01]

Description was changed from

==========
Replace kPreferHtmlOverPlugins feature checks with
PluginUtils::ShouldPreferHtmlOverPlugins

This replaces occurences of
base::FeatureList::IsEnabled(features::kPreferHtmlOverPlugins)
with PluginUtils::ShouldPreferHtmlOverPlugins which includes extra checks to
determine whether the feature should be enabled. PreferHtmlOverPlugins should
not be enabled if an enterprise setting is set because it currently doesn't
work correctly when the enterprise setting is set to ASK. The Profile has been
plumbed through to places where it's needed in order to make checking this
possible.

BUG=654072
==========

to

==========
Replace kPreferHtmlOverPlugins feature checks with
PluginUtils::ShouldPreferHtmlOverPlugins

This replaces occurences of
base::FeatureList::IsEnabled(features::kPreferHtmlOverPlugins)
with PluginUtils::ShouldPreferHtmlOverPlugins which includes extra checks to
determine whether the feature should be enabled. PreferHtmlOverPlugins should
not be enabled if an enterprise setting is set because it currently doesn't
work correctly when the enterprise setting is set to ASK. The Profile has been
plumbed through to places where it's needed in order to make checking this
possible.

BUG=654072
==========

[raymes, 2016-10-11 03:50:02]

https://codereview.chromium.org/2408613002/diff/290001/chrome/browser/ui/coco...
File
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h
(right):

https://codereview.chromium.org/2408613002/diff/290001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h:109:
- (Profile*)profile;
On 2016/10/11 02:22:16, tapted wrote:
> nit: I think this can be "private". Which you can achieve just be deleting
this
> line. (we usually also put the declaration into a private category to get
better
> compile errors, but WebsiteSettingsBubbleController has a ton of stuff that
> doesn't do this currently.)

Done.

https://codereview.chromium.org/2408613002/diff/290001/chrome/browser/ui/coco...
File
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.mm
(right):

https://codereview.chromium.org/2408613002/diff/290001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.mm:182:
- (Profile*)profile {
On 2016/10/11 02:22:16, tapted wrote:
> nit: move after the initializer (line ~242)

Done.

[raymes, 2016-10-11 05:14:21]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 05:14:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-11 05:55:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-11 05:55:25]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2016-10-11 22:58:34]

lgtm

[raymes, 2016-10-11 22:59:24]

The CQ bit was checked by raymes@chromium.org

[raymes, 2016-10-11 22:59:25]

The patchset sent to the CQ was uploaded after l-g-t-m from groby@chromium.org,
bauerb@chromium.org, tapted@chromium.org
Link to the patchset: https://codereview.chromium.org/2408613002/#ps310001
(title: "Replace remaining kPreferHtmlOverPlugins feature checks with
PluginUtils::ShouldPreferHtmlOverPlugi…")

[commit-bot: I haz the power, 2016-10-11 23:00:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-11 23:07:47]

Description was changed from

==========
Replace kPreferHtmlOverPlugins feature checks with
PluginUtils::ShouldPreferHtmlOverPlugins

This replaces occurences of
base::FeatureList::IsEnabled(features::kPreferHtmlOverPlugins)
with PluginUtils::ShouldPreferHtmlOverPlugins which includes extra checks to
determine whether the feature should be enabled. PreferHtmlOverPlugins should
not be enabled if an enterprise setting is set because it currently doesn't
work correctly when the enterprise setting is set to ASK. The Profile has been
plumbed through to places where it's needed in order to make checking this
possible.

BUG=654072
==========

to

==========
Replace kPreferHtmlOverPlugins feature checks with
PluginUtils::ShouldPreferHtmlOverPlugins

This replaces occurences of
base::FeatureList::IsEnabled(features::kPreferHtmlOverPlugins)
with PluginUtils::ShouldPreferHtmlOverPlugins which includes extra checks to
determine whether the feature should be enabled. PreferHtmlOverPlugins should
not be enabled if an enterprise setting is set because it currently doesn't
work correctly when the enterprise setting is set to ASK. The Profile has been
plumbed through to places where it's needed in order to make checking this
possible.

BUG=654072
==========

[commit-bot: I haz the power, 2016-10-11 23:07:49]

Committed patchset #17 (id:310001)

[commit-bot: I haz the power, 2016-10-11 23:11:29]

Description was changed from

==========
Replace kPreferHtmlOverPlugins feature checks with
PluginUtils::ShouldPreferHtmlOverPlugins

This replaces occurences of
base::FeatureList::IsEnabled(features::kPreferHtmlOverPlugins)
with PluginUtils::ShouldPreferHtmlOverPlugins which includes extra checks to
determine whether the feature should be enabled. PreferHtmlOverPlugins should
not be enabled if an enterprise setting is set because it currently doesn't
work correctly when the enterprise setting is set to ASK. The Profile has been
plumbed through to places where it's needed in order to make checking this
possible.

BUG=654072
==========

to

==========
Replace kPreferHtmlOverPlugins feature checks with
PluginUtils::ShouldPreferHtmlOverPlugins

This replaces occurences of
base::FeatureList::IsEnabled(features::kPreferHtmlOverPlugins)
with PluginUtils::ShouldPreferHtmlOverPlugins which includes extra checks to
determine whether the feature should be enabled. PreferHtmlOverPlugins should
not be enabled if an enterprise setting is set because it currently doesn't
work correctly when the enterprise setting is set to ASK. The Profile has been
plumbed through to places where it's needed in order to make checking this
possible.

BUG=654072

Committed: https://crrev.com/b512db8bfb5266fb346540e24bca1416eb9260a7
Cr-Commit-Position: refs/heads/master@{#424583}
==========

[commit-bot: I haz the power, 2016-10-11 23:11:31]

Patchset 17 (id:??) landed as
https://crrev.com/b512db8bfb5266fb346540e24bca1416eb9260a7
Cr-Commit-Position: refs/heads/master@{#424583}