New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 698943 link

Starred by 1 user
Status: Fixed
Owner:
btolsch@chromium.org
Closed: Nov 2017
Cc:
█ zhaobin@chromium.org
mfo...@chromium.org
taku...@chromium.org
powerb@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

[Sink discovery] Implement network API to monitor network connectivity changes

Project Member Reported by imch...@chromium.org, Mar 7 2017 
Implement a browser-side API with the following purposes:
- Notify clients when the set of connected networks has changed
- Identify the set of networks that machine is currently connected to

This API should abstract away any details that are platform-specific whenever possible. The goal is to provide a consistent networking API that can be used by both DIAL (which is used to invalidate the current sink list if machine is disconnected) and Cast MediaSinkServices (which is used to perform sink list caching accoriding to the set of connected networks).

This is what the extension code currently uses:

DIAL: NetworkChangeNotifier: https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/dial/dial_registry.cc?type=cs&sq=package:chromium&l=326 to fire an chrome.dial.onError event.
Cast: chrome.networkingPrivate.onNetworksChanged (onNetworkListChanged on ChromeOS), and chrome.networkingPrivate.getNetworks: https://cs.chromium.org/chromium/src/extensions/common/api/networking_private.idl?type=cs&q=f:networking_private.idl&sq=package:chromium&l=1

Note: there's already some work started on this: https://codereview.chromium.org/2602593002/. We can pick things up from there, with some renaming so it can be shared across DIAL and Cast MediaSinkServices (and possibly others).
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 6 2017 

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

commit 2bf972dc59af2654f71f577122e39fed4c0a986e
Author: btolsch <btolsch@chromium.org>
Date: Tue Jun 06 19:48:19 2017

Add DiscoveryNetworkMonitor implementation

This changes adds a singleton class DiscoveryNetworkMonitor which
provides general information to services doing device discovery on the
local network.

BUG= 698943 

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

[modify] https://crrev.com/2bf972dc59af2654f71f577122e39fed4c0a986e/chrome/browser/media/router/discovery/BUILD.gn
[add] https://crrev.com/2bf972dc59af2654f71f577122e39fed4c0a986e/chrome/browser/media/router/discovery/discovery_network_info.cc
[add] https://crrev.com/2bf972dc59af2654f71f577122e39fed4c0a986e/chrome/browser/media/router/discovery/discovery_network_info.h
[add] https://crrev.com/2bf972dc59af2654f71f577122e39fed4c0a986e/chrome/browser/media/router/discovery/discovery_network_list.h
[add] https://crrev.com/2bf972dc59af2654f71f577122e39fed4c0a986e/chrome/browser/media/router/discovery/discovery_network_list_posix.cc
[add] https://crrev.com/2bf972dc59af2654f71f577122e39fed4c0a986e/chrome/browser/media/router/discovery/discovery_network_list_unittest.cc
[add] https://crrev.com/2bf972dc59af2654f71f577122e39fed4c0a986e/chrome/browser/media/router/discovery/discovery_network_list_wifi.h
[add] https://crrev.com/2bf972dc59af2654f71f577122e39fed4c0a986e/chrome/browser/media/router/discovery/discovery_network_list_wifi_linux.cc
[add] https://crrev.com/2bf972dc59af2654f71f577122e39fed4c0a986e/chrome/browser/media/router/discovery/discovery_network_list_win.cc
[add] https://crrev.com/2bf972dc59af2654f71f577122e39fed4c0a986e/chrome/browser/media/router/discovery/discovery_network_monitor.cc
[add] https://crrev.com/2bf972dc59af2654f71f577122e39fed4c0a986e/chrome/browser/media/router/discovery/discovery_network_monitor.h
[add] https://crrev.com/2bf972dc59af2654f71f577122e39fed4c0a986e/chrome/browser/media/router/discovery/discovery_network_monitor_unittest.cc
[modify] https://crrev.com/2bf972dc59af2654f71f577122e39fed4c0a986e/chrome/test/BUILD.gn
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 7 2017 

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

commit bf4426c77462169c84c817eeae03a9cd7169672a
Author: btolsch <btolsch@chromium.org>
Date: Fri Jul 07 04:16:30 2017

Add Mac implementation of GetDiscoveryNetworkInfoList

