[Extensions Bindings] Fix content settings validation

[Extensions Bindings] Fix content settings validation

Fix two issues in content settings validation:
a) Properly validate the `set` value against the schema in JS bindings.
   This wasn't being done before, and could lead to validation failures
   on the browser side.
b) Move deprecated checks before `set` value validation in both native
   and JS bindings. This is necessary since the accepted values for
   the deprecated settings are restricted to a single value (the
   default setting).

BUG=653596
Review-Url: https://codereview.chromium.org/2903803004
Cr-Commit-Position: refs/heads/master@{#475696}
Committed: https://chromium.googlesource.com/chromium/src/+/00d8fa1c1d356a6e98bfffccf62463f1450d4432

[Devlin, 2017-05-25 18:37:25]

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

[commit-bot: I haz the power, 2017-05-25 18:38:13]

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, 2017-05-25 19:26:19]

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

[commit-bot: I haz the power, 2017-05-25 19:26:20]

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_...)

[Devlin, 2017-05-25 21:34:07]

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

[commit-bot: I haz the power, 2017-05-25 21:35:17]

Dry run: CQ is trying da patch.

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

[Devlin, 2017-05-26 01:36:00]

rdevlin.cronin@chromium.org changed reviewers:
+ jbroman@chromium.org, lazyboy@chromium.org

[Devlin, 2017-05-26 01:36:01]

Hey folks, mind taking a look?

https://codereview.chromium.org/2903803004/diff/1/chrome/renderer/resources/e...
File chrome/renderer/resources/extensions/content_setting.js (left):

https://codereview.chromium.org/2903803004/diff/1/chrome/renderer/resources/e...
chrome/renderer/resources/extensions/content_setting.js:64: validate([details,
callback], rawSetSchema);
This was validating only against the raw schema, which was "any".  I introduced
this back in
https://codereview.chromium.org/1906593002/diff/120001/chrome/renderer/resour....
:(

This works now, and the test in the JS exercises this path.

[commit-bot: I haz the power, 2017-05-26 02:04:08]

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

[commit-bot: I haz the power, 2017-05-26 02:04:09]

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

[jbroman, 2017-05-26 18:43:08]

I don't understand (b) in the description. There are a limited number of
accepted values for deprecated settings...therefore they should not be
validated?

[Devlin, 2017-05-26 18:46:41]

Description was changed from

==========
[Extensions Bindings] Fix content settings validation

Fix two issues in content settings validation:
a) Properly validate the `set` value against the schema in JS bindings.
   This wasn't being done before, and could lead to validation failures
   on the browser side.
b) Move deprecated checks before `set` value validation in both native
   and JS bindings. This is necessary since the accepted values for
   the deprecated settings are limited.

BUG=653596
==========

to

==========
[Extensions Bindings] Fix content settings validation

Fix two issues in content settings validation:
a) Properly validate the `set` value against the schema in JS bindings.
   This wasn't being done before, and could lead to validation failures
   on the browser side.
b) Move deprecated checks before `set` value validation in both native
   and JS bindings. This is necessary since the accepted values for
   the deprecated settings are restricted to a single value (the
   default setting).

BUG=653596
==========

[Devlin, 2017-05-26 18:48:58]

On 2017/05/26 18:43:08, jbroman wrote:
> I don't understand (b) in the description. There are a limited number of
> accepted values for deprecated settings...therefore they should not be
> validated?

Yeah, it's kind of weird.  When we deprecated the settings, we restricted the
available values to a set of 1 - whatever the enforced default is.  To support
extensions that haven't been updated, we avoid throwing an error there.  Longer
term, we should probably either throw an error or just remove the settings
entirely, but we need some data on how often they are actually used, and what
that might break.

[jbroman, 2017-05-26 18:59:40]

lgtm

[lazyboy, 2017-05-30 17:07:15]

lgtm

[Devlin, 2017-05-30 19:09:04]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2017-05-30 19:10:40]

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, 2017-05-30 19:19:17]

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

[commit-bot: I haz the power, 2017-05-30 19:19:18]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_headless_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Devlin, 2017-05-30 20:13:14]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2017-05-30 20:13:43]

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, 2017-05-30 23:53:08]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1496175194837780,
"parent_rev": "78b07cc63dc1acfa4a6d94d10ce024d5139448d6", "commit_rev":
"00d8fa1c1d356a6e98bfffccf62463f1450d4432"}

[commit-bot: I haz the power, 2017-05-30 23:53:18]

Description was changed from

==========
[Extensions Bindings] Fix content settings validation

Fix two issues in content settings validation:
a) Properly validate the `set` value against the schema in JS bindings.
   This wasn't being done before, and could lead to validation failures
   on the browser side.
b) Move deprecated checks before `set` value validation in both native
   and JS bindings. This is necessary since the accepted values for
   the deprecated settings are restricted to a single value (the
   default setting).

BUG=653596
==========

to

==========
[Extensions Bindings] Fix content settings validation

Fix two issues in content settings validation:
a) Properly validate the `set` value against the schema in JS bindings.
   This wasn't being done before, and could lead to validation failures
   on the browser side.
b) Move deprecated checks before `set` value validation in both native
   and JS bindings. This is necessary since the accepted values for
   the deprecated settings are restricted to a single value (the
   default setting).

BUG=653596

Review-Url: https://codereview.chromium.org/2903803004
Cr-Commit-Position: refs/heads/master@{#475696}
Committed:
https://chromium.googlesource.com/chromium/src/+/00d8fa1c1d356a6e98bfffccf624...
==========

[commit-bot: I haz the power, 2017-05-30 23:53:19]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/00d8fa1c1d356a6e98bfffccf624...