Convert path usage in NetworkListViewBase subclasses to use guids.

Convert path usage in NetworkListViewBase subclasses to use guids.

BUG=457855
Review-Url: https://codereview.chromium.org/2698473007
Cr-Commit-Position: refs/heads/master@{#451499}
Committed: https://chromium.googlesource.com/chromium/src/+/ee86b6c003b66c41ad820151cbeda650920fbe76

[Ryan Hansberry, 2017-02-16 21:48:31]

Description was changed from

==========
Convert path usage in NetworkListViewBase subclasses to use guids.

BUG=
==========

to

==========
Convert path usage in NetworkListViewBase subclasses to use guids.

BUG=
==========

[Ryan Hansberry, 2017-02-16 21:48:31]

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

[Ryan Hansberry, 2017-02-16 22:18:05]

Tested this change manually and it works, but there don't seem to be any unit
tests?

[Ryan Hansberry, 2017-02-16 22:18:48]

Let me know if there's a bug I should associate this change with.

[stevenjb, 2017-02-16 22:25:33]

On 2017/02/16 22:18:48, Ryan Hansberry wrote:
> Let me know if there's a bug I should associate this change with.

457855

[stevenjb, 2017-02-16 22:40:32]

Please also grep for 'service_path', e.g. SystemTrayNotifier still references it
in NotifyOnCaptivePortalDetected (I believe that it is ignored, but we should
still change the interface to use guid).

https://codereview.chromium.org/2698473007/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/network/network_list.cc (right):

https://codereview.chromium.org/2698473007/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/network/network_list.cc:121:
handler->GetNetworkState(info->guid);
Soo, this is why this change is dangerous and I partly why I haven't done it.
This needs to be GetNetworkStateFromGuid.

https://codereview.chromium.org/2698473007/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/network/network_state_list_detailed_view.cc
(right):

https://codereview.chromium.org/2698473007/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/network/network_state_list_detailed_view.cc:647:
networking_config_delegate->LookUpExtensionForNetwork(info.guid);
This delegate function needs to be modified to accept guid. I think this is the
only place it is called from though.

[Ryan Hansberry, 2017-02-16 23:14:18]

Grepped across src/ash/ and changed all service_path vars to guid. Also made
sure to change GetNetworkState calls to GetNetworkStateFromGuid.

https://codereview.chromium.org/2698473007/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/network/network_list.cc (right):

https://codereview.chromium.org/2698473007/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/network/network_list.cc:121:
handler->GetNetworkState(info->guid);
On 2017/02/16 22:40:31, stevenjb wrote:
> Soo, this is why this change is dangerous and I partly why I haven't done it.
> This needs to be GetNetworkStateFromGuid.

Darn, thought I caught all of these :/ went through all the files again and I'm
pretty sure I caught them all.

https://codereview.chromium.org/2698473007/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/network/network_state_list_detailed_view.cc
(right):

https://codereview.chromium.org/2698473007/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/network/network_state_list_detailed_view.cc:647:
networking_config_delegate->LookUpExtensionForNetwork(info.guid);
On 2017/02/16 22:40:31, stevenjb wrote:
> This delegate function needs to be modified to accept guid. I think this is
the
> only place it is called from though.

Thankfully this was the only place that calls this. :)

[stevenjb, 2017-02-16 23:57:48]

Thanks! LGTM

https://codereview.chromium.org/2698473007/diff/40001/ash/common/system/chrom...
File ash/common/system/chromeos/network/network_portal_detector_observer.h
(right):

https://codereview.chromium.org/2698473007/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/network/network_portal_detector_observer.h:16: //
Called when captive portal is detected for a |network|.
nit: Fix comment

[stevenjb, 2017-02-16 23:58:23]

