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

Issue 779892 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[DIAL] Implement sink querying in DialMediaSinkService

Project Member Reported by zhaobin@chromium.org, Oct 31 2017

Issue description

Implement logic to answer sink queries in DialMediaSinkService.

It should answer sink queries for the following sources:
- DIAL URLs
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 15 2017

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

commit d94535b3f9d5cf87989883e310b31f442d1450a5
Author: Bin Zhao <zhaobin@chromium.org>
Date: Fri Dec 15 18:22:54 2017

[DIAL] (1st) Added SafeDialAppInfoParser class to parse DIAL app info XML

Recent changes removed dial_device_description_parser_impl* from
chrome/utility/media_router and introduces a new SafeXmlParser.
(https://chromium-review.googlesource.com/c/chromium/src/+/770920)

Abandoned previous DialAppInfoParser patch
(https://chromium-review.googlesource.com/c/chromium/src/+/745538),
resolved most comments in the new patch and added a SafeDialAppInfoParser
class which uses SafeXmlParser.

Bug:  779892 
Change-Id: I1c0d141381e1a8c992888476e8643385a96db06f
Reviewed-on: https://chromium-review.googlesource.com/826126
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Commit-Queue: Bin Zhao <zhaobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524412}
[modify] https://crrev.com/d94535b3f9d5cf87989883e310b31f442d1450a5/chrome/browser/media/router/discovery/BUILD.gn
[modify] https://crrev.com/d94535b3f9d5cf87989883e310b31f442d1450a5/chrome/browser/media/router/discovery/dial/README.md
[add] https://crrev.com/d94535b3f9d5cf87989883e310b31f442d1450a5/chrome/browser/media/router/discovery/dial/parsed_dial_app_info.cc
[add] https://crrev.com/d94535b3f9d5cf87989883e310b31f442d1450a5/chrome/browser/media/router/discovery/dial/parsed_dial_app_info.h
[add] https://crrev.com/d94535b3f9d5cf87989883e310b31f442d1450a5/chrome/browser/media/router/discovery/dial/safe_dial_app_info_parser.cc
[add] https://crrev.com/d94535b3f9d5cf87989883e310b31f442d1450a5/chrome/browser/media/router/discovery/dial/safe_dial_app_info_parser.h
[add] https://crrev.com/d94535b3f9d5cf87989883e310b31f442d1450a5/chrome/browser/media/router/discovery/dial/safe_dial_app_info_parser_unittest.cc
[modify] https://crrev.com/d94535b3f9d5cf87989883e310b31f442d1450a5/chrome/test/BUILD.gn
[modify] https://crrev.com/d94535b3f9d5cf87989883e310b31f442d1450a5/services/data_decoder/public/cpp/safe_xml_parser.cc
[modify] https://crrev.com/d94535b3f9d5cf87989883e310b31f442d1450a5/services/data_decoder/public/cpp/safe_xml_parser.h

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 5 2018

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

commit 0aeec07b185fd69374f358c0f62a774f1c4e11c2
Author: Bin Zhao <zhaobin@chromium.org>
Date: Fri Jan 05 18:23:51 2018

[DIAL] (2nd) Added DialAppDiscoveryService to fetch and parse app info XML

- added DialAppInfoFetcher, which fetches app info XML from |app_url|.
- added DialAppDiscoveryService, which fetches and parses app info XML.

Actual XML parsing happens in utility process
(patch: https://chromium-review.googlesource.com/c/chromium/src/+/745538)

DialAppDiscoveryService is used in patch:
https://chromium-review.googlesource.com/c/chromium/src/+/754039

TODO: add unit tests

Bug:  779892 
Change-Id: I2b6b075ec1833dae981fb67fa223af450c12ac5b
Reviewed-on: https://chromium-review.googlesource.com/754294
Commit-Queue: Bin Zhao <zhaobin@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527325}
[modify] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/browser/media/router/discovery/BUILD.gn
[modify] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/browser/media/router/discovery/dial/device_description_fetcher.cc
[modify] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/browser/media/router/discovery/dial/device_description_fetcher_unittest.cc
[add] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/browser/media/router/discovery/dial/dial_app_discovery_service.cc
[add] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/browser/media/router/discovery/dial/dial_app_discovery_service.h
[add] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/browser/media/router/discovery/dial/dial_app_discovery_service_unittest.cc
[add] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/browser/media/router/discovery/dial/dial_app_info_fetcher.cc
[add] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/browser/media/router/discovery/dial/dial_app_info_fetcher.h
[add] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/browser/media/router/discovery/dial/dial_app_info_fetcher_unittest.cc
[modify] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/browser/media/router/discovery/dial/safe_dial_app_info_parser.cc
[modify] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/browser/media/router/discovery/dial/safe_dial_app_info_parser.h
[modify] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/browser/media/router/discovery/dial/safe_dial_app_info_parser_unittest.cc
[modify] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl_unittest.cc
[modify] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/browser/media/router/test/test_helper.cc
[modify] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/browser/media/router/test/test_helper.h
[modify] https://crrev.com/0aeec07b185fd69374f358c0f62a774f1c4e11c2/chrome/test/BUILD.gn

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 22 2018

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

commit 8f4849d7239a4f98a2d817f1f87070c118aeb0c1
Author: Bin Zhao <zhaobin@chromium.org>
Date: Mon Jan 22 19:10:19 2018

[Media Router] Change MediaSinkServiceBase::current_sinks_ to map to support update

|current_sinks_| used to be a set, and |current_sinks_|.insert(dial_sink) will
not update the existing sink. Change it to a map.

Resolve code review comments in:
https://chromium-review.googlesource.com/c/chromium/src/+/754039/10/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.cc#267

Bug:  779892 
Change-Id: I270cc7e61bbb7e408fc30ffc6e831c2fc0ecc4c6
Reviewed-on: https://chromium-review.googlesource.com/871838
Commit-Queue: Bin Zhao <zhaobin@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530936}
[modify] https://crrev.com/8f4849d7239a4f98a2d817f1f87070c118aeb0c1/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.cc
[modify] https://crrev.com/8f4849d7239a4f98a2d817f1f87070c118aeb0c1/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.h
[modify] https://crrev.com/8f4849d7239a4f98a2d817f1f87070c118aeb0c1/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl_unittest.cc
[modify] https://crrev.com/8f4849d7239a4f98a2d817f1f87070c118aeb0c1/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.cc
[modify] https://crrev.com/8f4849d7239a4f98a2d817f1f87070c118aeb0c1/chrome/common/media_router/discovery/media_sink_service_base.cc
[modify] https://crrev.com/8f4849d7239a4f98a2d817f1f87070c118aeb0c1/chrome/common/media_router/discovery/media_sink_service_base.h
[modify] https://crrev.com/8f4849d7239a4f98a2d817f1f87070c118aeb0c1/chrome/common/media_router/discovery/media_sink_service_base_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 1 2018

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

commit 822e52472a6f2bbc0dc7a4815ef2b1bfc9f62566
Author: Bin Zhao <zhaobin@chromium.org>
Date: Thu Feb 01 19:28:40 2018

[Media Router] Pass processed device UUID as sink id to extension

We are going to use device UUID instead of hashed device UUID as sink id
in both browser side and extension side.

Added MediaSinkInternal::GetSinkIdFromDeviceUUID() to unify different
device ids from DIAL and cast.

Extension side change: cl/182557041

Bug:  779892 
Change-Id: I5843af35c75d74006849a70d07236a8def3dd922
Reviewed-on: https://chromium-review.googlesource.com/889501
Commit-Queue: Bin Zhao <zhaobin@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533776}
[modify] https://crrev.com/822e52472a6f2bbc0dc7a4815ef2b1bfc9f62566/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.cc
[modify] https://crrev.com/822e52472a6f2bbc0dc7a4815ef2b1bfc9f62566/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.cc
[modify] https://crrev.com/822e52472a6f2bbc0dc7a4815ef2b1bfc9f62566/chrome/common/media_router/discovery/media_sink_internal.cc
[modify] https://crrev.com/822e52472a6f2bbc0dc7a4815ef2b1bfc9f62566/chrome/common/media_router/discovery/media_sink_internal.h
[modify] https://crrev.com/822e52472a6f2bbc0dc7a4815ef2b1bfc9f62566/chrome/common/media_router/discovery/media_sink_internal_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 3 2018

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

commit 2459d2cdbe49cc71efee27fa973c7cd04c4f8517
Author: Bin Zhao <zhaobin@chromium.org>
Date: Sat Feb 03 00:39:17 2018

[Media Router] Added EnableDialSinkQuery feature to enable/disable in-browser DIAL sink query

We are moving DIAL sink query logic from extension side to browser side.
Added a feature to enable/disable in-browser sink query.

- Added EnableDialSinkQuery feature
- Added enable_dial_sink_query field to MediaRouteProviderConfig in media_router.mojom

Bug:  779892 
Change-Id: Ib05013b1124f22a45a3647765f026f20614e8d4f
Reviewed-on: https://chromium-review.googlesource.com/896265
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Bin Zhao <zhaobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534213}
[modify] https://crrev.com/2459d2cdbe49cc71efee27fa973c7cd04c4f8517/chrome/browser/media/router/media_router_feature.cc
[modify] https://crrev.com/2459d2cdbe49cc71efee27fa973c7cd04c4f8517/chrome/browser/media/router/media_router_feature.h
[modify] https://crrev.com/2459d2cdbe49cc71efee27fa973c7cd04c4f8517/chrome/browser/media/router/mojo/media_router_desktop.cc
[modify] https://crrev.com/2459d2cdbe49cc71efee27fa973c7cd04c4f8517/chrome/common/media_router/mojo/media_router.mojom
[modify] https://crrev.com/2459d2cdbe49cc71efee27fa973c7cd04c4f8517/chrome/renderer/resources/extensions/media_router_bindings.js

Blockedon: 808720
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 5 2018

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

commit 46cae9e4a532cc4b2776578bb55a4dc3fdd02e71
Author: Bin Zhao <zhaobin@chromium.org>
Date: Mon Feb 05 19:06:32 2018

[DIAL] (3rd) Make DialMediaSinkService handle sink query to DIAL media source

- MediaRouterDesktop directs media sink observers for DIAL media source
  to DialMediaSinkService.
- DialMediaSinkService keeps a map of DIAL media sources keyed by
  app name
- DialMediaSinkService delegates DialMediaSinkServiceImpl to start
  fetching and parsing app info when:
  1. user opens MR dialog (DialMediaSinkServiceImpl::OnUserGesture())
  2. DialMediaSinkServiceImpl finishes parsing device description XML
     (DialMediaSinkServiceImpl::OnDeviceDescriptionAvailable())
  3. DialMediaSinkServiceImpl starts observing app name
     (DialMediaSinkServiceImpl::StartMonitoringAvailableSinksForApp())
- When parsed app info gets back to DialMediaSinkServiceImpl, it checks
  if available sinks for app name has changed. If so, post task to
  DialMediaSinkServiceProxy, which will notify corresponding media sink
  observers.

Bug:  779892 
Change-Id: Ibdf99df03b37e7d6dfea07092bc28a54970219b5
Reviewed-on: https://chromium-review.googlesource.com/754039
Commit-Queue: Bin Zhao <zhaobin@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534442}
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/browser/media/router/discovery/dial/dial_app_discovery_service.cc
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/browser/media/router/discovery/dial/dial_app_discovery_service.h
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/browser/media/router/discovery/dial/dial_media_sink_service.cc
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/browser/media/router/discovery/dial/dial_media_sink_service.h
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.cc
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.h
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl_unittest.cc
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/browser/media/router/discovery/dial/dial_media_sink_service_unittest.cc
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/browser/media/router/media_sinks_observer.h
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/browser/media/router/providers/cast/dual_media_sink_service.cc
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/browser/media/router/providers/cast/dual_media_sink_service.h
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/common/media_router/discovery/media_sink_service_util.cc
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/common/media_router/discovery/media_sink_service_util.h
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/common/media_router/media_source_helper.cc
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/common/media_router/media_source_helper.h
[modify] https://crrev.com/46cae9e4a532cc4b2776578bb55a4dc3fdd02e71/chrome/common/media_router/media_source_helper_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 9 2018

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

commit df68543c0d9b9407ff82d5e3ad360fdb46de68fb
Author: Derek Cheng <imcheng@chromium.org>
Date: Fri Feb 09 00:56:50 2018

