Commit a5e10914 authored by Thomas Anderson's avatar Thomas Anderson Committed by Commit Bot

Revert "[Media Router] Better sink availability tracking"

This reverts commit 52dda118.

Reason for revert: Breaks Linux x64 build:
https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium%2FLinux_x64%2F54249%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

Original change's description:
> [Media Router] Better sink availability tracking
> 
> - When sink availability changes for an MRP, start/stop observing sinks only for that MRP
> - Make WiredDisplayMRP report sink availability updates
> 
> Bug: 789277,777650
> Change-Id: I3d1a087ca39a4830403d6d2f0f00bf1411388f33
> Reviewed-on: https://chromium-review.googlesource.com/794106
> Reviewed-by: Derek Cheng <imcheng@chromium.org>
> Commit-Queue: Takumi Fujimoto <takumif@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#521550}

TBR=imcheng@chromium.org,takumif@chromium.org

Change-Id: Id4d13f0b2097ccfe398b05ce239d21c5d3c2b626
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 789277, 777650
Reviewed-on: https://chromium-review.googlesource.com/807588Reviewed-by: default avatarThomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521561}
parent 457e73a6
...@@ -538,14 +538,6 @@ bool MediaRouterMojoImpl::ProviderSinkAvailability::SetAvailabilityForProvider( ...@@ -538,14 +538,6 @@ bool MediaRouterMojoImpl::ProviderSinkAvailability::SetAvailabilityForProvider(
} }
} }
bool MediaRouterMojoImpl::ProviderSinkAvailability::IsAvailableForProvider(
mojom::MediaRouteProvider::Id provider_id) const {
const auto& it = availabilities_.find(provider_id);
return it == availabilities_.end()
? false
: it->second != SinkAvailability::UNAVAILABLE;
}
bool MediaRouterMojoImpl::ProviderSinkAvailability::IsAvailable() const { bool MediaRouterMojoImpl::ProviderSinkAvailability::IsAvailable() const {
return overall_availability_ != SinkAvailability::UNAVAILABLE; return overall_availability_ != SinkAvailability::UNAVAILABLE;
} }
...@@ -586,11 +578,9 @@ bool MediaRouterMojoImpl::RegisterMediaSinksObserver( ...@@ -586,11 +578,9 @@ bool MediaRouterMojoImpl::RegisterMediaSinksObserver(
// If sink availability is UNAVAILABLE or the query isn't new, then there is // If sink availability is UNAVAILABLE or the query isn't new, then there is
// no need to call MRPs. // no need to call MRPs.
if (is_new_query) { if (availability_.IsAvailable() && is_new_query) {
for (const auto& provider : media_route_providers_) { for (const auto& provider : media_route_providers_)
if (sink_availability_.IsAvailableForProvider(provider.first)) provider.second->StartObservingMediaSinks(source_id);
provider.second->StartObservingMediaSinks(source_id);
}
} }
return true; return true;
} }
...@@ -601,8 +591,9 @@ void MediaRouterMojoImpl::UnregisterMediaSinksObserver( ...@@ -601,8 +591,9 @@ void MediaRouterMojoImpl::UnregisterMediaSinksObserver(
const MediaSource::Id& source_id = observer->source().id(); const MediaSource::Id& source_id = observer->source().id();
auto it = sinks_queries_.find(source_id); auto it = sinks_queries_.find(source_id);
if (it == sinks_queries_.end() || !it->second->HasObserver(observer)) if (it == sinks_queries_.end() || !it->second->HasObserver(observer)) {
return; return;
}
// If we are removing the final observer for the source, then stop // If we are removing the final observer for the source, then stop
// observing sinks for it. // observing sinks for it.
...@@ -612,8 +603,8 @@ void MediaRouterMojoImpl::UnregisterMediaSinksObserver( ...@@ -612,8 +603,8 @@ void MediaRouterMojoImpl::UnregisterMediaSinksObserver(
if (!it->second->HasObservers()) { if (!it->second->HasObservers()) {
// Only ask MRPs to stop observing media sinks if there are sinks available. // Only ask MRPs to stop observing media sinks if there are sinks available.
// Otherwise, the MRPs would have discarded the queries already. // Otherwise, the MRPs would have discarded the queries already.
for (const auto& provider : media_route_providers_) { if (availability_.IsAvailable()) {
if (sink_availability_.IsAvailableForProvider(provider.first)) for (const auto& provider : media_route_providers_)
provider.second->StopObservingMediaSinks(source_id); provider.second->StopObservingMediaSinks(source_id);
} }
sinks_queries_.erase(source_id); sinks_queries_.erase(source_id);
...@@ -762,15 +753,16 @@ void MediaRouterMojoImpl::OnRouteMessagesReceived( ...@@ -762,15 +753,16 @@ void MediaRouterMojoImpl::OnRouteMessagesReceived(
void MediaRouterMojoImpl::OnSinkAvailabilityUpdated( void MediaRouterMojoImpl::OnSinkAvailabilityUpdated(
MediaRouteProviderId provider_id, MediaRouteProviderId provider_id,
SinkAvailability availability) { SinkAvailability availability) {
if (!sink_availability_.SetAvailabilityForProvider(provider_id, availability)) if (!availability_.SetAvailabilityForProvider(provider_id, availability))
return; return;
if (availability != SinkAvailability::UNAVAILABLE) { if (availability_.IsAvailable()) {
// Sinks are now available. Tell the MRP to start all sink queries again. // Sinks are now available. Tell MRPs to start all sink queries again.
auto& provider = media_route_providers_[provider_id]; for (const auto& source_and_query : sinks_queries_) {
for (const auto& source_and_query : sinks_queries_) for (const auto& provider : media_route_providers_)
provider->StartObservingMediaSinks(source_and_query.first); provider.second->StartObservingMediaSinks(source_and_query.first);
} else if (!sink_availability_.IsAvailable()) { }
} else {
// Sinks are no longer available. MRPs have already removed all sink // Sinks are no longer available. MRPs have already removed all sink
// queries. // queries.
for (auto& source_and_query : sinks_queries_) for (auto& source_and_query : sinks_queries_)
...@@ -815,7 +807,7 @@ void MediaRouterMojoImpl::SyncStateToMediaRouteProvider( ...@@ -815,7 +807,7 @@ void MediaRouterMojoImpl::SyncStateToMediaRouteProvider(
MediaRouteProviderId provider_id) { MediaRouteProviderId provider_id) {
const auto& provider = media_route_providers_[provider_id]; const auto& provider = media_route_providers_[provider_id];
// Sink queries. // Sink queries.
if (sink_availability_.IsAvailableForProvider(provider_id)) { if (availability_.IsAvailable()) {
for (const auto& it : sinks_queries_) for (const auto& it : sinks_queries_)
provider->StartObservingMediaSinks(it.first); provider->StartObservingMediaSinks(it.first);
} }
......
...@@ -296,10 +296,6 @@ class MediaRouterMojoImpl : public MediaRouterBase, ...@@ -296,10 +296,6 @@ class MediaRouterMojoImpl : public MediaRouterBase,
bool SetAvailabilityForProvider(MediaRouteProviderId provider_id, bool SetAvailabilityForProvider(MediaRouteProviderId provider_id,
SinkAvailability availability); SinkAvailability availability);
// Returns true if the availability for the provider is not UNAVAILABLE.
bool IsAvailableForProvider(
mojom::MediaRouteProvider::Id provider_id) const;
// Returns true if there is a provider whose sink availability isn't // Returns true if there is a provider whose sink availability isn't
// UNAVAILABLE. // UNAVAILABLE.
bool IsAvailable() const; bool IsAvailable() const;
...@@ -411,7 +407,7 @@ class MediaRouterMojoImpl : public MediaRouterBase, ...@@ -411,7 +407,7 @@ class MediaRouterMojoImpl : public MediaRouterBase,
std::string instance_id_; std::string instance_id_;
// The last reported sink availability from the media route providers. // The last reported sink availability from the media route providers.
ProviderSinkAvailability sink_availability_; ProviderSinkAvailability availability_;
// Bindings for Mojo pointers to |this| held by media route providers. // Bindings for Mojo pointers to |this| held by media route providers.
mojo::BindingSet<mojom::MediaRouter> bindings_; mojo::BindingSet<mojom::MediaRouter> bindings_;
......
...@@ -68,7 +68,6 @@ WiredDisplayMediaRouteProvider::WiredDisplayMediaRouteProvider( ...@@ -68,7 +68,6 @@ WiredDisplayMediaRouteProvider::WiredDisplayMediaRouteProvider(
: binding_(this, std::move(request)), : binding_(this, std::move(request)),
media_router_(std::move(media_router)) { media_router_(std::move(media_router)) {
display::Screen::GetScreen()->AddObserver(this); display::Screen::GetScreen()->AddObserver(this);
ReportSinkAvailability(GetSinks());
} }
WiredDisplayMediaRouteProvider::~WiredDisplayMediaRouteProvider() { WiredDisplayMediaRouteProvider::~WiredDisplayMediaRouteProvider() {
...@@ -265,7 +264,6 @@ void WiredDisplayMediaRouteProvider::NotifyRouteObservers() const { ...@@ -265,7 +264,6 @@ void WiredDisplayMediaRouteProvider::NotifyRouteObservers() const {
void WiredDisplayMediaRouteProvider::NotifySinkObservers() { void WiredDisplayMediaRouteProvider::NotifySinkObservers() {
std::vector<MediaSinkInternal> sinks = GetSinks(); std::vector<MediaSinkInternal> sinks = GetSinks();
device_count_metrics_.RecordDeviceCountsIfNeeded(sinks.size(), sinks.size()); device_count_metrics_.RecordDeviceCountsIfNeeded(sinks.size(), sinks.size());
ReportSinkAvailability(sinks);
for (const auto& sink_query : sink_queries_) for (const auto& sink_query : sink_queries_)
media_router_->OnSinksReceived(kProviderId, sink_query, sinks, {}); media_router_->OnSinksReceived(kProviderId, sink_query, sinks, {});
} }
...@@ -292,12 +290,4 @@ std::vector<Display> WiredDisplayMediaRouteProvider::GetAvailableDisplays() ...@@ -292,12 +290,4 @@ std::vector<Display> WiredDisplayMediaRouteProvider::GetAvailableDisplays()
return displays; return displays;
} }
void WiredDisplayMediaRouteProvider::ReportSinkAvailability(
const std::vector<MediaSinkInternal>& sinks) {
mojom::MediaRouter::SinkAvailability sink_availability =
sinks.empty() ? mojom::MediaRouter::SinkAvailability::UNAVAILABLE
: mojom::MediaRouter::SinkAvailability::PER_SOURCE;
media_router_->OnSinkAvailabilityUpdated(kProviderId, sink_availability);
}
} // namespace media_router } // namespace media_router
...@@ -106,9 +106,6 @@ class WiredDisplayMediaRouteProvider : public mojom::MediaRouteProvider, ...@@ -106,9 +106,6 @@ class WiredDisplayMediaRouteProvider : public mojom::MediaRouteProvider,
// Sends the current list of sinks to each query in |sink_queries_|. // Sends the current list of sinks to each query in |sink_queries_|.
void NotifySinkObservers(); void NotifySinkObservers();
// Notifies |media_router_| of the current sink availability.
void ReportSinkAvailability(const std::vector<MediaSinkInternal>& sinks);
// Returns a list of available sinks. A display can be a sink if it is // Returns a list of available sinks. A display can be a sink if it is
// secondary and does not mirror a primary display. // secondary and does not mirror a primary display.
std::vector<MediaSinkInternal> GetSinks() const; std::vector<MediaSinkInternal> GetSinks() const;
......
...@@ -157,41 +157,6 @@ TEST_F(WiredDisplayMediaRouteProviderTest, GetDisplaysAsSinks) { ...@@ -157,41 +157,6 @@ TEST_F(WiredDisplayMediaRouteProviderTest, GetDisplaysAsSinks) {
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
TEST_F(WiredDisplayMediaRouteProviderTest, NotifyOnDisplayChange) {
const std::string sink_id1 = GetSinkId(sink_display1_);
provider_pointer_->StartObservingMediaSinks(kPresentationSource);
base::RunLoop().RunUntilIdle();
// Add an external display. MediaRouter should be notified of the sink and the
// sink availability change.
provider_->set_all_displays({primary_display_, sink_display1_});
EXPECT_CALL(router_, OnSinkAvailabilityUpdated(
mojom::MediaRouteProvider::Id::WIRED_DISPLAY,
mojom::MediaRouter::SinkAvailability::PER_SOURCE));
EXPECT_CALL(
router_,
OnSinksReceived(mojom::MediaRouteProvider::Id::WIRED_DISPLAY, _, _, _))
.WillOnce(WithArg<2>(
Invoke([&sink_id1](const std::vector<MediaSinkInternal>& sinks) {
EXPECT_EQ(sinks.size(), 1u);
EXPECT_EQ(sinks[0].sink().id(), sink_id1);
})));
provider_->OnDisplayAdded(sink_display1_);
base::RunLoop().RunUntilIdle();
// Remove the external display. MediaRouter should be notified of the lack of
// sinks.
provider_->set_all_displays({primary_display_});
EXPECT_CALL(router_, OnSinkAvailabilityUpdated(
mojom::MediaRouteProvider::Id::WIRED_DISPLAY,
mojom::MediaRouter::SinkAvailability::UNAVAILABLE));
EXPECT_CALL(router_,
OnSinksReceived(mojom::MediaRouteProvider::Id::WIRED_DISPLAY, _,
IsEmpty(), _));
provider_->OnDisplayRemoved(sink_display1_);
base::RunLoop().RunUntilIdle();
}
TEST_F(WiredDisplayMediaRouteProviderTest, NoSinksForNonPresentationSource) { TEST_F(WiredDisplayMediaRouteProviderTest, NoSinksForNonPresentationSource) {
EXPECT_CALL(router_, EXPECT_CALL(router_,
OnSinksReceived(kProviderId, kNonPresentationSource, _, _)) OnSinksReceived(kProviderId, kNonPresentationSource, _, _))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment