Deleted CONTENT_SETTINGS_TYPE_FULLSCREEN and MOUSELOCK.

Deleted CONTENT_SETTINGS_TYPE_FULLSCREEN and MOUSELOCK.

Fullscreen and mouselock content settings have not been used for some
time. The pref data was recently deleted and now all code associated
with these settings is vestigial.

BUG=591896, 610900
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/273b9672cfbf83ffed8f52a91cb59a6e78a64577
Cr-Commit-Position: refs/heads/master@{#431465}

[Matt Giuca, 2016-10-17 06:28:09]

Description was changed from

==========
Deleted CONTENT_SETTINGS_TYPE_FULLSCREEN and MOUSELOCK.

Fullscreen and mouselock have not had content settings for some time;
all code associated with these settings is vestigial.

NOTE: As a side effect, this will automatically delete extensions data
on startup associated with these content settings (i.e., the per-site
settings that have been set by extensions). The data set directly by the
user remains in preferences files, and will be deleted in a follow-up.

BUG=591896,610900
==========

to

==========
Deleted CONTENT_SETTINGS_TYPE_FULLSCREEN and MOUSELOCK.

Fullscreen and mouselock have not had content settings for some time;
all code associated with these settings is vestigial.

NOTE: As a side effect, this will automatically delete extensions data
on startup associated with these content settings (i.e., the per-site
settings that have been set by extensions). The data set directly by the
user remains in preferences files, and will be deleted in a follow-up.

BUG=591896,610900
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Matt Giuca, 2016-10-21 05:35:27]

Description was changed from

==========
Deleted CONTENT_SETTINGS_TYPE_FULLSCREEN and MOUSELOCK.

Fullscreen and mouselock have not had content settings for some time;
all code associated with these settings is vestigial.

NOTE: As a side effect, this will automatically delete extensions data
on startup associated with these content settings (i.e., the per-site
settings that have been set by extensions). The data set directly by the
user remains in preferences files, and will be deleted in a follow-up.

BUG=591896,610900
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Deleted CONTENT_SETTINGS_TYPE_FULLSCREEN and MOUSELOCK.

Fullscreen and mouselock have not had content settings for some time;
all code associated with these settings is vestigial.

BUG=591896,610900
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Matt Giuca, 2016-10-21 05:36:23]

Description was changed from

==========
Deleted CONTENT_SETTINGS_TYPE_FULLSCREEN and MOUSELOCK.

Fullscreen and mouselock have not had content settings for some time;
all code associated with these settings is vestigial.

BUG=591896,610900
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Deleted CONTENT_SETTINGS_TYPE_FULLSCREEN and MOUSELOCK.

Fullscreen and mouselock content settings have not been used for some
time. The pref data was recently deleted and now all code associated
with these settings is vestigial.

BUG=591896,610900
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Matt Giuca, 2016-10-28 05:00:48]

Patchset #5 (id:80001) has been deleted

[Matt Giuca, 2016-11-02 03:46:28]

Patchset #8 (id:160001) has been deleted

[Matt Giuca, 2016-11-03 07:27:21]

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

[raymes, 2016-11-06 23:50:12]

https://codereview.chromium.org/2419413002/diff/180001/components/content_set...
File components/content_settings/core/common/content_settings.cc (left):

https://codereview.chromium.org/2419413002/diff/180001/components/content_set...
components/content_settings/core/common/content_settings.cc:35:
CONTENT_SETTINGS_TYPE_MOUSELOCK,
Please replace these with CONTENT_SETTINGS_TYPE_DEFAULT so that the histogram
order stays correct (see e.g. MEDIASTREAM below)

[Matt Giuca, 2016-11-07 00:19:24]

Patchset #9 (id:200001) has been deleted

[Matt Giuca, 2016-11-07 00:38:48]

https://codereview.chromium.org/2419413002/diff/180001/components/content_set...
File components/content_settings/core/common/content_settings.cc (left):

https://codereview.chromium.org/2419413002/diff/180001/components/content_set...
components/content_settings/core/common/content_settings.cc:35:
CONTENT_SETTINGS_TYPE_MOUSELOCK,
On 2016/11/06 23:50:12, raymes wrote:
> Please replace these with CONTENT_SETTINGS_TYPE_DEFAULT so that the histogram
> order stays correct (see e.g. MEDIASTREAM below)