[Media Router] Eagerly initialize DualMediaSinkService with MRDesktop.

DualMediaSinkService should be instantiated along with MRDesktop to
start discovery. In subsequent sink query patches, we will also need
to use it to initialize in-browser DIAL / Cast MRPs.

This behavior causes some unit tests and browser tests to fail.

For the former, it is mostly because they were not using the mock
version of MediaRouter consistently, so this patch fixes that.

For browser tests, MediaRouterDesktop, and therefore DualMSS, are
instantiated during test setup. The extension API tests fail because
they assume the test is run in an isolated environment (i.e. no outer
devices) and that they are the only DIAL / mDNS observers. Because
we don't want actual discovery to run in those tests, a no-op version
of DualMSS is introduced and set as the instance before the browser
test setup.

Bug:  698940 , 779892 
Change-Id: Iea7467c430fa78944f802dcd15964483c65e674b
Reviewed-on: https://chromium-review.googlesource.com/907640
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Gene Gutnik <gene@chromium.org>
Reviewed-by: Bin Zhao <zhaobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535599}
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/extensions/api/dial/dial_apitest.cc
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/media/router/BUILD.gn
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/media/router/media_router_factory_unittest.cc
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/media/router/mojo/media_router_desktop.cc
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/media/router/presentation/presentation_service_delegate_impl.cc
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/media/router/presentation/presentation_service_delegate_impl.h
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/media/router/presentation/presentation_service_delegate_impl_unittest.cc
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/media/router/providers/cast/dual_media_sink_service.cc
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/media/router/providers/cast/dual_media_sink_service.h
[add] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/media/router/test/noop_dual_media_sink_service.cc
[add] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/media/router/test/noop_dual_media_sink_service.h
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/ui/webui/local_discovery/local_discovery_ui_browsertest.cc
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/ui/webui/media_router/media_router_ui.cc
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/ui/webui/media_router/media_router_ui.h
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/ui/webui/media_router/media_router_ui_service_factory_unittest.cc
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/ui/webui/media_router/media_router_ui_unittest.cc
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/chrome/browser/ui/webui/media_router/media_router_web_ui_test.cc
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/extensions/browser/api/cast_channel/DEPS
[modify] https://crrev.com/df68543c0d9b9407ff82d5e3ad360fdb46de68fb/extensions/browser/api/cast_channel/cast_channel_apitest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 9 2018

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

commit 145e34dec303fd240e4d8bb0592ba57043d69b36
Author: Olga Sharonova <olka@chromium.org>
Date: Fri Feb 09 09:06:52 2018

Revert "[Media Router] Eagerly initialize DualMediaSinkService with MRDesktop."

This reverts commit df68543c0d9b9407ff82d5e3ad360fdb46de68fb.

Reason for revert: suspected cause of  Issue 810692 
Bug:  810692 

Original change's description:
> [Media Router] Eagerly initialize DualMediaSinkService with MRDesktop.
> 
> DualMediaSinkService should be instantiated along with MRDesktop to
> start discovery. In subsequent sink query patches, we will also need
> to use it to initialize in-browser DIAL / Cast MRPs.
> 
> This behavior causes some unit tests and browser tests to fail.
> 
> For the former, it is mostly because they were not using the mock
> version of MediaRouter consistently, so this patch fixes that.
> 
> For browser tests, MediaRouterDesktop, and therefore DualMSS, are
> instantiated during test setup. The extension API tests fail because
> they assume the test is run in an isolated environment (i.e. no outer
> devices) and that they are the only DIAL / mDNS observers. Because
> we don't want actual discovery to run in those tests, a no-op version
> of DualMSS is introduced and set as the instance before the browser
> test setup.
> 
> Bug:  698940 , 779892 
> Change-Id: Iea7467c430fa78944f802dcd15964483c65e674b
> Reviewed-on: https://chromium-review.googlesource.com/907640
> Commit-Queue: Derek Cheng <imcheng@chromium.org>
> Reviewed-by: Gene Gutnik <gene@chromium.org>
> Reviewed-by: Bin Zhao <zhaobin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#535599}

TBR=gene@chromium.org,imcheng@chromium.org,zhaobin@chromium.org

