MD Settings: Internet: Move network lists to a subpage

MD Settings: Internet: Move network lists to a subpage

This CL:
* Moves the lists of networks and the associated logic
from network-summary-item to internet-subpage.
* Changes the network-summary listener pattern to match other usage.
* Adds missing aria-label for buttons and toggles.
* Fixes a bug in cr-network-icon causing it to show the security badge for disconnected summary items.
* Adds a separator bar before the subpage button in cr-network-list-item.


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

Review-Url: https://codereview.chromium.org/2720503006
Cr-Commit-Position: refs/heads/master@{#454368}
Committed: https://chromium.googlesource.com/chromium/src/+/945290e9adf66a1cc7cd5a1fd089fe3ae3400a8b

[stevenjb, 2017-02-28 01:25:07]

Description was changed from

==========
MD Settings: Internet: Move network lists to a subpage

This CL moves the lists of networks and the associated logic
from network-summary-item to internet-subpage.

BUG=673944
==========

to

==========
MD Settings: Internet: Move network lists to a subpage

This CL moves the lists of networks and the associated logic
from network-summary-item to internet-subpage.

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

[stevenjb, 2017-02-28 18:27:13]

Description was changed from

==========
MD Settings: Internet: Move network lists to a subpage

This CL moves the lists of networks and the associated logic
from network-summary-item to internet-subpage.

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

to

==========
MD Settings: Internet: Move network lists to a subpage

This CL:
* Moves the lists of networks and the associated logic
from network-summary-item to internet-subpage.
* Changes the network-summary listener pattern to match other usage.
* Adds missing aria-label for buttons and toggles.
* Fixes a bug in cr-network-icon causing it to show the security badge for
disconnected summary items.
* Adds a separator bar before the subpage button in cr-network-list-item.


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

[stevenjb, 2017-02-28 18:33:53]

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

[stevenjb, 2017-02-28 18:33:53]

PTAL when you have a chance. I'd love to get this in before the branch point
Thurs.

[stevenjb, 2017-03-01 21:47:27]

re-uploaded with --similarity=20 to get delta from network-summary-item.js ->
internet_subpage.js, but it's still a pretty significant delta, sorry.

[michaelpg, 2017-03-01 23:28:00]

Comments span a couple patches, just FYI.

Most of these are nits (and many of those are optional) so I'm happy to stamp as
soon as you like.

https://codereview.chromium.org/2720503006/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/internet_page.js (right):

https://codereview.chromium.org/2720503006/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_page.js:56: notify:
true,
notify: true means it should propagate upwards; internet-subpage needs this but
not internet-page

as a rule of thumb it doesn't make sense on @private: it's encapsulated, and not
part of the contract our element has with its users

https://codereview.chromium.org/2720503006/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_page.js:159: var type =
queryParams.get('type');
nit: instead of creating a variable, use queryParams.has('type')

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/internet_subpage.html
(right):

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.html:28:
margin: 8px 12px 8px 4px;
needs RTLification

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/internet_subpage.js
(right):

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:6: *
@fileoverview Polymer element for displaying informaiton about WiFi,
information

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:24: *
Highest priority connected network or null. Set by network-summary.
I think "Set by network-summary." can be misleading. network-summary is where
the value originates, but that's a detail for settings-internet-page, which is
what binds it to this.

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:41: *  
{!Array<!chrome.networkingPrivate.ThirdPartyVPNProperties>|undefined}
nit: 4-space indent?

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:97:
currentRouteChanged: function(route) {
nit: per the style guide, should be ordered after lifecycle methods

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:105: //
Request the list of networks and start scanning of necessary.
of necessary => if necessary

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:267:
getAddThirdPartyVpnA11yString: function(vpnState) {
add trailing _ for private method

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:299: *      
     !{item: !chrome.networkingPrivate.ThirdPartyVPNProperties},
nit: 4-space indent relative to the nearest opening brace, ie placing "!"
directly below the "l" in model

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:333: var
type = this.deviceState ? this.deviceState.Type : '';
guaranteed to be true after assert (assuming deviceIsEnabled_() doesn't have
side effects...)

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:396: return
state.ConnectionState == CrOnc.ConnectionState.NOT_CONNECTED;
opt nit: I would make this just another condition, at the top of the function:
    if (state.CS != NOT_CONNECTED)
      return false;
and then return true at the end, so all the conditions have the same format. As
a bonus, I think it does less work in the common case.

https://codereview.chromium.org/2720503006/diff/40001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/fake_networking_private.js (right):

https://codereview.chromium.org/2720503006/diff/40001/chrome/test/data/webui/...
chrome/test/data/webui/settings/fake_networking_private.js:75: var states = [];
opt nit: It's just a style thing, but this.networkStates_.filter() would fit
well here.

https://codereview.chromium.org/2720503006/diff/40001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/internet_page_tests.js (right):

https://codereview.chromium.org/2720503006/diff/40001/chrome/test/data/webui/...
chrome/test/data/webui/settings/internet_page_tests.js:115: var api =
networkingPrivateApi_;
is this just to have a shortcut? networkingPrivateApi_ is only used twice :-\

https://codereview.chromium.org/2720503006/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list.html (left):

https://codereview.chromium.org/2720503006/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list.html:16: max-height:
1000px;
would this affect OOBE somehow?

[stevenjb, 2017-03-02 00:25:17]

PTAL

https://codereview.chromium.org/2720503006/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/internet_page.js (right):

https://codereview.chromium.org/2720503006/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_page.js:56: notify:
true,
On 2017/03/01 23:27:59, michaelpg wrote:
> notify: true means it should propagate upwards; internet-subpage needs this
but
> not internet-page
> 
> as a rule of thumb it doesn't make sense on @private: it's encapsulated, and
not
> part of the contract our element has with its users

Done.

https://codereview.chromium.org/2720503006/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_page.js:159: var type =
queryParams.get('type');
On 2017/03/01 23:27:59, michaelpg wrote:
> nit: instead of creating a variable, use queryParams.has('type')

But.... that requires two lookups! I know it's probably a hash, but the C++
developer in me bristles at the notion :)

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/internet_subpage.html
(right):

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.html:28:
margin: 8px 12px 8px 4px;
On 2017/03/01 23:27:59, michaelpg wrote:
> needs RTLification

Oops, thanks, done.

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/internet_subpage.js
(right):

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:6: *
@fileoverview Polymer element for displaying informaiton about WiFi,
On 2017/03/01 23:27:59, michaelpg wrote:
> information

Done.

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:24: *
Highest priority connected network or null. Set by network-summary.
On 2017/03/01 23:27:59, michaelpg wrote:
> I think "Set by network-summary." can be misleading. network-summary is where
> the value originates, but that's a detail for settings-internet-page, which is
> what binds it to this.

While that is true, I think it is a useful detail for debugging. Data binding is
not super intuitive. I tried to clarify this a bit.

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:41: *  
{!Array<!chrome.networkingPrivate.ThirdPartyVPNProperties>|undefined}
On 2017/03/01 23:27:59, michaelpg wrote:
> nit: 4-space indent?

Wasn't sure here. Done.

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:97:
currentRouteChanged: function(route) {
On 2017/03/01 23:27:59, michaelpg wrote:
> nit: per the style guide, should be ordered after lifecycle methods

Done.

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:105: //
Request the list of networks and start scanning of necessary.
On 2017/03/01 23:27:59, michaelpg wrote:
> of necessary => if necessary

Done.

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:267:
getAddThirdPartyVpnA11yString: function(vpnState) {
On 2017/03/01 23:27:59, michaelpg wrote:
> add trailing _ for private method

Oops, good catch. No headphones here, I haven't tested chromevox, I plan to do
an a networking audit soon.

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:299: *      
     !{item: !chrome.networkingPrivate.ThirdPartyVPNProperties},
On 2017/03/01 23:27:59, michaelpg wrote:
> nit: 4-space indent relative to the nearest opening brace, ie placing "!"
> directly below the "l" in model

Hm. OK, done. (FWIW that pattern makes this sort of wrapping almost useless - it
only saves 2 characters - but in this case and the one above it happens to
work...)

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:333: var
type = this.deviceState ? this.deviceState.Type : '';
On 2017/03/01 23:27:59, michaelpg wrote:
> guaranteed to be true after assert (assuming deviceIsEnabled_() doesn't have
> side effects...)

Done.

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:396: return
state.ConnectionState == CrOnc.ConnectionState.NOT_CONNECTED;
On 2017/03/01 23:27:59, michaelpg wrote:
> opt nit: I would make this just another condition, at the top of the function:
>     if (state.CS != NOT_CONNECTED)
>       return false;
> and then return true at the end, so all the conditions have the same format.
As
> a bonus, I think it does less work in the common case.

Done.

https://codereview.chromium.org/2720503006/diff/40001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/fake_networking_private.js (right):

https://codereview.chromium.org/2720503006/diff/40001/chrome/test/data/webui/...
chrome/test/data/webui/settings/fake_networking_private.js:75: var states = [];
On 2017/03/01 23:27:59, michaelpg wrote:
> opt nit: It's just a style thing, but this.networkStates_.filter() would fit
> well here.

One of these days I'll learn these crazy Javascripty ways. Done.

https://codereview.chromium.org/2720503006/diff/40001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/internet_page_tests.js (right):

https://codereview.chromium.org/2720503006/diff/40001/chrome/test/data/webui/...
chrome/test/data/webui/settings/internet_page_tests.js:115: var api =
networkingPrivateApi_;
On 2017/03/01 23:27:59, michaelpg wrote:
> is this just to have a shortcut? networkingPrivateApi_ is only used twice :-\

Yeah but it's long and makes the second usage ugly. We do this a few places,
I'll just rename the variable...

https://codereview.chromium.org/2720503006/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list.html (left):

https://codereview.chromium.org/2720503006/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list.html:16: max-height:
1000px;
On 2017/03/01 23:28:00, michaelpg wrote:
> would this affect OOBE somehow?

Nope, it's infinite also. This is from an ancient early prototype and no longer
has nay purpose.

[michaelpg, 2017-03-02 01:25:00]

lgtm

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/internet_subpage.js
(right):

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:24: *
Highest priority connected network or null. Set by network-summary.
On 2017/03/02 00:25:17, stevenjb wrote:
> On 2017/03/01 23:27:59, michaelpg wrote:
> > I think "Set by network-summary." can be misleading. network-summary is
where
> > the value originates, but that's a detail for settings-internet-page, which
is
> > what binds it to this.
> 
> While that is true, I think it is a useful detail for debugging. Data binding
is
> not super intuitive. I tried to clarify this a bit.

Great, thanks for making it clearer.

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:41: *  
{!Array<!chrome.networkingPrivate.ThirdPartyVPNProperties>|undefined}
On 2017/03/02 00:25:17, stevenjb wrote:
> On 2017/03/01 23:27:59, michaelpg wrote:
> > nit: 4-space indent?
> 
> Wasn't sure here. Done.

There are almost no examples in the codebase. I found these two which each do
different things:

https://cs.chromium.org/chromium/src/ui/webui/resources/js/cr/ui/table.js?typ...

https://cs.chromium.org/chromium/src/third_party/google_input_tools/third_par...

So... do whatever seems reasonable :-)

https://codereview.chromium.org/2720503006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_subpage.js:299: *      
     !{item: !chrome.networkingPrivate.ThirdPartyVPNProperties},
On 2017/03/02 00:25:17, stevenjb wrote:
> On 2017/03/01 23:27:59, michaelpg wrote:
> > nit: 4-space indent relative to the nearest opening brace, ie placing "!"
> > directly below the "l" in model
> 
> Hm. OK, done. (FWIW that pattern makes this sort of wrapping almost useless -
it
> only saves 2 characters - but in this case and the one above it happens to
> work...)
> 

indenting only 4 characters (relative to @param) would probably be fine, too

or break after the {, like in JS?

[stevenjb, 2017-03-02 01:53:49]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2017-03-02 01:54:51]

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-03-02 05:30:35]

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

[commit-bot: I haz the power, 2017-03-02 05:30:36]

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

[stevenjb, 2017-03-02 16:34:56]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2017-03-02 16:35:18]

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-03-02 18:08:00]

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

