Notify ARC VPN on physical connection changes |
|||||||
Issue descriptionIf the system is connected to an ARC VPN and the underlying physical connection (ethernet, wifi, cellular) changes state, we do not currently notify Android. This is because the ARC VPN is the default service, so ArcNetHostImpl::DefaultNetworkChanged will not get invoked if the *underlying* service changes. It would be good if we notified Android of these connection changes, because then the Android VPN app can make more informed decisions about when to disconnect or reconnect the VPN, rather than allowing a "dead" connection to retry and time out. (It might make sense to roll this into the multinetwork project rather than trying to fix the code we have today.)
,
Sep 22 2017
The observer method you want to listen to instead of DefaultNetworkChanged() is NetwworkStateHandlerObserver::NetworkConnectionStateChanged(network), then compare |network| to FirstNetworkByTyoe(NonVirtual()).
,
Sep 23 2017
,
Nov 3 2017
,
Nov 7 2017
> NetwworkStateHandlerObserver::NetworkConnectionStateChanged(network), then compare |network| to FirstNetworkByTyoe(NonVirtual()). Hi Steven, this does not seem to be working for me. I am not getting a callback from either NetworkConnectionStateChanged or DefaultNetworkChanged when the physical connection change is complete. I am getting a callback when the existing default physical connection goes offline, but not when the new one goes online. Is there a different callback that will help here? My WIP CL: https://chromium-review.googlesource.com/#/c/chromium/src/+/757679
,
Nov 7 2017
That sounds like a bug, can you maybe investigate, look at chrome://device-log? The code that should be triggering the observer is here: https://cs.chromium.org/chromium/src/chromeos/network/network_state_handler.cc?type=cs&q=NetworkConnectionStateChanged&sq=package:chromium&l=1604
,
Nov 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/583cd55a4300e4ea50356f7921b3c62acf2056e7 commit 583cd55a4300e4ea50356f7921b3c62acf2056e7 Author: Kevin Cernekee <cernekee@chromium.org> Date: Sat Nov 18 02:04:23 2017 Fix ConnectedNetworkByType() results during state transitions If wifi and ethernet are both connected, and then the ethernet adapter is unplugged, network_list_ can end up looking like: network_list_[0] -> ethernet, idle network_list_[1] -> wifi, online shill will generally update the service's State property before it updates the manager's Services / ServiceCompleteList properties, leaving the now-idle service in the list. Since ConnectedNetworkByType() assumes that connected networks are listed first, it won't return the newly-active wifi network. Fix this by sorting the network list, and changing the sort function so that it doesn't prioritize idle ethernet services. BUG=768053 TEST=manually verify order with debug prints TEST=unit tests Change-Id: I3a110da7d99bd5e6463b86e17bc557124e875b85 Reviewed-on: https://chromium-review.googlesource.com/777970 Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Commit-Queue: Kevin Cernekee <cernekee@chromium.org> Cr-Commit-Position: refs/heads/master@{#517679} [modify] https://crrev.com/583cd55a4300e4ea50356f7921b3c62acf2056e7/chromeos/network/network_state_handler.cc [modify] https://crrev.com/583cd55a4300e4ea50356f7921b3c62acf2056e7/chromeos/network/network_state_handler.h
,
Nov 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9c18d1f8cfa4855034c6655e406938b804e8548d commit 9c18d1f8cfa4855034c6655e406938b804e8548d Author: Kevin Cernekee <cernekee@chromium.org> Date: Sat Nov 25 00:54:40 2017 Notify ARC of physical connection changes when ARC VPN is up If ARC VPN is up, Chrome should send notifications whenever the underlying physical network connection (wifi, ethernet, cellular) changes so that the VPN app can trigger a reconnection. BUG=768053 TEST=manually connect/disconnect from VPN, ethernet, wifi and observe logs Change-Id: I64ad4955c96d2eb2a66a9cc2bf12bb82b69c2418 Reviewed-on: https://chromium-review.googlesource.com/757679 Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Elijah Taylor <elijahtaylor@chromium.org> Commit-Queue: Kevin Cernekee <cernekee@chromium.org> Cr-Commit-Position: refs/heads/master@{#519172} [modify] https://crrev.com/9c18d1f8cfa4855034c6655e406938b804e8548d/components/arc/net/arc_net_host_impl.cc [modify] https://crrev.com/9c18d1f8cfa4855034c6655e406938b804e8548d/components/arc/net/arc_net_host_impl.h
,
Dec 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/871e1bd910a31bb7cbdb831c08d4344f5da1c6b4 commit 871e1bd910a31bb7cbdb831c08d4344f5da1c6b4 Author: Kevin Cernekee <cernekee@chromium.org> Date: Fri Dec 01 23:32:48 2017 Disconnect ARC VPN if ARC++ is shut down Currently, if ARC++ is uninstalled while an Android VPN is running, shill will not receive a disconnection event and this will break networking on Chrome OS. Fix this by forcing ARC VPN to disconnect whenever ArcNetHostImpl shuts down. BUG=768053 TEST=uninstall ARC++ through Settings with a VPN connected Change-Id: Iccbdc20bb26bce154f8041a517b1220519a2b738 Reviewed-on: https://chromium-review.googlesource.com/778340 Commit-Queue: Kevin Cernekee <cernekee@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Elijah Taylor <elijahtaylor@chromium.org> Cr-Commit-Position: refs/heads/master@{#521121} [modify] https://crrev.com/871e1bd910a31bb7cbdb831c08d4344f5da1c6b4/components/arc/net/arc_net_host_impl.cc
,
Oct 29
,
Oct 29
,
Oct 29
,
Jan 11
This issue has an owner, a component and a priority, but is still listed as untriaged or unconfirmed. By definition, this bug is triaged. Changing status to "assigned". Please reach out to me if you disagree with how I've done this. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by cernekee@chromium.org
, Sep 22 2017