Change-Id: I1baf83954e6f5a838fc52ca95062a375d869b2cd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  698940 ,  779892 
Reviewed-on: https://chromium-review.googlesource.com/910050
Reviewed-by: Olga Sharonova <olka@chromium.org>
Commit-Queue: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535672}
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/extensions/api/dial/dial_apitest.cc
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/media/router/BUILD.gn
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/media/router/media_router_factory_unittest.cc
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/media/router/mojo/media_router_desktop.cc
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/media/router/presentation/presentation_service_delegate_impl.cc
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/media/router/presentation/presentation_service_delegate_impl.h
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/media/router/presentation/presentation_service_delegate_impl_unittest.cc
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/media/router/providers/cast/dual_media_sink_service.cc
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/media/router/providers/cast/dual_media_sink_service.h
[delete] https://crrev.com/5641d755098b3e7cae73b2bb87fa16adc6580f69/chrome/browser/media/router/test/noop_dual_media_sink_service.cc
[delete] https://crrev.com/5641d755098b3e7cae73b2bb87fa16adc6580f69/chrome/browser/media/router/test/noop_dual_media_sink_service.h
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/ui/webui/local_discovery/local_discovery_ui_browsertest.cc
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/ui/webui/media_router/media_router_ui.cc
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/ui/webui/media_router/media_router_ui.h
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/ui/webui/media_router/media_router_ui_service_factory_unittest.cc
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/ui/webui/media_router/media_router_ui_unittest.cc
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/chrome/browser/ui/webui/media_router/media_router_web_ui_test.cc
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/extensions/browser/api/cast_channel/DEPS
[modify] https://crrev.com/145e34dec303fd240e4d8bb0592ba57043d69b36/extensions/browser/api/cast_channel/cast_channel_apitest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Feb 21 2018

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

commit cd6b757d953001eae759c4786b5afe9c7e00916f
Author: Derek Cheng <imcheng@chromium.org>
Date: Wed Feb 21 20:15:23 2018

[Media Router] Fix URLRequestContext leak and race condition.

Remove usage of SystemURLRequestContextGetter which has led to several
issues including shutdown leak and test setup problems. This means 2
things:
- No more NetLog for DIAL and Cast sockets (for now. When we switch to
NetworkService we can reconsider adding back support depending on
our needs)
- DIAL device description / app info fetch now replaced with URLLoader
APIs that's part of network service.

The race condition issue caused by calling CMSSImpl::GetWeakPtr on two
different sequence is fixed by not using a WeakPtr by assuming
CMSSImpl outlives DialMSSImpl, which is true due to their ordering in
DualMediaSinkService. Also removed the PostTask trampoline by assuming
they both run on the same sequence.

Bug:  810676 , 805728 , 698940 , 779892 , 811226 
Change-Id: Ib3ebb4f7f9d3690c952d2ddcbd579bb5d876906e
Reviewed-on: https://chromium-review.googlesource.com/912561
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Bin Zhao <zhaobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538195}
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/extensions/api/dial/dial_api.cc
[add] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/DEPS
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/device_description_fetcher.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/device_description_fetcher.h
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/device_description_fetcher_unittest.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/device_description_service.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/device_description_service.h
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/device_description_service_unittest.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/dial_app_discovery_service.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/dial_app_discovery_service.h
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/dial_app_discovery_service_unittest.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/dial_app_info_fetcher.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/dial_app_info_fetcher.h
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/dial_app_info_fetcher_unittest.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/dial_media_sink_service.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/dial_media_sink_service.h
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.h
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl_unittest.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/dial/dial_media_sink_service_unittest.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.h
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.h
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl_unittest.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_unittest.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/mojo/media_router_desktop_unittest.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/providers/cast/dual_media_sink_service.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/providers/cast/dual_media_sink_service.h
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/providers/cast/dual_media_sink_service_unittest.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/test/test_helper.cc
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/browser/media/router/test/test_helper.h
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/chrome/test/BUILD.gn
[modify] https://crrev.com/cd6b757d953001eae759c4786b5afe9c7e00916f/components/cast_channel/cast_socket.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Feb 22 2018

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

commit ea1cbe27ced63d2d6aa72cc95db460fb06a9f554
Author: Derek Cheng <imcheng@chromium.org>
Date: Thu Feb 22 18:24:33 2018