[commit-bot: I haz the power, 2017-03-02 18:08:03]

Failed to apply patch for
chrome/browser/resources/settings/internet_page/network_summary_item.html:
While running git apply --index -p1;
  error: patch failed:
chrome/browser/resources/settings/internet_page/network_summary_item.html:1
  error:
chrome/browser/resources/settings/internet_page/network_summary_item.html: patch
does not apply

Patch:      
chrome/browser/resources/settings/internet_page/network_summary_item.html
Index: chrome/browser/resources/settings/internet_page/network_summary_item.html
diff --git
a/chrome/browser/resources/settings/internet_page/network_summary_item.html
b/chrome/browser/resources/settings/internet_page/network_summary_item.html
index
ee3e90f35c2a224f44d4aa392ec469d59d00167b..69e6dff83ae2ac751ff5f0f768fe6b0459f31266
100644
--- a/chrome/browser/resources/settings/internet_page/network_summary_item.html
+++ b/chrome/browser/resources/settings/internet_page/network_summary_item.html
@@ -1,27 +1,18 @@
-<link rel="import"
href="chrome://resources/cr_elements/cr_expand_button/cr_expand_button.html">
 <link rel="import"
href="chrome://resources/cr_elements/network/cr_network_list.html">
 <link rel="import"
href="chrome://resources/cr_elements/network/cr_network_list_item.html">
-<link rel="import" href="chrome://resources/html/action_link.html">
-<link rel="import" href="chrome://resources/html/action_link_css.html">
 <link rel="import" href="chrome://resources/html/i18n_behavior.html">
 <link rel="import" href="chrome://resources/html/polymer.html">
 <link rel="import"