This change enables DiscoveryNetworkMonitor to work on Mac.  Some small
syntax changes are made to the _posix.cc file and an implementation for
getting the WiFi SSID associated with an interface is also added.

Bug:  698943 ,  713378 
Change-Id: I1f50bbe7169e5ac017dba9ee24b3f84a24d4aecd
Reviewed-on: https://chromium-review.googlesource.com/527952
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Kevin Marshall <kmarshall@chromium.org>
Commit-Queue: Brandon Tolsch <btolsch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484828}
[modify] https://crrev.com/bf4426c77462169c84c817eeae03a9cd7169672a/chrome/browser/media/router/discovery/BUILD.gn
[modify] https://crrev.com/bf4426c77462169c84c817eeae03a9cd7169672a/chrome/browser/media/router/discovery/discovery_network_list_posix.cc
[add] https://crrev.com/bf4426c77462169c84c817eeae03a9cd7169672a/chrome/browser/media/router/discovery/discovery_network_list_wifi_mac.mm
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 10 2017 

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

commit d0591a302b05e7756eacc356ccd157c52f45e0be
Author: btolsch <btolsch@chromium.org>
Date: Mon Jul 10 22:43:18 2017

Add Windows implementation of GetDiscoveryNetworkInfoList

This change enables DiscoveryNetworkMonitor to work on Windows.  It
supports both MAC addresses and WiFi SSIDs for the network ID.

Bug:  698943 
Change-Id: I0c94743fd51019438fd53472f38a2d83e21a3246
Reviewed-on: https://chromium-review.googlesource.com/527774
Commit-Queue: Brandon Tolsch <btolsch@chromium.org>
Reviewed-by: Kevin Marshall <kmarshall@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485431}
[modify] https://crrev.com/d0591a302b05e7756eacc356ccd157c52f45e0be/chrome/browser/media/router/discovery/BUILD.gn
[modify] https://crrev.com/d0591a302b05e7756eacc356ccd157c52f45e0be/chrome/browser/media/router/discovery/discovery_network_list_win.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 18 2017 

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

commit 2d03d0fdfb8fa2e767926ce78ec6bae49ad1248e
Author: Alexei Filippov <alph@chromium.org>
Date: Tue Jul 18 01:13:01 2017

Revert "Add Windows implementation of GetDiscoveryNetworkInfoList"

This reverts commit d0591a302b05e7756eacc356ccd157c52f45e0be.

Reason for revert: Speculative revert in an attempt to find a culprit of https://bugs.chromium.org/p/chromium/issues/detail?id=741837

Original change's description:
> Add Windows implementation of GetDiscoveryNetworkInfoList
> 
> This change enables DiscoveryNetworkMonitor to work on Windows.  It
> supports both MAC addresses and WiFi SSIDs for the network ID.
> 
> Bug:  698943 
> Change-Id: I0c94743fd51019438fd53472f38a2d83e21a3246
> Reviewed-on: https://chromium-review.googlesource.com/527774
> Commit-Queue: Brandon Tolsch <btolsch@chromium.org>
> Reviewed-by: Kevin Marshall <kmarshall@chromium.org>
> Reviewed-by: Derek Cheng <imcheng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#485431}

TBR=kmarshall@chromium.org,imcheng@chromium.org,btolsch@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  698943 
Change-Id: I99404e9121fc59ce0d52a7049836ec2d2e7e0bd6
Reviewed-on: https://chromium-review.googlesource.com/574614
Commit-Queue: Alexei Filippov <alph@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487331}
[modify] https://crrev.com/2d03d0fdfb8fa2e767926ce78ec6bae49ad1248e/chrome/browser/media/router/discovery/BUILD.gn
[modify] https://crrev.com/2d03d0fdfb8fa2e767926ce78ec6bae49ad1248e/chrome/browser/media/router/discovery/discovery_network_list_win.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 25 2017 

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

commit 8f7a5ea7d66888d0c1d5b73e2a56e9b9de714e5e
Author: btolsch <btolsch@chromium.org>
Date: Tue Jul 25 21:47:01 2017

Reland "Add Windows implementation of GetDiscoveryNetworkInfoList"

This is a reland of d0591a302b05e7756eacc356ccd157c52f45e0be with a
linking fix for wlanapi.dll.