[Reland][Media Router] Eagerly initialize DualMSS with MRDesktop.

The patch was previously reverted in 910050 due to leaking system
URLRequestContext. This issue has been fixed with 912561 and this patch
is ready to be relanded.


TBR=gene@chromium.org

Bug:  698940 , 779892 
Change-Id: Ib67ffdc8cf6fe1504bdad95ecdf9f51face030a9
Reviewed-on: https://chromium-review.googlesource.com/929864
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538491}
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/extensions/api/dial/dial_apitest.cc
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/media/router/BUILD.gn
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/media/router/media_router_factory_unittest.cc
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/media/router/mojo/media_router_desktop.cc
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/media/router/presentation/presentation_service_delegate_impl.cc
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/media/router/presentation/presentation_service_delegate_impl.h
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/media/router/presentation/presentation_service_delegate_impl_unittest.cc
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/media/router/providers/cast/dual_media_sink_service.cc
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/media/router/providers/cast/dual_media_sink_service.h
[add] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/media/router/test/noop_dual_media_sink_service.cc
[add] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/media/router/test/noop_dual_media_sink_service.h
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/ui/webui/local_discovery/local_discovery_ui_browsertest.cc
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/ui/webui/media_router/media_router_ui.cc
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/ui/webui/media_router/media_router_ui.h
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/ui/webui/media_router/media_router_ui_service_factory_unittest.cc
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/ui/webui/media_router/media_router_ui_unittest.cc
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/chrome/browser/ui/webui/media_router/media_router_web_ui_test.cc
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/extensions/browser/api/cast_channel/DEPS
[modify] https://crrev.com/ea1cbe27ced63d2d6aa72cc95db460fb06a9f554/extensions/browser/api/cast_channel/cast_channel_apitest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Mar 16 2018

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

commit 3e0c55db441fd9dbc74964b426c210bca8beb093
Author: Bin Zhao <zhaobin@chromium.org>
Date: Fri Mar 16 23:40:51 2018

[DIAL] added an in-browser DIAL mrp skeleton for DIAL sink query

dial_media_sink_service:
  - Added Register/UnregisterOnSinkQueryUpdatedCallback()
  - Start/StopObservingMediaSinks() takes std::string media_source as parameter

dial_media_sink_service_impl:
  - Do not check app availability for discovery only devices

media_router_desktop:
  - Initialize DialMediaRouteProvider if DialSinkQueryEnabled()

media_router_mojo_impl:
  - Use extension MRP to create/terminate route for DIAL sink

dial_media_route_provider:
  - Register OnSinkQueryUpdatedCallback with dial_media_sink_service

Bug:  779892 
Change-Id: Iddc760c4fa035c00210480f28de7db96e42fe61c
Reviewed-on: https://chromium-review.googlesource.com/903419
Commit-Queue: Bin Zhao <zhaobin@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543872}
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/BUILD.gn
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/discovery/dial/dial_media_sink_service.cc
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/discovery/dial/dial_media_sink_service.h
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.cc
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.h
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl_unittest.cc
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/discovery/dial/dial_media_sink_service_unittest.cc
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/mojo/media_router_desktop.cc
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/mojo/media_router_desktop.h
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/mojo/media_router_mojo_metrics.cc
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/providers/cast/dual_media_sink_service.cc
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/providers/cast/dual_media_sink_service.h
[add] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/providers/dial/dial_media_route_provider.cc
[add] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/providers/dial/dial_media_route_provider.h
[add] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/providers/dial/dial_media_route_provider_unittest.cc
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/test/noop_dual_media_sink_service.h
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/test/test_helper.cc
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/browser/media/router/test/test_helper.h
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/common/media_router/media_route_provider_helper.cc
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/common/media_router/media_route_provider_helper.h
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/common/media_router/media_source.cc
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/common/media_router/media_source.h
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/common/media_router/mojo/media_router.mojom
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/common/media_router/mojo/media_router_struct_traits.h
[modify] https://crrev.com/3e0c55db441fd9dbc74964b426c210bca8beb093/chrome/test/BUILD.gn

Status: Fixed (was: Started)
Query is now implemented and behind a feature flag.
Blockedon: -808720

Sign in to add a comment