Done. Sorry I didn't spot this.

[raymes, 2016-11-07 02:16:51]

lgtm

[Matt Giuca, 2016-11-07 02:35:47]

mgiuca@chromium.org changed reviewers:
+ dbeam@chromium.org, holte@chromium.org, rdevlin.cronin@chromium.org

[Matt Giuca, 2016-11-07 02:35:48]

This CL finally removes fullscreen and mouselock content settings. Thanks,
reviewers.

devlin:
- chrome/browser/extensions/api/content_settings
dbeam:
- chrome/browser/resources/settings/site_settings
- chrome/browser/ui/webui
holte:
- tools/metrics

[Steven Holte, 2016-11-07 20:27:06]

lgtm

[Devlin, 2016-11-07 21:39:52]

Do we have a plan for cleaning up content_settings.js [1]?  It doesn't need to
necessarily be in this CL, but I'd like it to not be something we leave around
indefinitely.

[1]
https://cs.chromium.org/chromium/src/chrome/renderer/resources/extensions/con...

https://codereview.chromium.org/2419413002/diff/240001/components/content_set...
File components/content_settings/core/browser/content_settings_pref_provider.cc
(right):

https://codereview.chromium.org/2419413002/diff/240001/components/content_set...
components/content_settings/core/browser/content_settings_pref_provider.cc:70:
// These prefs have been removed, but need to be registered so they can
Drive-by: We have "MigrateObsoleteBrowserPrefs", which I think is usually the
canonical place to put these, but I could be wrong.

[Matt Giuca, 2016-11-08 00:25:43]

On 2016/11/07 21:39:52, Devlin (slow) wrote:
> Do we have a plan for cleaning up content_settings.js [1]?  It doesn't need to
> necessarily be in this CL, but I'd like it to not be something we leave around
> indefinitely.

You mean the deprecation behaviour for the
chrome.contentSettings.{fullscreen,mouselock} API which returns dummy data when
you access them, instead of exploding? We just put that in a month ago (in the
M55 branch). I think we should wait at least two milestones before taking it out
(because then it will break extensions). I don't mind if you want to do this in
M57 but my gut feeling would be to keep it around a bit longer. Is there
precedent for this?

[Devlin, 2016-11-08 00:45:09]

On 2016/11/08 00:25:43, Matt Giuca wrote:
> On 2016/11/07 21:39:52, Devlin (slow) wrote:
> > Do we have a plan for cleaning up content_settings.js [1]?  It doesn't need
to
> > necessarily be in this CL, but I'd like it to not be something we leave
around
> > indefinitely.
> 
> You mean the deprecation behaviour for the
> chrome.contentSettings.{fullscreen,mouselock} API which returns dummy data
when
> you access them, instead of exploding? We just put that in a month ago (in the
> M55 branch). I think we should wait at least two milestones before taking it
out
> (because then it will break extensions). I don't mind if you want to do this
in
> M57 but my gut feeling would be to keep it around a bit longer. Is there
> precedent for this?

I'm good with leaving it there for now, just making sure it was a deliberate
omission of this CL and that it hadn't fallen off the radar. :)

extensions lgtm

[Matt Giuca, 2016-11-08 00:54:40]

https://codereview.chromium.org/2419413002/diff/240001/components/content_set...
File components/content_settings/core/browser/content_settings_pref_provider.cc
(right):

https://codereview.chromium.org/2419413002/diff/240001/components/content_set...
components/content_settings/core/browser/content_settings_pref_provider.cc:70:
// These prefs have been removed, but need to be registered so they can
On 2016/11/07 21:39:52, Devlin (slow) wrote:
> Drive-by: We have "MigrateObsoleteBrowserPrefs", which I think is usually the
> canonical place to put these, but I could be wrong.

Oh... yeah I didn't know about that :( I wish I did when I implemented this in
https://codereview.chromium.org/2428433002. Thanks for pointing it out.

That's definitely the place to do it, but I don't want to do it in this CL. This
CL is about deleting the content settings enum and minimally updating all the
things that depend on it.

I will migrate these cleanups to MigrateObsoleteProfilePrefs in another CL
(since it will involve rewriting the migration tests as well).

[Devlin, 2016-11-08 00:58:10]

https://codereview.chromium.org/2419413002/diff/240001/components/content_set...
File components/content_settings/core/browser/content_settings_pref_provider.cc
(right):

https://codereview.chromium.org/2419413002/diff/240001/components/content_set...
components/content_settings/core/browser/content_settings_pref_provider.cc:70:
// These prefs have been removed, but need to be registered so they can
On 2016/11/08 00:54:40, Matt Giuca wrote:
> On 2016/11/07 21:39:52, Devlin (slow) wrote:
> > Drive-by: We have "MigrateObsoleteBrowserPrefs", which I think is usually
the
> > canonical place to put these, but I could be wrong.
> 
> Oh... yeah I didn't know about that :( I wish I did when I implemented this in
> https://codereview.chromium.org/2428433002. Thanks for pointing it out.
> 
> That's definitely the place to do it, but I don't want to do it in this CL.
This
> CL is about deleting the content settings enum and minimally updating all the
> things that depend on it.
> 
> I will migrate these cleanups to MigrateObsoleteProfilePrefs in another CL
> (since it will involve rewriting the migration tests as well).

I don't own this code, and only know about MigrateObsoleteProfilePrefs because I
was told to put things there before. :)  If content settings owners are happy
with this, I'm happy.

[Matt Giuca, 2016-11-08 00:59:52]

https://codereview.chromium.org/2419413002/diff/240001/components/content_set...
File components/content_settings/core/browser/content_settings_pref_provider.cc
(right):

https://codereview.chromium.org/2419413002/diff/240001/components/content_set...
components/content_settings/core/browser/content_settings_pref_provider.cc:70:
// These prefs have been removed, but need to be registered so they can
On 2016/11/08 00:58:10, Devlin (slow) wrote:
> On 2016/11/08 00:54:40, Matt Giuca wrote:
> > On 2016/11/07 21:39:52, Devlin (slow) wrote:
> > > Drive-by: We have "MigrateObsoleteBrowserPrefs", which I think is usually
> the
> > > canonical place to put these, but I could be wrong.
> > 
> > Oh... yeah I didn't know about that :( I wish I did when I implemented this
in
> > https://codereview.chromium.org/2428433002. Thanks for pointing it out.
> > 
> > That's definitely the place to do it, but I don't want to do it in this CL.
> This
> > CL is about deleting the content settings enum and minimally updating all
the
> > things that depend on it.
> > 
> > I will migrate these cleanups to MigrateObsoleteProfilePrefs in another CL
> > (since it will involve rewriting the migration tests as well).
> 
> I don't own this code, and only know about MigrateObsoleteProfilePrefs because
I
> was told to put things there before. :)  If content settings owners are happy
> with this, I'm happy.

I think Raymes didn't know about it. Content settings have been migrated
directly in this file before. But I think it makes more sense to have them in
MOBP because they are date-stamped and someone can easily clean them all away
after a year or so.

[Dan Beam, 2016-11-08 07:42:07]

lgtm

[Matt Giuca, 2016-11-08 23:17:19]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2016-11-08 23:18:21]

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-11-08 23:36:42]

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

[commit-bot: I haz the power, 2016-11-08 23:36:43]

Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[raymes, 2016-11-09 04:38:46]

lgtm

[Matt Giuca, 2016-11-09 04:39:21]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2016-11-09 04:39:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org, dbeam@chromium.org, holte@chromium.org
Link to the patchset: https://codereview.chromium.org/2419413002/#ps260001
(title: "Delete tests for content settings.")

[commit-bot: I haz the power, 2016-11-09 04:39:38]

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-11-09 04:55:41]

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

[commit-bot: I haz the power, 2016-11-09 04:55:43]

Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[Matt Giuca, 2016-11-09 05:57:41]

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

[commit-bot: I haz the power, 2016-11-09 05:57: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-11-09 06:15:17]

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

[commit-bot: I haz the power, 2016-11-09 06:15:17]

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

[Dan Beam, 2016-11-09 17:39:41]

https://codereview.chromium.org/2419413002/diff/280001/chrome/browser/resourc...
File chrome/browser/resources/settings/site_settings/site_settings_behavior.js
(right):

https://codereview.chromium.org/2419413002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/settings/site_settings/site_settings_behavior.js:189:
var categoryEnabled = this.computeIsSettingEnabled(category, setting);
fix

[Matt Giuca, 2016-11-09 23:22:19]

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

[commit-bot: I haz the power, 2016-11-09 23:23:01]

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

[Matt Giuca, 2016-11-09 23:41:20]

https://codereview.chromium.org/2419413002/diff/280001/chrome/browser/resourc...
File chrome/browser/resources/settings/site_settings/site_settings_behavior.js
(right):

https://codereview.chromium.org/2419413002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/settings/site_settings/site_settings_behavior.js:189:
var categoryEnabled = this.computeIsSettingEnabled(category, setting);
On 2016/11/09 17:39:41, Dan Beam wrote:
> fix

Done.

[Matt Giuca, 2016-11-10 00:17:25]

mgiuca@chromium.org changed reviewers:
+ palmer@chromium.org

[Matt Giuca, 2016-11-10 00:17:26]

palmer@chromium.org: chrome/browser/ui/cocoa/website_settings, and also a
general FYI since you were one of the people who kicked off this project in the
first place.

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

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

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

Dry run: This issue passed the CQ dry run.

[palmer, 2016-11-10 20:03:19]

LGTM

https://codereview.chromium.org/2419413002/diff/300001/components/content_set...
File components/content_settings/core/browser/content_settings_pref_provider.cc
(right):

https://codereview.chromium.org/2419413002/diff/300001/components/content_set...
components/content_settings/core/browser/content_settings_pref_provider.cc:68:
// Obsolete prefs ----------------------------------------------------------
Likely not in this CL, but is it possible to reduce the duplicated code between
this file and the default_prefs one?

[Matt Giuca, 2016-11-10 23:35:49]

https://codereview.chromium.org/2419413002/diff/300001/components/content_set...
File components/content_settings/core/browser/content_settings_pref_provider.cc
(right):

https://codereview.chromium.org/2419413002/diff/300001/components/content_set...
components/content_settings/core/browser/content_settings_pref_provider.cc:68:
// Obsolete prefs ----------------------------------------------------------
On 2016/11/10 20:03:18, palmer wrote:
> Likely not in this CL, but is it possible to reduce the duplicated code
between
> this file and the default_prefs one?

I'll be moving the obsolete removal stuff to MigrateObsoleteProfilePrefs in
another CL which might reduce some of the duplication.

I don't plan to reduce the more general duplication between these two modules,
though it might be a good thing for someone to do.

[Matt Giuca, 2016-11-10 23:35:58]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2016-11-10 23:35:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org, raymes@chromium.org, dbeam@chromium.org,
holte@chromium.org
Link to the patchset: https://codereview.chromium.org/2419413002/#ps300001
(title: "Fix more things.")

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

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-11-10 23:56:37]

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

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

Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[Matt Giuca, 2016-11-11 01:25:53]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2016-11-11 01:26:12]

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-11-11 02:22:50]

Committed patchset #13 (id:300001)

[commit-bot: I haz the power, 2016-11-11 02:26:45]

Description was changed from

==========
Deleted CONTENT_SETTINGS_TYPE_FULLSCREEN and MOUSELOCK.

Fullscreen and mouselock content settings have not been used for some
time. The pref data was recently deleted and now all code associated
with these settings is vestigial.

BUG=591896,610900
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Deleted CONTENT_SETTINGS_TYPE_FULLSCREEN and MOUSELOCK.

Fullscreen and mouselock content settings have not been used for some
time. The pref data was recently deleted and now all code associated
with these settings is vestigial.

BUG=591896,610900
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/273b9672cfbf83ffed8f52a91cb59a6e78a64577
Cr-Commit-Position: refs/heads/master@{#431465}
==========

[commit-bot: I haz the power, 2016-11-11 02:26:46]

Patchset 13 (id:??) landed as
https://crrev.com/273b9672cfbf83ffed8f52a91cb59a6e78a64577
Cr-Commit-Position: refs/heads/master@{#431465}