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

Issue 687377 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocked on:
issue 687356
issue 723944



Sign in to add a comment

[Cast] Implement Cast MediaSinkService

Project Member Reported by imch...@chromium.org, Jan 31 2017

Issue description

Implement MediaSinkService for Cast which can be used by MediaRouter directly.

Prequisites:
- Refactor DnsSdRegistry and friends so that it can be used by MediaSinkService

Some things to keep in mind when porting over from MR extension:
- Dual discovery using DIAL MediaSinkService
- Network switching optimization - ideally we should have a unified network switch detection logic across mDNS / DIAL that works consistently across platforms.
 
Components: Blink>PresentationAPI
Blockedon: 687356
Blocking: 687383
Status: Available (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, May 16 2017

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

commit b9946bb24dac07cb38b87697114678f6f416b20d
Author: zhaobin <zhaobin@chromium.org>
Date: Tue May 16 17:46:49 2017

[mDns] Move dns_sd_registry from extension/api/mdns to media/router/discovery/mdns

Move DnsSdRegistry and related classed to MR folder so that it can be used by CastMediaSinkService.

BUG= 687377 

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

[modify] https://crrev.com/b9946bb24dac07cb38b87697114678f6f416b20d/chrome/browser/extensions/BUILD.gn
[modify] https://crrev.com/b9946bb24dac07cb38b87697114678f6f416b20d/chrome/browser/extensions/api/mdns/mdns_api.cc
[modify] https://crrev.com/b9946bb24dac07cb38b87697114678f6f416b20d/chrome/browser/extensions/api/mdns/mdns_api.h
[modify] https://crrev.com/b9946bb24dac07cb38b87697114678f6f416b20d/chrome/browser/extensions/api/mdns/mdns_api_unittest.cc
[modify] https://crrev.com/b9946bb24dac07cb38b87697114678f6f416b20d/chrome/browser/extensions/api/mdns/mdns_apitest.cc
[modify] https://crrev.com/b9946bb24dac07cb38b87697114678f6f416b20d/chrome/browser/media/router/discovery/BUILD.gn
[rename] https://crrev.com/b9946bb24dac07cb38b87697114678f6f416b20d/chrome/browser/media/router/discovery/mdns/dns_sd_delegate.cc
[rename] https://crrev.com/b9946bb24dac07cb38b87697114678f6f416b20d/chrome/browser/media/router/discovery/mdns/dns_sd_delegate.h
[rename] https://crrev.com/b9946bb24dac07cb38b87697114678f6f416b20d/chrome/browser/media/router/discovery/mdns/dns_sd_device_lister.cc
[rename] https://crrev.com/b9946bb24dac07cb38b87697114678f6f416b20d/chrome/browser/media/router/discovery/mdns/dns_sd_device_lister.h
[rename] https://crrev.com/b9946bb24dac07cb38b87697114678f6f416b20d/chrome/browser/media/router/discovery/mdns/dns_sd_registry.cc
[rename] https://crrev.com/b9946bb24dac07cb38b87697114678f6f416b20d/chrome/browser/media/router/discovery/mdns/dns_sd_registry.h
[rename] https://crrev.com/b9946bb24dac07cb38b87697114678f6f416b20d/chrome/browser/media/router/discovery/mdns/dns_sd_registry_unittest.cc
[modify] https://crrev.com/b9946bb24dac07cb38b87697114678f6f416b20d/chrome/test/BUILD.gn

Project Member

Comment 6 by bugdroid1@chromium.org, May 16 2017

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

commit 7b350bad16013dbcd2d827072ae7827f0c36a005
Author: zhaobin <zhaobin@chromium.org>
Date: Tue May 16 20:13:02 2017

[mDns] Make DnsSdRegistry a leaky singleton

Make DnsSdRegistry a leaky singleton so that it can be used by both CastMediaSinkService and MDnsAPI.

BUG= 687377 

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

[modify] https://crrev.com/7b350bad16013dbcd2d827072ae7827f0c36a005/chrome/browser/extensions/api/mdns/mdns_api.cc
[modify] https://crrev.com/7b350bad16013dbcd2d827072ae7827f0c36a005/chrome/browser/extensions/api/mdns/mdns_api.h
[modify] https://crrev.com/7b350bad16013dbcd2d827072ae7827f0c36a005/chrome/browser/extensions/api/mdns/mdns_api_unittest.cc
[modify] https://crrev.com/7b350bad16013dbcd2d827072ae7827f0c36a005/chrome/browser/extensions/api/mdns/mdns_apitest.cc
[modify] https://crrev.com/7b350bad16013dbcd2d827072ae7827f0c36a005/chrome/browser/media/router/BUILD.gn
[modify] https://crrev.com/7b350bad16013dbcd2d827072ae7827f0c36a005/chrome/browser/media/router/discovery/mdns/dns_sd_registry.cc
[modify] https://crrev.com/7b350bad16013dbcd2d827072ae7827f0c36a005/chrome/browser/media/router/discovery/mdns/dns_sd_registry.h
[add] https://crrev.com/7b350bad16013dbcd2d827072ae7827f0c36a005/chrome/browser/media/router/discovery/mdns/mock_dns_sd_registry.cc
[add] https://crrev.com/7b350bad16013dbcd2d827072ae7827f0c36a005/chrome/browser/media/router/discovery/mdns/mock_dns_sd_registry.h

Blockedon: 723944
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 21 2017

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

commit 487f46f96fbed1f58c8dc3fb2d940dd5f36ca7b3
Author: zhaobin <zhaobin@chromium.org>
Date: Wed Jun 21 22:35:21 2017

[cast_channel] Make CastMessageHandler a CastSocket::Observer instead of CastTransport::Delegate

We are going to create one CastSocket for one IP endpoint and share it between browser and different extensions. To handle message and errors, it seems more reasonable to have one CastTransport::Delegate and multiple observers per CastSocket.

BUG= 687377 

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

[modify] https://crrev.com/487f46f96fbed1f58c8dc3fb2d940dd5f36ca7b3/components/cast_channel/cast_socket.cc
[modify] https://crrev.com/487f46f96fbed1f58c8dc3fb2d940dd5f36ca7b3/components/cast_channel/cast_socket.h
[modify] https://crrev.com/487f46f96fbed1f58c8dc3fb2d940dd5f36ca7b3/components/cast_channel/cast_socket_service.cc
[modify] https://crrev.com/487f46f96fbed1f58c8dc3fb2d940dd5f36ca7b3/components/cast_channel/cast_socket_service.h
[modify] https://crrev.com/487f46f96fbed1f58c8dc3fb2d940dd5f36ca7b3/components/cast_channel/cast_socket_unittest.cc
[modify] https://crrev.com/487f46f96fbed1f58c8dc3fb2d940dd5f36ca7b3/components/cast_channel/cast_test_util.cc
[modify] https://crrev.com/487f46f96fbed1f58c8dc3fb2d940dd5f36ca7b3/components/cast_channel/cast_test_util.h
[modify] https://crrev.com/487f46f96fbed1f58c8dc3fb2d940dd5f36ca7b3/extensions/browser/api/cast_channel/cast_channel_api.cc
[modify] https://crrev.com/487f46f96fbed1f58c8dc3fb2d940dd5f36ca7b3/extensions/browser/api/cast_channel/cast_channel_api.h
[modify] https://crrev.com/487f46f96fbed1f58c8dc3fb2d940dd5f36ca7b3/extensions/browser/api/cast_channel/cast_channel_apitest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 28 2017

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

commit 9662cb983c489007e8dc9a58f91d4fbb47f64290
Author: zhaobin <zhaobin@chromium.org>
Date: Wed Jun 28 17:13:52 2017

[cast_channel] Implement CastSocketService::OpenSocket()

Extract open channel logic from CastChannelOpenFunction::AsyncWorkStart() so that it can be use by CastMediaSinkService.

OpenChannel(ip_address1, ..., op_cb_1);
OpenChannel(ip_address1, ..., op_cb_2);

Should only create one cast socket and invoke both op_cb_1 and op_cb_2 when socket open finishes.

BUG= 687377 

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

[modify] https://crrev.com/9662cb983c489007e8dc9a58f91d4fbb47f64290/components/cast_channel/cast_channel_enum.h
[modify] https://crrev.com/9662cb983c489007e8dc9a58f91d4fbb47f64290/components/cast_channel/cast_socket.cc
[modify] https://crrev.com/9662cb983c489007e8dc9a58f91d4fbb47f64290/components/cast_channel/cast_socket.h
[modify] https://crrev.com/9662cb983c489007e8dc9a58f91d4fbb47f64290/components/cast_channel/cast_socket_service.cc
[modify] https://crrev.com/9662cb983c489007e8dc9a58f91d4fbb47f64290/components/cast_channel/cast_socket_service.h
[modify] https://crrev.com/9662cb983c489007e8dc9a58f91d4fbb47f64290/components/cast_channel/cast_socket_service_unittest.cc
[modify] https://crrev.com/9662cb983c489007e8dc9a58f91d4fbb47f64290/components/cast_channel/cast_socket_unittest.cc
[modify] https://crrev.com/9662cb983c489007e8dc9a58f91d4fbb47f64290/components/cast_channel/cast_test_util.cc
[modify] https://crrev.com/9662cb983c489007e8dc9a58f91d4fbb47f64290/components/cast_channel/cast_test_util.h
[modify] https://crrev.com/9662cb983c489007e8dc9a58f91d4fbb47f64290/extensions/browser/api/cast_channel/cast_channel_api.cc
[modify] https://crrev.com/9662cb983c489007e8dc9a58f91d4fbb47f64290/extensions/browser/api/cast_channel/cast_channel_api.h
[modify] https://crrev.com/9662cb983c489007e8dc9a58f91d4fbb47f64290/extensions/browser/api/cast_channel/cast_channel_apitest.cc

Cc: julien.isorce@chromium.org
Project Member

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

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

commit b9a2a9b60034d3ee2138c4815bc3c66b13cee3b5
Author: zhaobin <zhaobin@chromium.org>
Date: Fri Jul 07 17:50:54 2017

[Media Router] Add CastMediaSinkService

NOTE: those extensions header files and extensions::api:: namespace will go away when https://codereview.chromium.org/2913033003/ lands

Discover process is similar to DialMediaSinkService:
- CastMediaSinkService::Start() registers itself with DnsRegistry
- CastMediaSinkService::OnDnsSdEvent() gets invoked when Dns services comes back and starts a 3s timer
- Start opening channel for each service on IO thread
- CastMediaSinkService::OnChannelOpenedOnIOThread() gets invoked when channel open finishes
- Post task back to UI thread via CastMediaSinkService::OnChannelOpenOnUIThread()
- Create MediaSinkInternal and store it in CastSinkService's sink map
- Invoke MediaSinkBase::FetchCompleted() and send sinks to MRP when timer expires

TODO:
  Unit test
  Implement CastSocketService::OpenChannel

BUG= 687377 

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

[modify] https://crrev.com/b9a2a9b60034d3ee2138c4815bc3c66b13cee3b5/chrome/browser/media/router/discovery/BUILD.gn
[add] https://crrev.com/b9a2a9b60034d3ee2138c4815bc3c66b13cee3b5/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.cc
[add] https://crrev.com/b9a2a9b60034d3ee2138c4815bc3c66b13cee3b5/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.h
[add] https://crrev.com/b9a2a9b60034d3ee2138c4815bc3c66b13cee3b5/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_unittest.cc
[modify] https://crrev.com/b9a2a9b60034d3ee2138c4815bc3c66b13cee3b5/chrome/browser/media/router/discovery/mdns/mock_dns_sd_registry.cc
[modify] https://crrev.com/b9a2a9b60034d3ee2138c4815bc3c66b13cee3b5/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
[modify] https://crrev.com/b9a2a9b60034d3ee2138c4815bc3c66b13cee3b5/chrome/browser/media/router/mojo/media_router_mojo_impl.h
[modify] https://crrev.com/b9a2a9b60034d3ee2138c4815bc3c66b13cee3b5/chrome/test/BUILD.gn
[modify] https://crrev.com/b9a2a9b60034d3ee2138c4815bc3c66b13cee3b5/components/cast_channel/cast_socket_service.h

Project Member

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

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

commit 989a05a98d9d8a1dbd1bec1d943e2456e395e671
Author: bsep <bsep@chromium.org>
Date: Fri Jul 07 18:38:04 2017

Revert of [Media Router] Add CastMediaSinkService (patchset #11 id:200001 of https://codereview.chromium.org/2927833002/ )

Reason for revert:
Causing unit_tests failures on multiple bots

For example:
https://uberchromegw.corp.google.com/i/chromium.mac/builders/Mac10.9%20Tests/builds/44050
https://uberchromegw.corp.google.com/i/chromium.mac/builders/Mac10.10%20Tests/builds/20527
https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20TSan%20Tests/builds/8549

Original issue's description:
> [Media Router] Add CastMediaSinkService
>
> NOTE: those extensions header files and extensions::api:: namespace will go away when https://codereview.chromium.org/2913033003/ lands
>
> Discover process is similar to DialMediaSinkService:
> - CastMediaSinkService::Start() registers itself with DnsRegistry
> - CastMediaSinkService::OnDnsSdEvent() gets invoked when Dns services comes back and starts a 3s timer
> - Start opening channel for each service on IO thread
> - CastMediaSinkService::OnChannelOpenedOnIOThread() gets invoked when channel open finishes
> - Post task back to UI thread via CastMediaSinkService::OnChannelOpenOnUIThread()
> - Create MediaSinkInternal and store it in CastSinkService's sink map
> - Invoke MediaSinkBase::FetchCompleted() and send sinks to MRP when timer expires
>
> TODO:
>   Unit test
>   Implement CastSocketService::OpenChannel
>
> BUG= 687377 
>
> Review-Url: https://codereview.chromium.org/2927833002
> Cr-Commit-Position: refs/heads/master@{#484979}
> Committed: https://chromium.googlesource.com/chromium/src/+/b9a2a9b60034d3ee2138c4815bc3c66b13cee3b5

TBR=mfoltz@chromium.org,imcheng@chromium.org,zhaobin@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 687377 

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

[modify] https://crrev.com/989a05a98d9d8a1dbd1bec1d943e2456e395e671/chrome/browser/media/router/discovery/BUILD.gn
[delete] https://crrev.com/4b488983778a6d9d6c60c2620ddf8e3ca99e84cc/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.cc
[delete] https://crrev.com/4b488983778a6d9d6c60c2620ddf8e3ca99e84cc/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.h
[delete] https://crrev.com/4b488983778a6d9d6c60c2620ddf8e3ca99e84cc/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_unittest.cc
[modify] https://crrev.com/989a05a98d9d8a1dbd1bec1d943e2456e395e671/chrome/browser/media/router/discovery/mdns/mock_dns_sd_registry.cc
[modify] https://crrev.com/989a05a98d9d8a1dbd1bec1d943e2456e395e671/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
[modify] https://crrev.com/989a05a98d9d8a1dbd1bec1d943e2456e395e671/chrome/browser/media/router/mojo/media_router_mojo_impl.h
[modify] https://crrev.com/989a05a98d9d8a1dbd1bec1d943e2456e395e671/chrome/test/BUILD.gn
[modify] https://crrev.com/989a05a98d9d8a1dbd1bec1d943e2456e395e671/components/cast_channel/cast_socket_service.h

Project Member

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

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

commit 59516e7383c4e6e0231ca54d664f8fa0ea039af0
Author: zhaobin <zhaobin@chromium.org>
Date: Sat Jul 08 00:22:47 2017

[Media Router] Add CastMediaSinkService

NOTE: those extensions header files and extensions::api:: namespace will go away when https://codereview.chromium.org/2913033003/ lands

Discover process is similar to DialMediaSinkService:
- CastMediaSinkService::Start() registers itself with DnsRegistry
- CastMediaSinkService::OnDnsSdEvent() gets invoked when Dns services comes back and starts a 3s timer
- Start opening channel for each service on IO thread
- CastMediaSinkService::OnChannelOpenedOnIOThread() gets invoked when channel open finishes
- Post task back to UI thread via CastMediaSinkService::OnChannelOpenOnUIThread()
- Create MediaSinkInternal and store it in CastSinkService's sink map
- Invoke MediaSinkBase::FetchCompleted() and send sinks to MRP when timer expires

BUG= 687377 

Review-Url: https://codereview.chromium.org/2927833002
Cr-Original-Commit-Position: refs/heads/master@{#484979}
Committed: https://chromium.googlesource.com/chromium/src/+/b9a2a9b60034d3ee2138c4815bc3c66b13cee3b5
Review-Url: https://codereview.chromium.org/2927833002
Cr-Commit-Position: refs/heads/master@{#485110}

[modify] https://crrev.com/59516e7383c4e6e0231ca54d664f8fa0ea039af0/chrome/browser/media/router/discovery/BUILD.gn
[add] https://crrev.com/59516e7383c4e6e0231ca54d664f8fa0ea039af0/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.cc
[add] https://crrev.com/59516e7383c4e6e0231ca54d664f8fa0ea039af0/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.h
[add] https://crrev.com/59516e7383c4e6e0231ca54d664f8fa0ea039af0/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_unittest.cc
[modify] https://crrev.com/59516e7383c4e6e0231ca54d664f8fa0ea039af0/chrome/browser/media/router/discovery/mdns/mock_dns_sd_registry.cc
[modify] https://crrev.com/59516e7383c4e6e0231ca54d664f8fa0ea039af0/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
[modify] https://crrev.com/59516e7383c4e6e0231ca54d664f8fa0ea039af0/chrome/browser/media/router/mojo/media_router_mojo_impl.h
[modify] https://crrev.com/59516e7383c4e6e0231ca54d664f8fa0ea039af0/chrome/test/BUILD.gn
[modify] https://crrev.com/59516e7383c4e6e0231ca54d664f8fa0ea039af0/components/cast_channel/cast_socket_service.h

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 14 2017

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

commit edc090649966c37e060d073addca425382dc4e38
Author: zhaobin <zhaobin@chromium.org>
Date: Fri Jul 14 21:06:04 2017

[cast_channel] Make CastSocketService a global leaky singleton

- make CastSocketService a global leaky singleton and remove CastSocketServiceFactory
- make CastChannelAPI and CastMediaSinkService own CastSocket::Observer
- fix unit tests and browser tests

BUG= 687377 

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

[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.cc
[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.h
[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_unittest.cc
[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/chrome/test/data/extensions/api_test/cast_channel/api/test_open_timeout.js
[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/chrome/test/data/extensions/api_test/cast_channel/api/test_open_timeout_verified.js
[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/components/cast_channel/BUILD.gn
[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/components/cast_channel/cast_socket.cc
[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/components/cast_channel/cast_socket.h
[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/components/cast_channel/cast_socket_service.cc
[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/components/cast_channel/cast_socket_service.h
[delete] https://crrev.com/26a60c2908155fcba28a21b1065d21adbc39a1c6/components/cast_channel/cast_socket_service_factory.cc
[delete] https://crrev.com/26a60c2908155fcba28a21b1065d21adbc39a1c6/components/cast_channel/cast_socket_service_factory.h
[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/components/cast_channel/cast_socket_service_unittest.cc
[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/components/cast_channel/cast_test_util.cc
[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/components/cast_channel/cast_test_util.h
[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/extensions/browser/api/cast_channel/cast_channel_api.cc
[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/extensions/browser/api/cast_channel/cast_channel_api.h
[modify] https://crrev.com/edc090649966c37e060d073addca425382dc4e38/extensions/browser/api/cast_channel/cast_channel_apitest.cc

Project Member

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

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

commit 31325c45c6401f0d233db23eec1f2962ff796b69
Author: Bin Zhao <zhaobin@chromium.org>
Date: Tue Jul 18 23:56:47 2017

[cast_channel] Make CastSocket::Connect() take base::OnceCallback

Resolve code review comments: https://codereview.chromium.org/2965843002/diff/120001/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.cc

BUG:  687377 
Change-Id: I04d85925090a1cb1db67499fcde1bc9b2983f97f
Reviewed-on: https://chromium-review.googlesource.com/571213
Commit-Queue: Bin Zhao <zhaobin@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487672}
[modify] https://crrev.com/31325c45c6401f0d233db23eec1f2962ff796b69/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_unittest.cc
[modify] https://crrev.com/31325c45c6401f0d233db23eec1f2962ff796b69/components/cast_channel/cast_socket.cc
[modify] https://crrev.com/31325c45c6401f0d233db23eec1f2962ff796b69/components/cast_channel/cast_socket.h
[modify] https://crrev.com/31325c45c6401f0d233db23eec1f2962ff796b69/components/cast_channel/cast_socket_service.cc
[modify] https://crrev.com/31325c45c6401f0d233db23eec1f2962ff796b69/components/cast_channel/cast_socket_service.h
[modify] https://crrev.com/31325c45c6401f0d233db23eec1f2962ff796b69/components/cast_channel/cast_socket_service_unittest.cc
[modify] https://crrev.com/31325c45c6401f0d233db23eec1f2962ff796b69/components/cast_channel/cast_test_util.h
[modify] https://crrev.com/31325c45c6401f0d233db23eec1f2962ff796b69/extensions/browser/api/cast_channel/cast_channel_apitest.cc

Project Member

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

Project Member

Comment 18 by bugdroid1@chromium.org, Aug 3 2017

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

commit 031228fc986c15bee9c47e5053c34ebff560b5fd
Author: Bin Zhao <zhaobin@chromium.org>
Date: Thu Aug 03 17:59:35 2017

[Media Router] Add Cast device count metrics

- Moved device count metrics into its own class
- Added Cast device count metrics

Bug:  687377 
Change-Id: Id7ca7b7a529a79a8827889ad63f51b1da74db1db
Reviewed-on: https://chromium-review.googlesource.com/596615
Commit-Queue: Bin Zhao <zhaobin@chromium.org>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491789}
[modify] https://crrev.com/031228fc986c15bee9c47e5053c34ebff560b5fd/chrome/browser/media/router/discovery/BUILD.gn
[modify] https://crrev.com/031228fc986c15bee9c47e5053c34ebff560b5fd/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.cc
[modify] https://crrev.com/031228fc986c15bee9c47e5053c34ebff560b5fd/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.h
[add] https://crrev.com/031228fc986c15bee9c47e5053c34ebff560b5fd/chrome/browser/media/router/discovery/media_sink_discovery_metrics.cc
[add] https://crrev.com/031228fc986c15bee9c47e5053c34ebff560b5fd/chrome/browser/media/router/discovery/media_sink_discovery_metrics.h
[add] https://crrev.com/031228fc986c15bee9c47e5053c34ebff560b5fd/chrome/browser/media/router/discovery/media_sink_discovery_metrics_unittest.cc
[modify] https://crrev.com/031228fc986c15bee9c47e5053c34ebff560b5fd/chrome/browser/media/router/media_router_metrics.cc
[modify] https://crrev.com/031228fc986c15bee9c47e5053c34ebff560b5fd/chrome/browser/media/router/media_router_metrics.h
[modify] https://crrev.com/031228fc986c15bee9c47e5053c34ebff560b5fd/chrome/browser/media/router/media_router_metrics_unittest.cc
[modify] https://crrev.com/031228fc986c15bee9c47e5053c34ebff560b5fd/chrome/test/BUILD.gn

Project Member

Comment 19 by bugdroid1@chromium.org, Aug 4 2017

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

commit a1a521d1a5aacfe3c11d019ee729dcada785b89d
Author: Bin Zhao <zhaobin@chromium.org>
Date: Fri Aug 04 17:41:22 2017

[Media Router] Split out CastMediaSinkServiceImpl which only runs on the IO thread from CastMediaSinkService

Dual discovery implementation results in complicated thread hopping logic in CastMediaSinkService class. Seperate the parts running on IO only into a seperate class to simplify the code.

Bug:  687377 
Change-Id: I1d0b5422180356f3c982f5af3d0e86f5df362db0
Reviewed-on: https://chromium-review.googlesource.com/575247
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@{#492062}
[modify] https://crrev.com/a1a521d1a5aacfe3c11d019ee729dcada785b89d/chrome/browser/media/router/discovery/BUILD.gn
[modify] https://crrev.com/a1a521d1a5aacfe3c11d019ee729dcada785b89d/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.cc
[modify] https://crrev.com/a1a521d1a5aacfe3c11d019ee729dcada785b89d/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.h
[add] https://crrev.com/a1a521d1a5aacfe3c11d019ee729dcada785b89d/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.cc
[add] https://crrev.com/a1a521d1a5aacfe3c11d019ee729dcada785b89d/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.h
[add] https://crrev.com/a1a521d1a5aacfe3c11d019ee729dcada785b89d/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl_unittest.cc
[modify] https://crrev.com/a1a521d1a5aacfe3c11d019ee729dcada785b89d/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_unittest.cc
[modify] https://crrev.com/a1a521d1a5aacfe3c11d019ee729dcada785b89d/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
[modify] https://crrev.com/a1a521d1a5aacfe3c11d019ee729dcada785b89d/chrome/common/media_router/discovery/media_sink_internal.cc
[modify] https://crrev.com/a1a521d1a5aacfe3c11d019ee729dcada785b89d/chrome/common/media_router/discovery/media_sink_internal.h
[modify] https://crrev.com/a1a521d1a5aacfe3c11d019ee729dcada785b89d/chrome/test/BUILD.gn

Project Member

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

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

commit 30355a91b9b5e913df1edefc407857a84fadb4ad
Author: Bin Zhao <zhaobin@chromium.org>
Date: Tue Aug 08 20:10:19 2017

[Media Router] Record device count in CastMediaSinkServiceImpl

- Add MediaRouter.Cast.Discovery.ConnectedDeviceCount/KnownDevicesCount
  to histograms.xml
- record device count in CastMediaSinkServiceImpl

Bug:  687377 
Change-Id: Ic446c96078ea20957446085589a5c7a5bce9d703
Reviewed-on: https://chromium-review.googlesource.com/603008
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Commit-Queue: Bin Zhao <zhaobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492739}
[modify] https://crrev.com/30355a91b9b5e913df1edefc407857a84fadb4ad/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.cc
[modify] https://crrev.com/30355a91b9b5e913df1edefc407857a84fadb4ad/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.h
[modify] https://crrev.com/30355a91b9b5e913df1edefc407857a84fadb4ad/tools/metrics/histograms/histograms.xml

Project Member

Comment 21 by bugdroid1@chromium.org, Aug 9 2017

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

commit 7c4a262806a1df0c7a305f2d3243cbc45bcc77a2
Author: Bin Zhao <zhaobin@chromium.org>
Date: Wed Aug 09 23:25:24 2017

[Media Router] Support dual discovery

Porting from https://codereview.chromium.org/2965843002/

- Added DialMediaSinkServiceDelegate class to monitor OnDialSinkAdded()
and OnDialSinksRemoved()
- CastMediaSinkServiceImpl implements DialMediaSinkServiceDelegate
- MediaRouterMojoImpl registers CastMediaSinkService as a delegate of
DialMediaSinkService

Dual discovery process:
  DialMediaSinkServiceImpl::OnDeviceDescriptionAvailable()
    CastMediaSinkService::OnDialSinkAdded()
      CastMediaSinkServiceImpl::OnDialSinkAdded()
        CastMediaSinkServiceImpl::OpenChannel()
          ... async open channel
            CastMediaSinkServiceImpl::OnChannelOpened() if succeeded,
	    add cast_sink to |current_sinks_by_dial_map_|

CastMediaSinkServiceImpl::OnFetchCompleted() is invoked when timer expires.
It will merge sinks discovered by mDNS service and DIAL.

Bug:  687377 
Change-Id: Ie5e526ccdde36daa9f871f66ca84cd1553484a4d
Reviewed-on: https://chromium-review.googlesource.com/590510
Commit-Queue: Bin Zhao <zhaobin@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493174}
[modify] https://crrev.com/7c4a262806a1df0c7a305f2d3243cbc45bcc77a2/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.cc
[modify] https://crrev.com/7c4a262806a1df0c7a305f2d3243cbc45bcc77a2/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.h
[modify] https://crrev.com/7c4a262806a1df0c7a305f2d3243cbc45bcc77a2/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl_unittest.cc
[modify] https://crrev.com/7c4a262806a1df0c7a305f2d3243cbc45bcc77a2/chrome/browser/media/router/discovery/dial/dial_media_sink_service_proxy.cc
[modify] https://crrev.com/7c4a262806a1df0c7a305f2d3243cbc45bcc77a2/chrome/browser/media/router/discovery/dial/dial_media_sink_service_proxy.h
[modify] https://crrev.com/7c4a262806a1df0c7a305f2d3243cbc45bcc77a2/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.cc
[modify] https://crrev.com/7c4a262806a1df0c7a305f2d3243cbc45bcc77a2/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.h
[modify] https://crrev.com/7c4a262806a1df0c7a305f2d3243cbc45bcc77a2/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.cc
[modify] https://crrev.com/7c4a262806a1df0c7a305f2d3243cbc45bcc77a2/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.h
[modify] https://crrev.com/7c4a262806a1df0c7a305f2d3243cbc45bcc77a2/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl_unittest.cc
[modify] https://crrev.com/7c4a262806a1df0c7a305f2d3243cbc45bcc77a2/chrome/browser/media/router/discovery/media_sink_service_base.h
[modify] https://crrev.com/7c4a262806a1df0c7a305f2d3243cbc45bcc77a2/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
[modify] https://crrev.com/7c4a262806a1df0c7a305f2d3243cbc45bcc77a2/chrome/common/media_router/discovery/media_sink_internal.cc
[modify] https://crrev.com/7c4a262806a1df0c7a305f2d3243cbc45bcc77a2/chrome/common/media_router/discovery/media_sink_internal.h

Project Member

Comment 22 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 23 by bugdroid1@chromium.org, Aug 30 2017

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

commit 172fb063c63640e7a9aa635ba51daf4fe703c300
Author: Bin Zhao <zhaobin@chromium.org>
Date: Wed Aug 30 20:39:32 2017

[Media Router] Code cleanup: combine current sink maps in CastMediaSinkServiceImpl

Combine |current_sinks_by_mdns_| and |current_sinks_by_dial_| into a
single map |current_sinks_map_|.

To resolve code review comments:
https://chromium-review.googlesource.com/#/c/618062/1/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.h

Bug:  687377 
Change-Id: I6b9f235239964f9c121b7a46eb5a3e53ad7931cb
Reviewed-on: https://chromium-review.googlesource.com/636803
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Commit-Queue: Bin Zhao <zhaobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498597}
[modify] https://crrev.com/172fb063c63640e7a9aa635ba51daf4fe703c300/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.cc
[modify] https://crrev.com/172fb063c63640e7a9aa635ba51daf4fe703c300/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.h
[modify] https://crrev.com/172fb063c63640e7a9aa635ba51daf4fe703c300/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl_unittest.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Sep 7 2017

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

commit f144ed8ee752b61f0607e458ba1a6427219c021b
Author: Bin Zhao <zhaobin@chromium.org>
Date: Thu Sep 07 17:35:11 2017

[Media Router] Add retry logic in CastMediaSinkServiceImpl

- added a RetryStrategy class
- retry if fail to open cast channel
- retry if cast channel is opened, but ping timeout after some time
- updated unit tests

OpenChannel() opens channel with max 3 retry attempts. If succeeded, invoke
OnChannelOpenSucceeded(). If failed after all retry attempts, invoke
OnChannelOpenFailed().

Pick 3 max retry attempts and 5 seconds delay because retry delay in
extension seems a little aggresive (1.5s, 2.25s, 3.375s). It took about 5s
for my local debug version to open a socket. May make this configurable in
future.

Bug:  687377 
Change-Id: I427efb0718697aa5369a077c7a5a86f1b14c1b11
Reviewed-on: https://chromium-review.googlesource.com/618062
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@{#500328}
[modify] https://crrev.com/f144ed8ee752b61f0607e458ba1a6427219c021b/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.cc
[modify] https://crrev.com/f144ed8ee752b61f0607e458ba1a6427219c021b/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.h
[modify] https://crrev.com/f144ed8ee752b61f0607e458ba1a6427219c021b/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl_unittest.cc
[modify] https://crrev.com/f144ed8ee752b61f0607e458ba1a6427219c021b/components/cast_channel/cast_socket.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Sep 11 2017

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

commit f16ff2bae4d56b7ea078259c054e9343b9bf9c06
Author: Derek Cheng <imcheng@chromium.org>
Date: Mon Sep 11 20:54:48 2017

[Media Router] Fix thread issue with NetLog in CastMediaSinkServiceImpl.

g_browser_process can only be called from UI thread. This is a minor
modification to obtain the NetLog pointer in the ctor and then passing
it in during OpenChannel().


Bug:  687377 
Change-Id: I5243aa3d6b736b1456ae8a2364e17faabc956d1a
Reviewed-on: https://chromium-review.googlesource.com/651807
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501035}
[modify] https://crrev.com/f16ff2bae4d56b7ea078259c054e9343b9bf9c06/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.cc
[modify] https://crrev.com/f16ff2bae4d56b7ea078259c054e9343b9bf9c06/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.h

Project Member

Comment 26 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

Project Member

Comment 27 by bugdroid1@chromium.org, Oct 12 2017

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

commit 510378a69168ba01a1e2d9f95ac187446a975b01
Author: Bin Zhao <zhaobin@chromium.org>
Date: Thu Oct 12 19:43:39 2017

[Media Router] Send sinks to MRP when component extension reloads / resumes

If MR component extension reloads, we will find 0 device because browser side
(MediaSinkService) does not know extension reloads. Since there is no change
in sink list, MediaSinkService will not send updates to extension.

- Added MediaSinkServiceBase::SendSinksToMediaRouterProvider().
- Forced sending sinks to extension in MediaRouterDesktop::SyncStateToMediaRouteProvider().

Bug:  687377 
Change-Id: If7a0afeb113c7599e653c7cd3e333726969257ee
Reviewed-on: https://chromium-review.googlesource.com/708194
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Commit-Queue: Bin Zhao <zhaobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508382}
[modify] https://crrev.com/510378a69168ba01a1e2d9f95ac187446a975b01/chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.h
[modify] https://crrev.com/510378a69168ba01a1e2d9f95ac187446a975b01/chrome/browser/media/router/discovery/dial/dial_media_sink_service_proxy.cc
[modify] https://crrev.com/510378a69168ba01a1e2d9f95ac187446a975b01/chrome/browser/media/router/discovery/dial/dial_media_sink_service_proxy.h
[modify] https://crrev.com/510378a69168ba01a1e2d9f95ac187446a975b01/chrome/browser/media/router/discovery/dial/dial_media_sink_service_proxy_unittest.cc
[modify] https://crrev.com/510378a69168ba01a1e2d9f95ac187446a975b01/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.cc
[modify] https://crrev.com/510378a69168ba01a1e2d9f95ac187446a975b01/chrome/browser/media/router/discovery/mdns/cast_media_sink_service.h
[modify] https://crrev.com/510378a69168ba01a1e2d9f95ac187446a975b01/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_unittest.cc
[modify] https://crrev.com/510378a69168ba01a1e2d9f95ac187446a975b01/chrome/browser/media/router/discovery/media_sink_service_base.cc
[modify] https://crrev.com/510378a69168ba01a1e2d9f95ac187446a975b01/chrome/browser/media/router/discovery/media_sink_service_base.h
[modify] https://crrev.com/510378a69168ba01a1e2d9f95ac187446a975b01/chrome/browser/media/router/mojo/media_router_desktop.cc
[modify] https://crrev.com/510378a69168ba01a1e2d9f95ac187446a975b01/chrome/common/media_router/discovery/media_sink_service.h

Project Member

Comment 28 by bugdroid1@chromium.org, Oct 19 2017

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

commit 5a53bfcec2b456a70c8b54e45837bc8e1fa93673
Author: Bin Zhao <zhaobin@chromium.org>
Date: Thu Oct 19 00:27:57 2017

[Media Router] Move MediaRouterMojoImpl::ProvideSinks() to MediaRouterDesktop

ProvideSink() is desktop specific and should not be part of MediaRouter interface.

Resolve code review comments
https://chromium-review.googlesource.com/c/chromium/src/+/708194/1/chrome/browser/media/router/mojo/media_router_desktop.cc#150

Bug:  687377 
Change-Id: I943a8fee32575fea92973707892cc60a01986178
Reviewed-on: https://chromium-review.googlesource.com/714819
Commit-Queue: Bin Zhao <zhaobin@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509942}
[modify] https://crrev.com/5a53bfcec2b456a70c8b54e45837bc8e1fa93673/chrome/browser/media/android/router/media_router_android.cc
[modify] https://crrev.com/5a53bfcec2b456a70c8b54e45837bc8e1fa93673/chrome/browser/media/android/router/media_router_android.h
[modify] https://crrev.com/5a53bfcec2b456a70c8b54e45837bc8e1fa93673/chrome/browser/media/router/media_router.h
[modify] https://crrev.com/5a53bfcec2b456a70c8b54e45837bc8e1fa93673/chrome/browser/media/router/mojo/media_router_desktop.cc
[modify] https://crrev.com/5a53bfcec2b456a70c8b54e45837bc8e1fa93673/chrome/browser/media/router/mojo/media_router_desktop.h
[modify] https://crrev.com/5a53bfcec2b456a70c8b54e45837bc8e1fa93673/chrome/browser/media/router/mojo/media_router_desktop_unittest.cc
[modify] https://crrev.com/5a53bfcec2b456a70c8b54e45837bc8e1fa93673/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
[modify] https://crrev.com/5a53bfcec2b456a70c8b54e45837bc8e1fa93673/chrome/browser/media/router/mojo/media_router_mojo_impl.h
[modify] https://crrev.com/5a53bfcec2b456a70c8b54e45837bc8e1fa93673/chrome/browser/media/router/mojo/media_router_mojo_impl_unittest.cc
[modify] https://crrev.com/5a53bfcec2b456a70c8b54e45837bc8e1fa93673/chrome/browser/media/router/mojo/media_router_mojo_test.cc
[modify] https://crrev.com/5a53bfcec2b456a70c8b54e45837bc8e1fa93673/chrome/browser/media/router/mojo/media_router_mojo_test.h

Blocking: -687383
Blocking: 752604
Components: -Blink>PresentationAPI Internals>Cast>API
Owner: zhaobin@chromium.org
Status: Assigned (was: Available)
Can this be closed?
Blocking: -752604
No longer blocking bug 752604, but still need to implement sink queries.
Status: Fixed (was: Assigned)
Actually, closing this in favor of 698940 which contains some more info on implementing queries.

Sign in to add a comment