Disable wifi scans on the VPN tray detailed view

Disable wifi scans on the VPN tray detailed view

Only scan for wifi networks when the user is trying to pick a wifi
network.

BUG=587301
Committed: https://crrev.com/21a8d8acf90b3d9bc19f9b7a042c8d88fff1a55f
Cr-Commit-Position: refs/heads/master@{#376027}

[Kevin Cernekee, 2016-02-17 06:17:15]

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

[stevenjb, 2016-02-17 16:22:52]

Thanks for noticing this. Just one change request.

https://codereview.chromium.org/1704803002/diff/1/ash/system/chromeos/network...
File ash/system/chromeos/network/network_state_list_detailed_view.cc (right):

https://codereview.chromium.org/1704803002/diff/1/ash/system/chromeos/network...
ash/system/chromeos/network/network_state_list_detailed_view.cc:564: if
(scanning && list_type_ != LIST_TYPE_VPN) {
we should wrap this entire section in if (list_type_ != VPN) and also prevent
creating the throbber.

[Kevin Cernekee, 2016-02-17 17:31:17]

https://codereview.chromium.org/1704803002/diff/1/ash/system/chromeos/network...
File ash/system/chromeos/network/network_state_list_detailed_view.cc (right):

https://codereview.chromium.org/1704803002/diff/1/ash/system/chromeos/network...
ash/system/chromeos/network/network_state_list_detailed_view.cc:564: if
(scanning && list_type_ != LIST_TYPE_VPN) {
On 2016/02/17 16:22:52, stevenjb wrote:
> we should wrap this entire section in if (list_type_ != VPN) and also prevent
> creating the throbber.

AIUI, if we don't create the throbber, we won't get the event asking us to show
the IP info when the user clicks on the info button.

(That feature doesn't seem to work correctly right now, but I'd like for it to
show the VPN IPs.)

[stevenjb, 2016-02-17 18:14:14]

Hmm. It looks like 'ThrobberPressed' should be deprecated / removed, and
NSLDV::ButtonPressed should handle the button event even if we don't create
the throbber. Either way we should fix the code so that scanning_throbber_
is not required for the info icon to work.



On Wed, Feb 17, 2016 at 10:31 AM, <cernekee@chromium.org> wrote:

>
>
https://codereview.chromium.org/1704803002/diff/1/ash/system/chromeos/network...
> File ash/system/chromeos/network/network_state_list_detailed_view.cc
> (right):
>
https://codereview.chromium.org/1704803002/diff/1/ash/system/chromeos/network...
> ash/system/chromeos/network/network_state_list_detailed_view.cc:564
<https://codereview.chromium.org/1704803002/diff/1/ash/system/chromeos/network...>:
if
> (scanning && list_type_ != LIST_TYPE_VPN) {
> On 2016/02/17 16:22:52, stevenjb wrote:
> > we should wrap this entire section in if (list_type_ != VPN) and also
> prevent
> > creating the throbber.
>
> AIUI, if we don't create the throbber, we won't get the event asking us
> to show the IP info when the user clicks on the info button.
>
> (That feature doesn't seem to work correctly right now, but I'd like for
> it to show the VPN IPs.)
> https://codereview.chromium.org/1704803002/
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[cernekee, 2016-02-17 19:24:34]

cernekee@google.com changed reviewers:
+ cernekee@google.com

[cernekee, 2016-02-17 19:24:35]

> It looks like 'ThrobberPressed' should be deprecated / removed

Upon further inspection I couldn't find anything that actually called this
method.  Removed.

> Either way we should fix the code so that 
> scanning_throbber_ is not required for the info icon
> to work.

Done

[stevenjb, 2016-02-17 23:02:26]

Thanks!
LGTM

[cernekee, 2016-02-17 23:18:57]

The CQ bit was checked by cernekee@google.com

[commit-bot: I haz the power, 2016-02-17 23:19:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1704803002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1704803002/20001

[commit-bot: I haz the power, 2016-02-17 23:27:31]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-17 23:28:50]

Description was changed from

==========
Disable wifi scans on the VPN tray detailed view

Only scan for wifi networks when the user is trying to pick a wifi
network.

BUG=587301
==========

to

==========
Disable wifi scans on the VPN tray detailed view

Only scan for wifi networks when the user is trying to pick a wifi
network.

BUG=587301

Committed: https://crrev.com/21a8d8acf90b3d9bc19f9b7a042c8d88fff1a55f
Cr-Commit-Position: refs/heads/master@{#376027}
==========

[commit-bot: I haz the power, 2016-02-17 23:28:51]

Patchset 2 (id:??) landed as
https://crrev.com/21a8d8acf90b3d9bc19f9b7a042c8d88fff1a55f
Cr-Commit-Position: refs/heads/master@{#376027}