Deprecate chrome.contentSettings.{fullscreen,mouselock} extension API.

Deprecate chrome.contentSettings.{fullscreen,mouselock} extension API.

These settings no longer hook up to actual data; they just return
'allow' always, and setting them has no effect. Updated documentation.

This is because Chrome now (and has for awhile) ignores these settings
and always permits these actions without asking the user.

BUG=591896
Committed: https://crrev.com/9b4b6268ca5e5eb81910dd304c8408eeff0f4e64
Cr-Commit-Position: refs/heads/master@{#422716}

[Matt Giuca, 2016-09-22 06:36:31]

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

[commit-bot: I haz the power, 2016-09-22 06:36:43]

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-09-22 07:11:28]

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

[commit-bot: I haz the power, 2016-09-22 07:11:29]

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

[Matt Giuca, 2016-09-23 04:41:09]

mgiuca@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Matt Giuca, 2016-09-23 04:41:10]

Hi Devlin,

As discussed, I think the stub is the best course of action for now.

This won't delete the data associated with the extensions; that needs to be done
in a follow-up.

[Matt Giuca, 2016-09-23 04:44:11]

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

[commit-bot: I haz the power, 2016-09-23 04:44:23]

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-09-23 05:43:53]

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

[commit-bot: I haz the power, 2016-09-23 05:43:54]

Dry run: This issue passed the CQ dry run.

[Devlin, 2016-09-26 20:30:37]

https://codereview.chromium.org/2358893004/diff/40001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/content_setting.js (right):