On 2017/02/16 23:57:48, stevenjb wrote:
> Thanks! LGTM
> 
>
https://codereview.chromium.org/2698473007/diff/40001/ash/common/system/chrom...
> File ash/common/system/chromeos/network/network_portal_detector_observer.h
> (right):
> 
>
https://codereview.chromium.org/2698473007/diff/40001/ash/common/system/chrom...
> ash/common/system/chromeos/network/network_portal_detector_observer.h:16: //
> Called when captive portal is detected for a |network|.
> nit: Fix comment

Please update the description with BUG=457855 before comitting.

[Ryan Hansberry, 2017-02-17 01:41:32]

Description was changed from

==========
Convert path usage in NetworkListViewBase subclasses to use guids.

BUG=
==========

to

==========
Convert path usage in NetworkListViewBase subclasses to use guids.

BUG=457855
==========

[Ryan Hansberry, 2017-02-17 01:43:18]

https://codereview.chromium.org/2698473007/diff/40001/ash/common/system/chrom...
File ash/common/system/chromeos/network/network_portal_detector_observer.h
(right):

https://codereview.chromium.org/2698473007/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/network/network_portal_detector_observer.h:16: //
Called when captive portal is detected for a |network|.
On 2017/02/16 23:57:48, stevenjb wrote:
> nit: Fix comment

Done.

[Ryan Hansberry, 2017-02-17 01:44:48]

The CQ bit was checked by hansberry@chromium.org

[Ryan Hansberry, 2017-02-17 01:44:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org
Link to the patchset: https://codereview.chromium.org/2698473007/#ps60001
(title: "Fix a comment.")

[commit-bot: I haz the power, 2017-02-17 01:45:44]

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-17 03:48:02]

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

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

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)

[Ryan Hansberry, 2017-02-17 18:03:53]

hansberry@chromium.org changed reviewers:
+ derat@chromium.org

[Ryan Hansberry, 2017-02-17 18:04:17]

derat@, I added you as an owner of ash/mus/system_tray_delegate_mus.

[Daniel Erat, 2017-02-17 18:08:15]

lgtm for ash/mus/

[Ryan Hansberry, 2017-02-17 18:20:32]

The CQ bit was checked by hansberry@chromium.org

[commit-bot: I haz the power, 2017-02-17 18:21:13]

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-17 19:50:57]

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

[commit-bot: I haz the power, 2017-02-17 19:50:58]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ryan Hansberry, 2017-02-17 22:52:12]

The CQ bit was checked by hansberry@chromium.org

[Ryan Hansberry, 2017-02-17 22:52:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, derat@chromium.org
Link to the patchset: https://codereview.chromium.org/2698473007/#ps80001
(title: "Tweak NetworkPortalNotificationControllerTest to create GUIDs for new
NetworkStates.")

[commit-bot: I haz the power, 2017-02-17 22:54:33]

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-18 00:58:25]

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

[commit-bot: I haz the power, 2017-02-18 00:58:27]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[Ryan Hansberry, 2017-02-18 01:25:48]

The CQ bit was checked by hansberry@chromium.org

[commit-bot: I haz the power, 2017-02-18 01:27:32]

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-18 03:29:21]

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

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

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

[Kyle Horimoto, 2017-02-18 23:46:06]

The CQ bit was checked by khorimoto@chromium.org

[commit-bot: I haz the power, 2017-02-18 23:46:13]

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-18 23:56:32]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487461566092650,
"parent_rev": "9187e4de3abf2fca18590e91f619b7b9cc0f8f25", "commit_rev":
"ee86b6c003b66c41ad820151cbeda650920fbe76"}

[commit-bot: I haz the power, 2017-02-18 23:57:10]

Description was changed from

==========
Convert path usage in NetworkListViewBase subclasses to use guids.

BUG=457855
==========

to

==========
Convert path usage in NetworkListViewBase subclasses to use guids.

BUG=457855

Review-Url: https://codereview.chromium.org/2698473007
Cr-Commit-Position: refs/heads/master@{#451499}
Committed:
https://chromium.googlesource.com/chromium/src/+/ee86b6c003b66c41ad820151cbed...
==========

[commit-bot: I haz the power, 2017-02-18 23:57:11]

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