MD Site Settings: Implement the third-party checkbox for Cookies.

MD Site Settings: Implement the third-party checkbox for Cookies.

BUG=614277, 543635
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/c1a841341be6f0ea2e6419ba6208d4c424fa021b
Cr-Commit-Position: refs/heads/master@{#397842}

[Finnur, 2016-06-02 17:55:43]

Description was changed from

==========
MD Site Settings: Implement the third-party checkbox for Cookies.

BUG=614277, 543635
==========

to

==========
MD Site Settings: Implement the third-party checkbox for Cookies.

BUG=614277, 543635
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[Finnur, 2016-06-02 18:04:37]

finnur@chromium.org changed reviewers:
+ michaelpg@chromium.org

[michaelpg, 2016-06-03 13:43:02]

Do you anticipate other categories having special UI? If so, perhaps this would
be better achieved with <content> nodes instead of hard-coding rarely used UI
inside a shared UI. Both have trade-offs and I'm sure you've thought about them,
so lgtm.

https://codereview.chromium.org/2031833003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/site_settings_category.js
(right):

https://codereview.chromium.org/2031833003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/site_settings_category.js:69:
this.$.cookieControls.hidden = false;
nit: no {} around single-line if

[Finnur, 2016-06-03 18:16:32]

I do anticipate more special and I have not thought about <content> nodes. Can
you point me to an example?

In the interest of time (I'm leaving the country shortly) I might check this in
as-is (with the nit fixed) and convert this to content nodes once I have the
next checkbox lined up.

[Finnur, 2016-06-03 18:16:46]

... more special UI*

[Finnur, 2016-06-03 20:02:05]

https://codereview.chromium.org/2031833003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/site_settings_category.js
(right):

https://codereview.chromium.org/2031833003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/site_settings_category.js:69:
this.$.cookieControls.hidden = false;
On 2016/06/03 13:43:02, michaelpg wrote:
> nit: no {} around single-line if

Done.

[Finnur, 2016-06-03 20:02:09]

The CQ bit was checked by finnur@chromium.org

[Finnur, 2016-06-03 20:02:10]

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

[commit-bot: I haz the power, 2016-06-03 20:02:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2031833003/40001

[Finnur, 2016-06-03 20:09:52]

The CQ bit was unchecked by finnur@chromium.org

[michaelpg, 2016-06-03 20:21:33]

On 2016/06/03 18:16:32, Finnur wrote:
> I do anticipate more special and I have not thought about <content> nodes. Can
> you point me to an example?

Polymer documentation:
https://www.polymer-project.org/1.0/docs/devguide/local-dom#dom-distribution

But it's just a Web Components thing, so you can find more detailed
documentation elsewhere.

Here's a simple example:
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/contro...

Here's a more complex example using selectors (in this case, class names):
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/settin...

If you search for uses of those elements, you'll see the DOM that gets
distributed into the content nodes.

> 
> In the interest of time (I'm leaving the country shortly) I might check this
in
> as-is (with the nit fixed) and convert this to content nodes once I have the
> next checkbox lined up.

lgtm

[michaelpg, 2016-06-03 20:21:45]

On 2016/06/03 20:21:33, michaelpg wrote:
> On 2016/06/03 18:16:32, Finnur wrote:
> > I do anticipate more special and I have not thought about <content> nodes.
Can
> > you point me to an example?
> 
> Polymer documentation:
> https://www.polymer-project.org/1.0/docs/devguide/local-dom#dom-distribution
> 
> But it's just a Web Components thing, so you can find more detailed
> documentation elsewhere.
> 
> Here's a simple example:
>
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/contro...
> 
> Here's a more complex example using selectors (in this case, class names):
>
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/settin...
> 
> If you search for uses of those elements, you'll see the DOM that gets
> distributed into the content nodes.
> 
> > 
> > In the interest of time (I'm leaving the country shortly) I might check this
> in
> > as-is (with the nit fixed) and convert this to content nodes once I have the
> > next checkbox lined up.
> 
> lgtm

sgtm*

[Finnur, 2016-06-03 22:10:10]

The CQ bit was checked by finnur@chromium.org

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

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2031833003/40001

[commit-bot: I haz the power, 2016-06-03 23:36:59]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-03 23:39:55]

Description was changed from

==========
MD Site Settings: Implement the third-party checkbox for Cookies.

BUG=614277, 543635
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Site Settings: Implement the third-party checkbox for Cookies.

BUG=614277, 543635
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

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

[commit-bot: I haz the power, 2016-06-03 23:39:56]

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