Original change's description:
> This change enables DiscoveryNetworkMonitor to work on Windows.  It
> supports both MAC addresses and WiFi SSIDs for the network ID.
>
> Bug:  698943 
> Change-Id: I0c94743fd51019438fd53472f38a2d83e21a3246
> Reviewed-on: https://chromium-review.googlesource.com/527774
> Commit-Queue: Brandon Tolsch <btolsch@chromium.org>
> Reviewed-by: Kevin Marshall <kmarshall@chromium.org>
> Reviewed-by: Derek Cheng <imcheng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#485431}

Bug:  698943 
Change-Id: I0c65f5af3a06b6fc673d7903cdc769fa9eaf4711
Reviewed-on: https://chromium-review.googlesource.com/576587
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Commit-Queue: Brandon Tolsch <btolsch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489449}
[modify] https://crrev.com/8f7a5ea7d66888d0c1d5b73e2a56e9b9de714e5e/chrome/browser/media/router/discovery/discovery_network_list_win.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 25 2017 

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

commit 58832329a38f29c8517ef5dfa4fbdeec347e41f2
Author: btolsch <btolsch@chromium.org>
Date: Tue Jul 25 22:21:37 2017

Convert DiscoveryNetworkMonitor to use SequencedTaskRunner

This change converts DiscoveryNetworkMonitor's use of the Browser IO
thread to a SequencedTaskRunner.

Bug:  698943 
Change-Id: I4155b154a115abdbd00f8032af930d6fb1b6a28c
Reviewed-on: https://chromium-review.googlesource.com/572665
Commit-Queue: Brandon Tolsch <btolsch@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489461}
[modify] https://crrev.com/58832329a38f29c8517ef5dfa4fbdeec347e41f2/chrome/browser/media/router/discovery/discovery_network_monitor.cc
[modify] https://crrev.com/58832329a38f29c8517ef5dfa4fbdeec347e41f2/chrome/browser/media/router/discovery/discovery_network_monitor.h
[modify] https://crrev.com/58832329a38f29c8517ef5dfa4fbdeec347e41f2/chrome/browser/media/router/discovery/discovery_network_monitor_unittest.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 2 2017 

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

commit 5b3f21e77559b80c30734a09aaa24d616df4b08d
Author: btolsch <btolsch@chromium.org>
Date: Wed Aug 02 23:29:08 2017

Revise DiscoveryNetworkMonitor testing

This change lets other test files avoid using the
DiscoveryNetworkMonitor singleton and converts TestBrowserThreadBundle
to ScopedTaskEnvironment.

Bug:  698943 
Change-Id: Ib69cd42901b739e651a87cafa2ca4a2e214c48e2
Reviewed-on: https://chromium-review.googlesource.com/595017
Commit-Queue: Brandon Tolsch <btolsch@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491550}
[modify] https://crrev.com/5b3f21e77559b80c30734a09aaa24d616df4b08d/chrome/browser/media/router/discovery/discovery_network_monitor.cc
[modify] https://crrev.com/5b3f21e77559b80c30734a09aaa24d616df4b08d/chrome/browser/media/router/discovery/discovery_network_monitor.h
[modify] https://crrev.com/5b3f21e77559b80c30734a09aaa24d616df4b08d/chrome/browser/media/router/discovery/discovery_network_monitor_unittest.cc
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 14 2017 

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

commit 3ea6c558572354c6cb77e7876ff16c3b03fca2c1
Author: btolsch <btolsch@chromium.org>
Date: Mon Aug 14 11:01:12 2017

Add mDNS service caching to CastMediaSinkService

This change uses the DiscoveryNetworkMonitor to remember the list of
services that were last seen on a set of connected
networks.  When a previously-seen set of networks is seen again,
CastMediaSinkService will load all of the cached services and attempt
to resolve them if they are not already present.