https://codereview.chromium.org/2358893004/diff/40001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/content_setting.js:64: if
(DEPRECATED_CONTENT_TYPES.hasOwnProperty(contentType)) {
Use the safe-builtin version.

https://codereview.chromium.org/2358893004/diff/40001/chrome/test/data/extens...
File chrome/test/data/extensions/api_test/content_settings/standard/test.js
(right):

https://codereview.chromium.org/2358893004/diff/40001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/content_settings/standard/test.js:42: var
expectations = {
I would add a bit more context here, and rename this to something like
deprecatedSettingsExpectations.

https://codereview.chromium.org/2358893004/diff/40001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/content_settings/standard/test.js:44:
"fullscreen": "allow",
prefer single quotes in js

[Matt Giuca, 2016-09-27 01:01:23]

Patchset #4 (id:60001) has been deleted

[Matt Giuca, 2016-09-27 01:18:33]

https://codereview.chromium.org/2358893004/diff/40001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/content_setting.js (right):

https://codereview.chromium.org/2358893004/diff/40001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/content_setting.js:64: if
(DEPRECATED_CONTENT_TYPES.hasOwnProperty(contentType)) {
On 2016/09/26 20:30:37, Devlin (catching up) wrote:
> Use the safe-builtin version.

Sorry, I don't know what you want.

Do you mean to just write "contentType in DEPRECATED_CONTENT_TYPES"? As I
understand it, that will unnecessarily search up the prototype chain so you're
better off using hasOwnProperty (I don't know what makes in "safe"). If you want
me to change it, I can, just please be clear.

https://codereview.chromium.org/2358893004/diff/40001/chrome/test/data/extens...
File chrome/test/data/extensions/api_test/content_settings/standard/test.js
(right):

https://codereview.chromium.org/2358893004/diff/40001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/content_settings/standard/test.js:42: var
expectations = {
On 2016/09/26 20:30:37, Devlin (catching up) wrote:
> I would add a bit more context here, and rename this to something like
> deprecatedSettingsExpectations.

Done.

https://codereview.chromium.org/2358893004/diff/40001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/content_settings/standard/test.js:44:
"fullscreen": "allow",
On 2016/09/26 20:30:37, Devlin (catching up) wrote:
> prefer single quotes in js

I think it's appropriate to stay consistent with the style of the above two
blocks which use double quotes.

[Devlin, 2016-09-27 15:22:36]

https://codereview.chromium.org/2358893004/diff/40001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/content_setting.js (right):

https://codereview.chromium.org/2358893004/diff/40001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/content_setting.js:13: var
DEPRECATED_CONTENT_TYPES = {
forgot - please add __proto__: null to this object.

https://codereview.chromium.org/2358893004/diff/40001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/content_setting.js:64: if
(DEPRECATED_CONTENT_TYPES.hasOwnProperty(contentType)) {
On 2016/09/27 01:18:33, Matt Giuca wrote:
> On 2016/09/26 20:30:37, Devlin (catching up) wrote:
> > Use the safe-builtin version.
> 
> Sorry, I don't know what you want.
> 
> Do you mean to just write "contentType in DEPRECATED_CONTENT_TYPES"? As I
> understand it, that will unnecessarily search up the prototype chain so you're
> better off using hasOwnProperty (I don't know what makes in "safe"). If you
want
> me to change it, I can, just please be clear.

Safe Builtins are cached (safe) versions of various built-in (native) methods
that we construct at context initialization so that if someone writes
Object.prototype.hasOwnProperty = function() { throw new Error('hahaha'); }, our
code doesn't break.  You're actually already using them - $Function is the alias
for our cached function implementation. :)

So here, instead of doing DEPRECATED_CONTENT_TYPES.hasOwnProperty(), we should
do $Object.hasOwnProperty(DEPRECATED_CONTENT_TYPES, contentType).

[Matt Giuca, 2016-09-29 03:13:45]

https://codereview.chromium.org/2358893004/diff/40001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/content_setting.js (right):

https://codereview.chromium.org/2358893004/diff/40001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/content_setting.js:13: var
DEPRECATED_CONTENT_TYPES = {
On 2016/09/27 15:22:36, Devlin (OOO until Thursday) wrote:
> forgot - please add __proto__: null to this object.

Done.

https://codereview.chromium.org/2358893004/diff/40001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/content_setting.js:64: if
(DEPRECATED_CONTENT_TYPES.hasOwnProperty(contentType)) {
On 2016/09/27 15:22:36, Devlin (OOO until Thursday) wrote:
> On 2016/09/27 01:18:33, Matt Giuca wrote:
> > On 2016/09/26 20:30:37, Devlin (catching up) wrote:
> > > Use the safe-builtin version.
> > 
> > Sorry, I don't know what you want.
> > 
> > Do you mean to just write "contentType in DEPRECATED_CONTENT_TYPES"? As I
> > understand it, that will unnecessarily search up the prototype chain so
you're
> > better off using hasOwnProperty (I don't know what makes in "safe"). If you
> want
> > me to change it, I can, just please be clear.
> 
> Safe Builtins are cached (safe) versions of various built-in (native) methods
> that we construct at context initialization so that if someone writes
> Object.prototype.hasOwnProperty = function() { throw new Error('hahaha'); },
our
> code doesn't break.  You're actually already using them - $Function is the
alias
> for our cached function implementation. :)
> 
> So here, instead of doing DEPRECATED_CONTENT_TYPES.hasOwnProperty(), we should
> do $Object.hasOwnProperty(DEPRECATED_CONTENT_TYPES, contentType).

Done. OK thanks for the explanation.

[Devlin, 2016-09-29 17:09:10]

lgtm

[Matt Giuca, 2016-10-04 04:45:57]

The CQ bit was checked by mgiuca@chromium.org

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

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-04 05:41:35]

Committed patchset #5 (id:100001)

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

Description was changed from

==========
Deprecate chrome.contentSettings.{fullscreen,mouselock} extension API.

These settings no longer hook up to actual data; they just return
'allow' always, and setting them has no effect. Updated documentation.

This is because Chrome now (and has for awhile) ignores these settings
and always permits these actions without asking the user.

BUG=591896
==========

to

==========
Deprecate chrome.contentSettings.{fullscreen,mouselock} extension API.

These settings no longer hook up to actual data; they just return
'allow' always, and setting them has no effect. Updated documentation.

This is because Chrome now (and has for awhile) ignores these settings
and always permits these actions without asking the user.

BUG=591896

Committed: https://crrev.com/9b4b6268ca5e5eb81910dd304c8408eeff0f4e64
Cr-Commit-Position: refs/heads/master@{#422716}
==========

[commit-bot: I haz the power, 2016-10-04 05:44:29]

Patchset 5 (id:??) landed as
https://crrev.com/9b4b6268ca5e5eb81910dd304c8408eeff0f4e64
Cr-Commit-Position: refs/heads/master@{#422716}