MD Settings: show an extension-controlled-indicator for proxy

MD Settings: show an extension-controlled-indicator for proxy

R=dpapad@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[Dan Beam, 2016-11-17 04:14:51]

Description was changed from

==========
MD Settings: show an extension-controlled-indicator for proxy

R=dpapad@chromium.org
BUG=614265
==========

to

==========
MD Settings: show an extension-controlled-indicator for proxy

R=dpapad@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2016-11-17 04:16:36]

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

[commit-bot: I haz the power, 2016-11-17 04:17: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, 2016-11-17 04:33:33]

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

[commit-bot: I haz the power, 2016-11-17 04:33:34]

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

[dpapad, 2016-11-17 18:36:34]

https://codereview.chromium.org/2506323002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/system_page/system_page.js (right):

https://codereview.chromium.org/2506323002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/system_page/system_page.js:42:
document.querySelector('settings-ui').getPrefsElement().refresh('proxy');
With this selector system_page assumes that a settings-ui parent exists, and
makes a function call on it, which breaks encapsulation. For example, can you
still unit tests a <settings-system-page> as the only element in a page?

Alternative: Can we fire an event
this.fire('refresh-pref', 'proxy') and catch that event at a higher level, in
settings-ui itself? That makes the coupling between system-page and its parents
much looser (and keeps system-page test friendlier).

[Dan Beam, 2016-11-17 19:12:25]

https://codereview.chromium.org/2506323002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/system_page/system_page.js (right):

https://codereview.chromium.org/2506323002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/system_page/system_page.js:42:
document.querySelector('settings-ui').getPrefsElement().refresh('proxy');
On 2016/11/17 18:36:34, dpapad wrote:
> With this selector system_page assumes that a settings-ui parent exists, and
> makes a function call on it, which breaks encapsulation. For example, can you
> still unit tests a <settings-system-page> as the only element in a page?
> 
> Alternative: Can we fire an event
> this.fire('refresh-pref', 'proxy') and catch that event at a higher level, in
> settings-ui itself? That makes the coupling between system-page and its
parents
> much looser (and keeps system-page test friendlier).

that's a good idea

[Dan Beam, 2016-11-17 22:10:10]

Patchset #2 (id:20001) has been deleted

[Dan Beam, 2016-11-17 22:10:38]

https://codereview.chromium.org/2506323002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/system_page/system_page.js (right):

https://codereview.chromium.org/2506323002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/system_page/system_page.js:42:
document.querySelector('settings-ui').getPrefsElement().refresh('proxy');
On 2016/11/17 19:12:25, Dan Beam wrote:
> On 2016/11/17 18:36:34, dpapad wrote:
> > With this selector system_page assumes that a settings-ui parent exists, and
> > makes a function call on it, which breaks encapsulation. For example, can
you
> > still unit tests a <settings-system-page> as the only element in a page?
> > 
> > Alternative: Can we fire an event
> > this.fire('refresh-pref', 'proxy') and catch that event at a higher level,
in
> > settings-ui itself? That makes the coupling between system-page and its
> parents
> > much looser (and keeps system-page test friendlier).
> 
> that's a good idea

Done.

[Dan Beam, 2016-11-17 22:10:42]

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

[commit-bot: I haz the power, 2016-11-17 22:11:51]

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

[dpapad, 2016-11-17 22:14:47]

lgtm

[Dan Beam, 2016-11-17 23:15:29]

The CQ bit was unchecked by dbeam@chromium.org

[Dan Beam, 2016-11-17 23:17:43]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2016-11-17 23:17:43]

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

[commit-bot: I haz the power, 2016-11-17 23:18:49]

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

[Dan Beam, 2016-11-18 00:32:45]

The CQ bit was unchecked by dbeam@chromium.org

[Dan Beam, 2016-11-18 00:32:50]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2016-11-18 00:34:42]

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-18 01:23:00]

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

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

Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[Dan Beam, 2016-11-18 02:30:55]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2016-11-18 02:32:24]

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-18 02:54:43]

Committed patchset #3 (id:60001)

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

Description was changed from

==========
MD Settings: show an extension-controlled-indicator for proxy

R=dpapad@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: show an extension-controlled-indicator for proxy

R=dpapad@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[commit-bot: I haz the power, 2016-11-18 02:56:47]

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