Bug:  698943 ,  687377 
Change-Id: Ib3a47ef412ad56e4348f4dbd754cbcc1825d1e41
Reviewed-on: https://chromium-review.googlesource.com/595018
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Commit-Queue: Brandon Tolsch <btolsch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494030}
[modify] https://crrev.com/3ea6c558572354c6cb77e7876ff16c3b03fca2c1/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.cc
[modify] https://crrev.com/3ea6c558572354c6cb77e7876ff16c3b03fca2c1/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.cc
[modify] https://crrev.com/3ea6c558572354c6cb77e7876ff16c3b03fca2c1/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.h
[modify] https://crrev.com/3ea6c558572354c6cb77e7876ff16c3b03fca2c1/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl_unittest.cc
[modify] https://crrev.com/3ea6c558572354c6cb77e7876ff16c3b03fca2c1/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_unittest.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 20 2017 

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

commit 379cf1b202c478e2744dd5add1a9f81665a7c140
Author: btolsch <btolsch@chromium.org>
Date: Wed Sep 20 20:54:27 2017

Add discovery-related network metrics

This change adds metrics for how DiscoveryNetworkMonitor behaves as well
as how well the cache in CastMediaSinkServiceImpl works.

Bug:  698943 ,  687377 
Change-Id: Iaa62fa959066f6fbe3fcce9e67da817c29b25a77
Reviewed-on: https://chromium-review.googlesource.com/666050
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Brandon Tolsch <btolsch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503242}
[modify] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/browser/media/router/discovery/BUILD.gn
[modify] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/browser/media/router/discovery/discovery_network_monitor.cc
[modify] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/browser/media/router/discovery/discovery_network_monitor.h
[add] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/browser/media/router/discovery/discovery_network_monitor_metric_observer.cc
[add] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/browser/media/router/discovery/discovery_network_monitor_metric_observer.h
[add] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/browser/media/router/discovery/discovery_network_monitor_metric_observer_unittest.cc
[add] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/browser/media/router/discovery/discovery_network_monitor_metrics.cc
[add] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/browser/media/router/discovery/discovery_network_monitor_metrics.h
[modify] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.cc
[modify] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.cc
[modify] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.h
[modify] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl_unittest.cc
[modify] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_unittest.cc
[modify] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/browser/media/router/discovery/media_sink_discovery_metrics.cc
[modify] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/browser/media/router/discovery/media_sink_discovery_metrics.h
[modify] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/chrome/test/BUILD.gn
[modify] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/379cf1b202c478e2744dd5add1a9f81665a7c140/tools/metrics/histograms/histograms.xml

Comment 10 by mfo...@chromium.org, Oct 26 2017

Labels: M-63
btolsch@, is there any more work to do here or can this be closed?

Comment 11 by mfo...@chromium.org, Oct 26 2017

Components: Internals>Cast

Comment 12 by mfo...@chromium.org, Oct 26 2017

Components: -Blink>PresentationAPI

Comment 13 by btolsch@chromium.org, Oct 27 2017 

I was going to leave it open until I resolved the missing metric issue.  I could make a new bug for that if you think that makes more sense though.
Project Member

Comment 14 by bugdroid1@chromium.org, Nov 6 2017 

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

commit 579786f80ac84190b8f85da93ff38ca4afbbe7aa
Author: btolsch <btolsch@chromium.org>
Date: Mon Nov 06 18:33:35 2017

Get discovery network info on creation for metrics

Currently, DiscoveryNetworkMonitor attempts to be lazy and doesn't
require network info is gathered when it is first created.  However,
since metrics are being collected by an observer, it's relevant for
network info to now be collected on creation of DiscoveryNetworkMonitor.

Bug:  698943 
Change-Id: If1214e1a1a484d20ea029d3742142db54ec92104
Reviewed-on: https://chromium-review.googlesource.com/730413
Commit-Queue: Brandon Tolsch <btolsch@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514190}
[modify] https://crrev.com/579786f80ac84190b8f85da93ff38ca4afbbe7aa/chrome/browser/media/router/discovery/discovery_network_monitor.cc
[modify] https://crrev.com/579786f80ac84190b8f85da93ff38ca4afbbe7aa/chrome/browser/media/router/discovery/discovery_network_monitor.h
[modify] https://crrev.com/579786f80ac84190b8f85da93ff38ca4afbbe7aa/chrome/browser/media/router/discovery/discovery_network_monitor_unittest.cc
[modify] https://crrev.com/579786f80ac84190b8f85da93ff38ca4afbbe7aa/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl_unittest.cc

Comment 15 by btolsch@chromium.org, Nov 8 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment