[Media Router] Add MediaSink subtypes

chrome/browser/media/router/mojo/media_router_mojo_impl.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/media/router/mojo/media_router_mojo_impl.h"
 
 #include <stddef.h>
 
 #include <utility>
 
@@ skipping 157 matching lines @@
 }
 
 void MediaRouterMojoImpl::OnIssue(const IssueInfo& issue) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DVLOG_WITH_INSTANCE(1) << "OnIssue " << issue.title;
   issue_manager_.AddIssue(issue);
 }
 
 void MediaRouterMojoImpl::OnSinksReceived(
     const std::string& media_source,
-    std::vector<mojom::MediaSinkPtr> sinks,
+    std::vector<std::unique_ptr<MediaSink>> sinks,
     const std::vector<std::string>& origins) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DVLOG_WITH_INSTANCE(1) << "OnSinksReceived";
   auto it = sinks_queries_.find(media_source);
   if (it == sinks_queries_.end()) {
     DVLOG_WITH_INSTANCE(1) << "Received sink list without MediaSinksQuery.";
     return;
   }
 
   std::vector<GURL> origin_list;
   origin_list.reserve(origins.size());
   for (size_t i = 0; i < origins.size(); ++i) {
     GURL origin(origins[i]);
     if (!origin.is_valid()) {
       LOG(WARNING) << "Received invalid origin: " << origin
                    << ". Dropping result.";
       return;
     }
     origin_list.push_back(origin);
   }
 
   std::vector<MediaSink> sink_list;
   sink_list.reserve(sinks.size());
   for (size_t i = 0; i < sinks.size(); ++i)
-    sink_list.push_back(sinks[i].To<MediaSink>());
+    sink_list.push_back(*sinks[i]);

[imcheng, 2017/02/08 01:28:09]

In here we are performing object slicing on derived MediaSink instances. In
practice this is fine, provided we don't downcast MediaSinks outside of
media_router_struct_traits. But it does create an inconsistency with the API
that pushes browser-discovered sinks, which we cannot slice, to MR extension. To
resolve this we would have to convert the MediaSinksObserver API to use
std::vector<std::unique_ptr<MediaSink>>.

I wonder if derived classes are truly useful enough to worth the trouble. The
alternative is to use a property map on MediaSink to store DIAL / Cast specific
fields, at the cost of losing strong type safety. Mark, WDYT? 

[zhaobin, 2017/02/09 00:13:53]

Acknowledged.

[imcheng, 2017/02/09 02:19:57]

So this is probably fine since we don't need to (or want to) expose any data
beyond what's in the generic MediaSink to consumers. We would have to be clear
in the documentation that the type field and the MediaSink subclasses are only
used by MediaSinkService and in Mojo conversion, or make sure we don't create
MediaSink instances that is not GENERIC - see comment in media_sink.h.

 
   auto* sinks_query = it->second.get();
   sinks_query->has_cached_result = true;
   sinks_query->origins.swap(origin_list);
   sinks_query->cached_sink_list.swap(sink_list);
 
   if (!sinks_query->observers.might_have_observers()) {
     DVLOG_WITH_INSTANCE(1)
         << "Received sink list without any active observers: " << media_source;
   } else {
@@ skipping 773 matching lines @@
                         base::Unretained(this), source_id));
 }
 
 void MediaRouterMojoImpl::DoUpdateMediaSinks(
     const MediaSource::Id& source_id) {
   DVLOG_WITH_INSTANCE(1) << "DoUpdateMediaSinks" << source_id;
   media_route_provider_->UpdateMediaSinks(source_id);
 }
 
 }  // namespace media_router