New issue
Advanced search Search tips

Issue 644355 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocked on: View detail
issue 862420
issue 923433
issue 923444

Blocking:
issue 629707



Sign in to add a comment

mash dbus: Networking / shill support

Project Member Reported by jamescook@chromium.org, Sep 6 2016

Issue description

shill provides a D-Bus API used by chrome browser. go/dbus-in-mustash

See DeviceClient / IPConfigClient / ManagerClient / ServiceClient / ProfileClient / ThirdPartyVpnDriverClient

Very complex. From steven’s comment in issue 629707:

We already have, effectively, a C++ wrapper in Chrome for networking (Network*Handler). We even have an extension API (networkingPrivate), based on a well considered spec ('ONC'), that covers about 90% of what the NetworkHandler classes provide. Converting the extension API to Mojo should (theoretically) be straightforward.
...
Based on my experience with maintaining ash::SystemTrayDelegate, I would very much prefer to avoid introducing any chrome <-> ash specific delegates for this, even using mojo.

Plan: Leave in chrome and write a mojo interface similar to the networkingPrivate extension API to be used by ash.

(Steven, who other than you is familiar with this code?)

 
Cc: michae...@chromium.org
Sadly, not many. Michael has some familiarity with it.

Also, FWIW, I think it might be valuable to create a .idl -> .mojom converter which would simplify the effort since we already have a well spec'd out networking_private.idl.

We already have python tools to generate JS externs or interfaces from an IDL:
tools/json_schema_compiler/compiler.py -g externs (or -g interface)



Project Member

Comment 3 by bugdroid1@chromium.org, Sep 29 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/40496aad11135e1b863823467e62c4bd956128ae

commit 40496aad11135e1b863823467e62c4bd956128ae
Author: jamescook <jamescook@chromium.org>
Date: Thu Sep 29 18:38:47 2016

mustash: Show networking items in ash system tray

* Initialize chromeos::NetworkHandler during startup
* Include //ui/chromeos strings and assets in the mash target
* Add VPNDelegateMus to SystemTrayDelegateMus because the system tray expects
a non-null VPN delegate during construction
* Add a StubNetworkingConfigDelegate to SystemTrayDelegateMus. This is just
a stub impl because I think the functionality will be rolled into an ash
network information cache in the future.

TODO: Sort out which pieces of NetworkHandler state are controlled by ash vs
browser

BUG= 647412 ,644355
TEST=manual, chrome --mash shows networking items

Review-Url: https://codereview.chromium.org/2378193002
Cr-Commit-Position: refs/heads/master@{#421881}

[modify] https://crrev.com/40496aad11135e1b863823467e62c4bd956128ae/ash/common/system/networking_config_delegate.h
[modify] https://crrev.com/40496aad11135e1b863823467e62c4bd956128ae/ash/mus/BUILD.gn
[modify] https://crrev.com/40496aad11135e1b863823467e62c4bd956128ae/ash/mus/shell_delegate_mus.cc
[modify] https://crrev.com/40496aad11135e1b863823467e62c4bd956128ae/ash/mus/system_tray_delegate_mus.cc
[modify] https://crrev.com/40496aad11135e1b863823467e62c4bd956128ae/ash/mus/system_tray_delegate_mus.h
[add] https://crrev.com/40496aad11135e1b863823467e62c4bd956128ae/ash/mus/vpn_delegate_mus.cc
[add] https://crrev.com/40496aad11135e1b863823467e62c4bd956128ae/ash/mus/vpn_delegate_mus.h
[modify] https://crrev.com/40496aad11135e1b863823467e62c4bd956128ae/ash/mus/window_manager_application.cc

Labels: Proj-Mustash
Components: Internals>MUS
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0f9528ae987e59f9fc4c89b96d607dd441520b40

commit 0f9528ae987e59f9fc4c89b96d607dd441520b40
Author: jamescook <jamescook@chromium.org>
Date: Wed Oct 05 00:37:39 2016

mustash: Fix crash when clicking on network in system tray

* Initialize ui::NetworkConnect during mash startup
* Provide a stub NetworkConnect::Delegate implementation

It still won't actually switch networks on the device (there are issues
talking to shill in lower-level code) but it's a step in the right direction.

BUG=644355
TEST=manual, click on network in system tray, no crash

Review-Url: https://codereview.chromium.org/2394503003
Cr-Commit-Position: refs/heads/master@{#423021}

[modify] https://crrev.com/0f9528ae987e59f9fc4c89b96d607dd441520b40/ash/mus/BUILD.gn
[modify] https://crrev.com/0f9528ae987e59f9fc4c89b96d607dd441520b40/ash/mus/window_manager_application.cc
[modify] https://crrev.com/0f9528ae987e59f9fc4c89b96d607dd441520b40/ash/mus/window_manager_application.h

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5374fbd4595be5190f3b9f93bb5aaafbdaef426f

commit 5374fbd4595be5190f3b9f93bb5aaafbdaef426f
Author: jamescook <jamescook@chromium.org>
Date: Sat Oct 15 00:15:33 2016

chromeos: Refactor system tray Wi-Fi connect dialog so it works with mash

* Add ShowNetworkConnect method to SystemTrayClient mojom interface
* Add NetworkConnectDelegateMus to route calls over that interface
* Show the config dialog

TODO: Dialog is in the wrong parent window in mash, in particular on the lock
screen. Fixing this will require plumbing through src/ui dialog code, which
will happen in a subsequent CL.

BUG=644355
TEST=existing networking tests

Review-Url: https://codereview.chromium.org/2411223002
Cr-Commit-Position: refs/heads/master@{#425515}

[modify] https://crrev.com/5374fbd4595be5190f3b9f93bb5aaafbdaef426f/ash/common/system/tray/system_tray_controller.cc
[modify] https://crrev.com/5374fbd4595be5190f3b9f93bb5aaafbdaef426f/ash/common/system/tray/system_tray_controller.h
[modify] https://crrev.com/5374fbd4595be5190f3b9f93bb5aaafbdaef426f/ash/mus/BUILD.gn
[add] https://crrev.com/5374fbd4595be5190f3b9f93bb5aaafbdaef426f/ash/mus/network_connect_delegate_mus.cc
[add] https://crrev.com/5374fbd4595be5190f3b9f93bb5aaafbdaef426f/ash/mus/network_connect_delegate_mus.h
[modify] https://crrev.com/5374fbd4595be5190f3b9f93bb5aaafbdaef426f/ash/mus/window_manager_application.cc
[modify] https://crrev.com/5374fbd4595be5190f3b9f93bb5aaafbdaef426f/ash/mus/window_manager_application.h
[modify] https://crrev.com/5374fbd4595be5190f3b9f93bb5aaafbdaef426f/ash/public/interfaces/system_tray.mojom
[modify] https://crrev.com/5374fbd4595be5190f3b9f93bb5aaafbdaef426f/chrome/browser/ui/ash/system_tray_client.cc
[modify] https://crrev.com/5374fbd4595be5190f3b9f93bb5aaafbdaef426f/chrome/browser/ui/ash/system_tray_client.h

Components: -Internals>MUS Internals>Services>WindowService
Components: -MUS
Components: -Internals>Services>WindowService Internals>Services>Ash
Labels: -Proj-Mustash-Mash
Cc: -steve...@chromium.org
Labels: -Proj-Mustash Proj-Mash-SingleProcess
Owner: steve...@chromium.org
Status: Assigned (was: Untriaged)
Steven, is this bug still applicable?
Labels: -Proj-Mash-SingleProcess Proj-Mash-MultiProcess

Comment 13 by steve...@chromium.org, Jan 18 (5 days ago)

Blockedon: 923433

Comment 14 by steve...@chromium.org, Jan 18 (5 days ago)

Blockedon: 862420

Comment 15 by steve...@chromium.org, Jan 18 (5 days ago)

Blockedon: 923444

Sign in to add a comment