MD Settings: Content Settings: Support guest mode.

MD Settings: Content Settings: Support guest mode.

Specifically, in Guest mode:
* Disable protected content enabled toggles
* Disable the primary toggle for 'popups'
* Hide the protocol-handler section and do not request the default
  value (which triggers a DCHECK).

BUG=688485
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2689303002
Cr-Commit-Position: refs/heads/master@{#450461}
Committed: https://chromium.googlesource.com/chromium/src/+/7c0137c32005da3ecfeab84cc6905464af3035c0

[stevenjb, 2017-02-13 22:54:09]

Description was changed from

==========
MD Settings: Content Settings: Support guest mode.

Specifically, in Guest mode:
* Disable protected content enabled toggles
* Disable the primary toggle for 'popups'
* Hide the protocol-handler section and do not request the default
  value (which triggers a DCHECK).

BUG=688485
==========

to

==========
MD Settings: Content Settings: Support guest mode.

Specifically, in Guest mode:
* Disable protected content enabled toggles
* Disable the primary toggle for 'popups'
* Hide the protocol-handler section and do not request the default
  value (which triggers a DCHECK).

BUG=688485
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[stevenjb, 2017-02-13 22:56:18]

stevenjb@chromium.org changed reviewers:
+ dschuyler@chromium.org, tommycli@chromium.org

[tommycli, 2017-02-14 00:28:39]

lgtm other than below

https://codereview.chromium.org/2689303002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/privacy_page/privacy_page.html (right):

https://codereview.chromium.org/2689303002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/privacy_page/privacy_page.html:427:
disabled$="[[isGuest_]]">
The existing usages of isGuest_ in the code specify that the control is entirely
gone (dom-ifed out). In these cases UI specified it should be just disabled?

https://codereview.chromium.org/2689303002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/site_settings/category_default_setting.js
(right):

https://codereview.chromium.org/2689303002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/category_default_setting.js:207:
isToggleEnabled_: function() {
since it's only disabled if it's on POPUPS AND in guest mode, (disabling is the
exceptional case), consider making this isToggleDisabled, and get rid of the !
in the binding.

That would be easier to grok for me, but i realize this is kind of subjective.

[stevenjb, 2017-02-14 00:49:23]

https://codereview.chromium.org/2689303002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/privacy_page/privacy_page.html (right):

https://codereview.chromium.org/2689303002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/privacy_page/privacy_page.html:427:
disabled$="[[isGuest_]]">
On 2017/02/14 00:28:39, tommycli wrote:
> The existing usages of isGuest_ in the code specify that the control is
entirely
> gone (dom-ifed out). In these cases UI specified it should be just disabled?

This is consistent with current Settings. I stuck with this model because:
a) Exceptions can still be added
b) It may help explain to the user why it doesn't work. Ideally at some point we
could add a policy like icon here with an explanatory tooltip.

https://codereview.chromium.org/2689303002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/site_settings/category_default_setting.js
(right):

https://codereview.chromium.org/2689303002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/category_default_setting.js:207:
isToggleEnabled_: function() {
On 2017/02/14 00:28:39, tommycli wrote:
> since it's only disabled if it's on POPUPS AND in guest mode, (disabling is
the
> exceptional case), consider making this isToggleDisabled, and get rid of the !
> in the binding.
> 
> That would be easier to grok for me, but i realize this is kind of subjective.

Hmm, I thought we had more disabled$="[[!isFooEnabled]]" than [[isFooDisabled]]
in Settings, but we appear to be split. I don't feel strongly, so done.

[stevenjb, 2017-02-14 00:49:28]

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

[commit-bot: I haz the power, 2017-02-14 00:50:37]

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-02-14 00:54:03]

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

[commit-bot: I haz the power, 2017-02-14 00:54:04]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[stevenjb, 2017-02-14 01:59:52]

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

[commit-bot: I haz the power, 2017-02-14 02:00:46]

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-02-14 04:30:09]

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

[commit-bot: I haz the power, 2017-02-14 04:30:09]

Dry run: This issue passed the CQ dry run.

[stevenjb, 2017-02-14 18:04:36]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2017-02-14 18:04:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
tommycli@chromium.org
Link to the patchset: https://codereview.chromium.org/2689303002/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2017-02-14 18:05:20]

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-02-14 18:39:33]

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

[commit-bot: I haz the power, 2017-02-14 18:39:35]

Failed to apply patch for
chrome/browser/resources/settings/privacy_page/privacy_page.js:
While running git apply --index -p1;
  error: patch failed:
chrome/browser/resources/settings/privacy_page/privacy_page.js:44
  error: chrome/browser/resources/settings/privacy_page/privacy_page.js: patch
does not apply

Patch:       chrome/browser/resources/settings/privacy_page/privacy_page.js
Index: chrome/browser/resources/settings/privacy_page/privacy_page.js
diff --git a/chrome/browser/resources/settings/privacy_page/privacy_page.js
b/chrome/browser/resources/settings/privacy_page/privacy_page.js
index
4268e9ba851d433fb214f26149ca7ffa46b32330..e7600c3e701f5968a7361f5a629540c42af9da65
100644
--- a/chrome/browser/resources/settings/privacy_page/privacy_page.js
+++ b/chrome/browser/resources/settings/privacy_page/privacy_page.js
@@ -44,6 +44,12 @@ Polymer({
      */
     pageVisibility: Object,
 
+    /** @private */
+    isGuest_: {
+      type: Boolean,
+      value: function() { return loadTimeData.getBoolean('isGuest'); }
+    },
+
 // <if expr="_google_chrome and not chromeos">
     /** @type {MetricsReporting} */
     metricsReporting_: Object,

[stevenjb, 2017-02-14 19:22:10]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2017-02-14 19:22:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
tommycli@chromium.org
Link to the patchset: https://codereview.chromium.org/2689303002/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2017-02-14 19:23:02]

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-02-14 20:53:35]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1487100130307340,
"parent_rev": "c02b4fd3cd8f6c0a95ca4bbba40326bf42ebf2cd", "commit_rev":
"7c0137c32005da3ecfeab84cc6905464af3035c0"}

[commit-bot: I haz the power, 2017-02-14 20:54:14]

Description was changed from

==========
MD Settings: Content Settings: Support guest mode.

Specifically, in Guest mode:
* Disable protected content enabled toggles
* Disable the primary toggle for 'popups'
* Hide the protocol-handler section and do not request the default
  value (which triggers a DCHECK).

BUG=688485
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Content Settings: Support guest mode.

Specifically, in Guest mode:
* Disable protected content enabled toggles
* Disable the primary toggle for 'popups'
* Hide the protocol-handler section and do not request the default
  value (which triggers a DCHECK).

BUG=688485
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2689303002
Cr-Commit-Position: refs/heads/master@{#450461}
Committed:
https://chromium.googlesource.com/chromium/src/+/7c0137c32005da3ecfeab84cc690...
==========

[commit-bot: I haz the power, 2017-02-14 20:54:15]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/7c0137c32005da3ecfeab84cc690...