href="chrome://resources/polymer/v1_0/iron-a11y-keys-behavior/iron-a11y-keys-behavior.html">
 <link rel="import"
href="chrome://resources/polymer/v1_0/iron-collapse/iron-collapse.html">
-<link rel="import"
href="chrome://resources/polymer/v1_0/paper-icon-button/paper-icon-button.html">
 <link rel="import"
href="chrome://resources/polymer/v1_0/paper-icon-button/paper-icon-button-light.html">
-<link rel="import"
href="chrome://resources/polymer/v1_0/paper-spinner/paper-spinner.html">
 <link rel="import"
href="chrome://resources/polymer/v1_0/paper-toggle-button/paper-toggle-button.html">
+<link rel="import" href="../settings_page/settings_subpage.html">
 <link rel="import" href="../settings_shared_css.html">
 <link rel="import" href="network_siminfo.html">
 
 <dom-module name="network-summary-item">
   <template>
-    <style include="settings-shared action-link">
-      paper-spinner {
-        @apply(--cr-icon-height-width);
-        margin: 0 var(--settings-box-row-padding);
-      }
-
+    <style include="settings-shared">
       network-siminfo {
         padding: 0 var(--settings-box-row-padding);
       }
@@ -30,10 +21,6 @@
         @apply(--settings-list-frame-padding);
       }
 
