chromeos: Fix crash when clicking on networking menu under mash

chromeos: Fix crash when clicking on networking menu under mash

The crash occurs because chrome --mash doesn't have a
chromeos::UIProxyConfigService. For classic ash this is usually
created by chrome browser during startup. This object is using in
ash to display a "your connection may be monitored" warning when
using a network proxy. It has dependencies on the user profile
pref service and the local state prefs, so we can't create one
for mash yet.

As a temporary measure, add a check for mash and skip the warning. This
can be fixed properly when the mojo prefs service supports local state.

Also add tests for TrayNetwork. This requires making it optional to
initialize chromeos::NetworkHandler and DBusThreadManager in
mash_unittests.

BUG=717645, 718072
TEST=ash_unittests, also chrome --mash doesn't crash when opening
system tray networking menu

Review-Url: https://codereview.chromium.org/2861773003
Cr-Commit-Position: refs/heads/master@{#469202}
Committed: https://chromium.googlesource.com/chromium/src/+/0249478f91c38f2c29642c4717eafcdc62e65bfd

[James Cook, 2017-05-03 21:14:05]

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

[commit-bot: I haz the power, 2017-05-03 21:14:41]

Dry run: CQ is trying da patch.

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

[James Cook, 2017-05-03 21:15:04]

Description was changed from

==========
chromeos: Fix crash when clicking on networking menu under mash

Mash doesn't have a preferences service available for "local state".
It can't create the chromeos::UIProxyConfigService used by the system
tray menu to display the "your connection may be monitored" warning.
This causes a crash when building the networking details view in the menu.

As a temporary measure, add a check for mash and skip the warning. This
can be fixed properly when the mojo prefs service supports local state.

Also add tests for TrayNetwork.

BUG=717645,718072
TEST=ash_unittests, also chrome --mash doesn't crash when opening
system tray networking menu
==========

to

==========
chromeos: Fix crash when clicking on networking menu under mash

Mash doesn't have a preferences service available for "local state".
It can't create the chromeos::UIProxyConfigService used by the system
tray menu to display the "your connection may be monitored" warning.
This causes a crash when building the networking details view in the menu.

As a temporary measure, add a check for mash and skip the warning. This
can be fixed properly when the mojo prefs service supports local state.

Also add tests for TrayNetwork. This requires making it optional to
initialize chromeos::NetworkHandler and DBusThreadManager in
mash_unittests.

BUG=717645,718072
TEST=ash_unittests, also chrome --mash doesn't crash when opening
system tray networking menu
==========

[commit-bot: I haz the power, 2017-05-03 21:22:16]

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

[commit-bot: I haz the power, 2017-05-03 21:22:17]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[James Cook, 2017-05-03 22:05:05]

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

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

Dry run: CQ is trying da patch.

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

[James Cook, 2017-05-03 22:05:41]

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

[James Cook, 2017-05-03 22:05:42]

stevenjb, please take a look.

sammiequon, just FYI

[James Cook, 2017-05-03 22:08:46]

Description was changed from

==========
chromeos: Fix crash when clicking on networking menu under mash

Mash doesn't have a preferences service available for "local state".
It can't create the chromeos::UIProxyConfigService used by the system
tray menu to display the "your connection may be monitored" warning.
This causes a crash when building the networking details view in the menu.

As a temporary measure, add a check for mash and skip the warning. This
can be fixed properly when the mojo prefs service supports local state.

Also add tests for TrayNetwork. This requires making it optional to
initialize chromeos::NetworkHandler and DBusThreadManager in
mash_unittests.

BUG=717645,718072
TEST=ash_unittests, also chrome --mash doesn't crash when opening
system tray networking menu
==========

to

==========
chromeos: Fix crash when clicking on networking menu under mash

The crash occurs because chrome --mash doesn't have a
chromeos::UIProxyConfigService. For classic ash this is usually
created by chrome browser during startup. This object is using in
ash to display a "your connection may be monitored" warning when
using a network proxy. It has dependencies on the user profile
pref service and the local state prefs, so we can't create one
for mash yet.

As a temporary measure, add a check for mash and skip the warning. This
can be fixed properly when the mojo prefs service supports local state.

Also add tests for TrayNetwork. This requires making it optional to
initialize chromeos::NetworkHandler and DBusThreadManager in
mash_unittests.

BUG=717645,718072
TEST=ash_unittests, also chrome --mash doesn't crash when opening
system tray networking menu
==========

[stevenjb, 2017-05-03 22:11:40]

lgtm w/ suggestions

https://codereview.chromium.org/2861773003/diff/40001/ash/mus/window_manager_...
File ash/mus/window_manager_application.cc (right):

https://codereview.chromium.org/2861773003/diff/40001/ash/mus/window_manager_...
ash/mus/window_manager_application.cc:110:
chromeos::NetworkHandler::Initialize();
Should we also check chromeos::NetworkHandler::IsInitialized() here just for
robustness?

https://codereview.chromium.org/2861773003/diff/40001/ash/system/network/netw...
File ash/system/network/network_list.cc (right):

https://codereview.chromium.org/2861773003/diff/40001/ash/system/network/netw...
ash/system/network/network_list.cc:539: ->ui_proxy_config_service()
We could just test ui_proxy_config_service() and then when it gets added this
would Just Work.

https://codereview.chromium.org/2861773003/diff/40001/ash/system/network/tray...
File ash/system/network/tray_network_unittest.cc (right):

https://codereview.chromium.org/2861773003/diff/40001/ash/system/network/tray...
ash/system/network/tray_network_unittest.cc:22: class TrayNetworkTest : public
test::AshTestBase {
Tests, hooray!

[James Cook, 2017-05-03 22:29:33]

Thanks for the quick review.

https://codereview.chromium.org/2861773003/diff/40001/ash/mus/window_manager_...
File ash/mus/window_manager_application.cc (right):

https://codereview.chromium.org/2861773003/diff/40001/ash/mus/window_manager_...
ash/mus/window_manager_application.cc:110:
chromeos::NetworkHandler::Initialize();
On 2017/05/03 22:11:39, stevenjb wrote:
> Should we also check chromeos::NetworkHandler::IsInitialized() here just for
> robustness?

Done.

https://codereview.chromium.org/2861773003/diff/40001/ash/system/network/netw...
File ash/system/network/network_list.cc (right):

https://codereview.chromium.org/2861773003/diff/40001/ash/system/network/netw...
ash/system/network/network_list.cc:539: ->ui_proxy_config_service()
On 2017/05/03 22:11:39, stevenjb wrote:
> We could just test ui_proxy_config_service() and then when it gets added this
> would Just Work.

Done.

[James Cook, 2017-05-03 22:29:36]

The CQ bit was checked by jamescook@chromium.org

[James Cook, 2017-05-03 22:29:37]

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/2861773003/#ps60001
(title: "review comments")

[commit-bot: I haz the power, 2017-05-03 22:30:04]

CQ is trying da patch.

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

[sammiequon, 2017-05-03 22:41:32]

Thanks for fixing this!

[commit-bot: I haz the power, 2017-05-03 23:49:15]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493850576670400,
"parent_rev": "dbeb857581b785c74788a00a7db375a7f1f5134a", "commit_rev":
"0249478f91c38f2c29642c4717eafcdc62e65bfd"}

[commit-bot: I haz the power, 2017-05-03 23:49:28]

Description was changed from

==========
chromeos: Fix crash when clicking on networking menu under mash

The crash occurs because chrome --mash doesn't have a
chromeos::UIProxyConfigService. For classic ash this is usually
created by chrome browser during startup. This object is using in
ash to display a "your connection may be monitored" warning when
using a network proxy. It has dependencies on the user profile
pref service and the local state prefs, so we can't create one
for mash yet.

As a temporary measure, add a check for mash and skip the warning. This
can be fixed properly when the mojo prefs service supports local state.

Also add tests for TrayNetwork. This requires making it optional to
initialize chromeos::NetworkHandler and DBusThreadManager in
mash_unittests.

BUG=717645,718072
TEST=ash_unittests, also chrome --mash doesn't crash when opening
system tray networking menu
==========

to

==========
chromeos: Fix crash when clicking on networking menu under mash

The crash occurs because chrome --mash doesn't have a
chromeos::UIProxyConfigService. For classic ash this is usually
created by chrome browser during startup. This object is using in
ash to display a "your connection may be monitored" warning when
using a network proxy. It has dependencies on the user profile
pref service and the local state prefs, so we can't create one
for mash yet.

As a temporary measure, add a check for mash and skip the warning. This
can be fixed properly when the mojo prefs service supports local state.

Also add tests for TrayNetwork. This requires making it optional to
initialize chromeos::NetworkHandler and DBusThreadManager in
mash_unittests.

BUG=717645,718072
TEST=ash_unittests, also chrome --mash doesn't crash when opening
system tray networking menu

Review-Url: https://codereview.chromium.org/2861773003
Cr-Commit-Position: refs/heads/master@{#469202}
Committed:
https://chromium.googlesource.com/chromium/src/+/0249478f91c38f2c29642c4717ea...
==========

[commit-bot: I haz the power, 2017-05-03 23:49:32]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/0249478f91c38f2c29642c4717ea...