-      #buttons {
-        align-items: center;
-      }
-
       #details {
         align-items: center;
         display: flex;
@@ -44,24 +31,17 @@
         flex: none;
       }
 
-      #deviceEnabledButton {
-        -webkit-margin-start: 10px;
-      }
-
       #networkList {
         margin: 5px 0 10px;
         max-height: 200px;
         min-height: 50px;
       }
     </style>
-    <div class="settings-box two-line" actionable on-tap="onDetailsTap_">
+    <div actionable class="settings-box two-line" on-tap="onShowDetailsTap_">
       <div id="details"
           no-flex$="[[showSimInfo_(deviceState)]]">
         <cr-network-list-item item="[[activeNetworkState]]" class="flex">
         </cr-network-list-item>
-        <paper-spinner active="[[scanningIsActive_(deviceState, expanded_)]]"
-            hidden$="[[!scanningIsVisible_(deviceState)]]">
-        </paper-spinner>
       </div>
 
       <template is="dom-if" if="[[showSimInfo_(deviceState)]]">
@@ -71,54 +51,24 @@
         </network-siminfo>
       </template>
 
-      <div id="buttons" class="layout horizontal">
-        <template is="dom-if"
-            if="[[expandIsVisible_(deviceState, networkStateList)]]">
-          <cr-expand-button id="expandListButton" expanded="{{expanded_}}">
-          </cr-expand-button>
-        </template>
-        <template is="dom-if"
-            if="[[showDetailsIsVisible_(deviceState, networkStateList)]]">
-          <!-- TODO(dbeam): this needs an aria-label. Maybe
-               [[activeNetworkState.name]]? -->
-          <button class="subpage-arrow" is="paper-icon-button-light"
-              on-tap="onShowDetailsTap_">
-          </button>
-        </template>
-      </div>
+      <template is="dom-if" if="[[showDetailsIsVisible_(deviceState)]]">
+        <button class="subpage-arrow" is="paper-icon-button-light"
+            aria-label="[[getDetailsA11yString_(activeNetworkState,
+                deviceSate)]]">
+        </button>
+      </template>
 
       <template is="dom-if" if="[[enableToggleIsVisible_(deviceState)]]">
         <div class="secondary-action">
           <paper-toggle-button  id="deviceEnabledButton"
-              aria-label$="[[getToggleA11yString_(deviceState)]]"
+              aria-label="[[getToggleA11yString_(deviceState)]]"
               checked="[[deviceIsEnabled_(deviceState)]]"
               enabled="[[enableToggleIsEnabled_(deviceState)]]"
               on-tap="onDeviceEnabledTap_">
           </paper-toggle-button>
         </div>
       </template>
-
     </div>
-    <template is="dom-if"
-        if="[[networksDomIfIsTrue_(expanded_, wasExpanded_)]]">
-      <iron-collapse
-          opened="[[networksIronCollapseIsOpened_(expanded_, wasExpanded_)]]">
-        <div class="layout vertical padded">
-          <cr-network-list id="networkList" networks="[[networkStateList]]"
-              show-buttons>
-          </cr-network-list>
-          <template is="dom-if"
-              if="[[knownNetworksIsVisible_(activeNetworkState)]]">
-            <div class="list-item">
-              <a is="action-link" class="list-button"
-                  on-tap="onKnownNetworksTap_">
-                $i18n{knownNetworksButton}
-              </a>
-            </div>
-          </template>
-        </div>
-      </iron-collapse>
-    </template>
   </template>
   <script src="network_summary_item.js"></script>
 </dom-module>

[stevenjb, 2017-03-02 18:55:44]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2017-03-02 18:55:44]

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/2720503006/#ps80001
(title: "Rebase")

[commit-bot: I haz the power, 2017-03-02 18:56:19]

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-03-02 20:47:05]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1488480944128170,
"parent_rev": "374b2669b0af7cd88882cff28555f5aaa04c52d7", "commit_rev":
"945290e9adf66a1cc7cd5a1fd089fe3ae3400a8b"}

[commit-bot: I haz the power, 2017-03-02 20:47:46]

Description was changed from

==========
MD Settings: Internet: Move network lists to a subpage

This CL:
* Moves the lists of networks and the associated logic
from network-summary-item to internet-subpage.
* Changes the network-summary listener pattern to match other usage.
* Adds missing aria-label for buttons and toggles.
* Fixes a bug in cr-network-icon causing it to show the security badge for
disconnected summary items.
* Adds a separator bar before the subpage button in cr-network-list-item.


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

to

==========
MD Settings: Internet: Move network lists to a subpage

This CL:
* Moves the lists of networks and the associated logic
from network-summary-item to internet-subpage.
* Changes the network-summary listener pattern to match other usage.
* Adds missing aria-label for buttons and toggles.
* Fixes a bug in cr-network-icon causing it to show the security badge for
disconnected summary items.
* Adds a separator bar before the subpage button in cr-network-list-item.


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

Review-Url: https://codereview.chromium.org/2720503006
Cr-Commit-Position: refs/heads/master@{#454368}
Committed:
https://chromium.googlesource.com/chromium/src/+/945290e9adf66a1cc7cd5a1fd089...
==========

[commit-bot: I haz the power, 2017-03-02 20:47:48]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/945290e9adf66a1cc7cd5a1fd089...

[chanli1, 2017-03-03 17:43:56]

On 2017/03/02 20:47:48, commit-bot: I haz the power wrote:
> Committed patchset #5 (id:80001) as
>
https://chromium.googlesource.com/chromium/src/+/945290e9adf66a1cc7cd5a1fd089...

Hello,

Based on Findit result on
https://findit-for-me.appspot.com/waterfall/failure?url=https://luci-milo.app...,
it looks like this change may have caused
CrSettingsInternetPageTest.InternetPageTest in browser_tests to be flaky. Could
you kindly check if that's true?

Thank you!
Chan Li

[stevenjb, 2017-03-03 18:31:06]

On 2017/03/03 17:43:56, chanli1 wrote:
> On 2017/03/02 20:47:48, commit-bot: I haz the power wrote:
> > Committed patchset #5 (id:80001) as
> >
>
https://chromium.googlesource.com/chromium/src/+/945290e9adf66a1cc7cd5a1fd089...
> 
> Hello,
> 
> Based on Findit result on
>
https://findit-for-me.appspot.com/waterfall/failure?url=https://luci-milo.app...,
> it looks like this change may have caused
> CrSettingsInternetPageTest.InternetPageTest in browser_tests to be flaky.
Could
> you kindly check if that's true?
> 
> Thank you!
> Chan Li

See crbug.com/698109 and https://codereview.chromium.org/2727643010/ :)