[Cast MRP] Fixed "source not supported" for local file casting.

This CL fixes an issue where all Cast devices show "source not
supported" after selecting a local file to cast.

Bug: 1078448, b/155777239
Change-Id: Ia71a0cca55c45ed9578e7d49ffccef7333b3319e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2210634Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Commit-Queue: John Williams <jrw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#775148}

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

@@ -289,7 +289,7 @@ void MediaRouterMojoImpl::CreateRoute(const MediaSource::Id& source_id,
   }
 
   const MediaSource source(source_id);
-  if (source.IsTabMirroringSource()) {
+  if (source.IsTabMirroringSource() || source.IsLocalFileSource()) {
     // Ensure the CastRemotingConnector is created before mirroring starts.
     CastRemotingConnector* const connector =
         CastRemotingConnector::Get(web_contents);
@@ -659,6 +659,8 @@ bool MediaRouterMojoImpl::RegisterMediaSinksObserver(
   if (is_new_query) {
     for (const auto& provider : media_route_providers_) {
       if (sink_availability_.IsAvailableForProvider(provider.first)) {
+        // TODO(crbug.com/1090890): Don't allow MediaSource::ForAnyTab().id() to
+        // be passed here.
         provider.second->StartObservingMediaSinks(source.id());
       }
     }
@@ -687,6 +689,8 @@ void MediaRouterMojoImpl::UnregisterMediaSinksObserver(
     // Otherwise, the MRPs would have discarded the queries already.
     for (const auto& provider : media_route_providers_) {
       if (sink_availability_.IsAvailableForProvider(provider.first)) {
+        // TODO(crbug.com/1090890): Don't allow MediaSource::ForAnyTab().id() to
+        // be passed here.
         provider.second->StopObservingMediaSinks(source.id());
       }
     }
@@ -839,8 +843,11 @@ void MediaRouterMojoImpl::OnSinkAvailabilityUpdated(
   if (availability != SinkAvailability::UNAVAILABLE) {
     // Sinks are now available. Tell the MRP 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_) {
+      // TODO(crbug.com/1090890): Don't allow MediaSource::ForAnyTab().id() to
+      // be passed here.
       provider->StartObservingMediaSinks(source_and_query.first);
+    }
   } else if (!sink_availability_.IsAvailable()) {
     // Sinks are no longer available. MRPs have already removed all sink
     // queries.
@@ -886,9 +893,12 @@ void MediaRouterMojoImpl::SyncStateToMediaRouteProvider(
   const auto& provider = media_route_providers_[provider_id];
   // Sink queries.
   if (sink_availability_.IsAvailableForProvider(provider_id)) {
-    for (const auto& it : sinks_queries_)
+    for (const auto& it : sinks_queries_) {
+      // TODO(crbug.com/1090890): Don't allow MediaSource::ForAnyTab().id() to
+      // be passed here.
       provider->StartObservingMediaSinks(it.first);
     }
+  }
 
   // Route queries.
   for (const auto& it : routes_queries_)

chrome/browser/media/router/providers/cast/cast_media_route_provider.cc

@@ -87,7 +87,10 @@ void CastMediaRouteProvider::Init(
 
 CastMediaRouteProvider::~CastMediaRouteProvider() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(sink_queries_.empty());
+  if (!sink_queries_.empty()) {
+    DCHECK_EQ(sink_queries_.size(), 1u);
+    DCHECK_EQ(sink_queries_.begin()->first, MediaSource::ForAnyTab().id());
+  }
 }
 
 void CastMediaRouteProvider::CreateRoute(const std::string& source_id,

chrome/browser/media/router/providers/cast/mirroring_activity_record.cc

@@ -82,7 +82,7 @@ base::Optional<MirroringActivityRecord::MirroringType> GetMirroringType(
     return base::nullopt;
 
   const auto source = route.media_source();
-  if (source.IsTabMirroringSource())
+  if (source.IsTabMirroringSource() || source.IsLocalFileSource())
     return MirroringActivityRecord::MirroringType::kTab;
   if (source.IsDesktopMirroringSource())
     return MirroringActivityRecord::MirroringType::kDesktop;

chrome/browser/ui/views/media_router/media_router_views_ui.cc

@@ -501,7 +501,7 @@ void MediaRouterViewsUI::InitCommon() {
 
   // File mirroring is always available.
   query_result_manager_->SetSourcesForCastMode(
-      MediaCastMode::LOCAL_FILE, {MediaSource::ForTab(0)}, origin);
+      MediaCastMode::LOCAL_FILE, {MediaSource::ForLocalFile()}, origin);
 
   SessionID::id_type tab_id =
       sessions::SessionTabHelper::IdForTab(initiator_).id();

chrome/common/media_router/media_source.cc

@@ -68,6 +68,20 @@ MediaSource::MediaSource(const GURL& presentation_url)
 
 MediaSource::~MediaSource() = default;
 
+// static
+MediaSource MediaSource::ForLocalFile() {
+  // TODO(crbug.com/1090878): Use something more sane here.  Fixing this
+  // requires tracking down other places where tab ID 0 is used to indicate
+  // local file casting.
+  //
+  // This probably isn't a source of bugs in practice, because tab IDs are
+  // generated by SessionIdGenerator, which appears to only produce positive
+  // values, but that fact isn't clearly documentated, and other parts of
+  // Chromium don't seem to rely on it, using -1 as the canonical invalid tab
+  // ID.
+  return MediaSource(base::StringPrintf(kTabMediaUrnFormat, 0));
+}
+
 // static
 MediaSource MediaSource::ForAnyTab() {
   return MediaSource(std::string(kAnyTabMediaUrn));
@@ -75,11 +89,17 @@ MediaSource MediaSource::ForAnyTab() {
 
 // static
 MediaSource MediaSource::ForTab(int tab_id) {
+  // Ideally we shouldn't allow -1 as a tab ID, but in unit tests, a tab ID of
+  // -1 can show up when this function is called from
+  // CastHandler::StartObservingForSinks() because SessionTabHelper::IdForTab
+  // can return -1.
+  DCHECK_GE(tab_id, -1);
   return MediaSource(base::StringPrintf(kTabMediaUrnFormat, tab_id));
 }
 
 // static
 MediaSource MediaSource::ForDesktop(const std::string& desktop_media_id) {
+  DCHECK(!desktop_media_id.empty());
   return MediaSource(kDesktopMediaUrnPrefix.as_string() + desktop_media_id);
 }
 
@@ -94,10 +114,7 @@ MediaSource MediaSource::ForPresentationUrl(const GURL& presentation_url) {
 }
 
 bool MediaSource::IsTabMirroringSource() const {
-  int tab_id;
-  return id() == kAnyTabMediaUrn ||
-         (std::sscanf(id().c_str(), kTabMediaUrnFormat, &tab_id) == 1 &&
-          tab_id > 0);
+  return id() == kAnyTabMediaUrn || TabId() > 0;
 }
 
 bool MediaSource::IsDesktopMirroringSource() const {
@@ -106,8 +123,9 @@ bool MediaSource::IsDesktopMirroringSource() const {
                           base::CompareCase::SENSITIVE);
 }
 
-bool MediaSource::IsMirroringSource() const {
-  return IsDesktopMirroringSource() || IsTabMirroringSource();
+bool MediaSource::IsLocalFileSource() const {
+  // TODO(crbug.com/1090878): Keep this method is sync with ForLocalFile().
+  return TabId() == 0;
 }
 
 bool MediaSource::IsCastPresentationUrl() const {
@@ -116,11 +134,9 @@ bool MediaSource::IsCastPresentationUrl() const {
 }
 
 int MediaSource::TabId() const {
-  int tab_id;
-  if (sscanf(id_.c_str(), kTabMediaUrnFormat, &tab_id) == 1)
+  int tab_id = -1;
+  sscanf(id_.c_str(), kTabMediaUrnFormat, &tab_id);
   return tab_id;
-  else
-    return -1;
 }
 
 base::Optional<std::string> MediaSource::DesktopStreamId() const {
@@ -131,11 +147,6 @@ base::Optional<std::string> MediaSource::DesktopStreamId() const {
   return base::nullopt;
 }
 
-bool MediaSource::IsValid() const {
-  return IsTabMirroringSource() || IsDesktopMirroringSource() ||
-         IsValidPresentationUrl(GURL(id_));
-}
-
 bool MediaSource::IsDialSource() const {
   return url_.SchemeIs(kCastDialPresentationUrlScheme);
 }

chrome/common/media_router/media_source.h

@@ -90,6 +90,7 @@ class MediaSource {
 
   // Protocol-specific media source object creation.
   // Returns MediaSource URI depending on the type of source.
+  static MediaSource ForLocalFile();
   static MediaSource ForAnyTab();
   static MediaSource ForTab(int tab_id);
   static MediaSource ForPresentationUrl(const GURL& presentation_url);
@@ -108,15 +109,23 @@ class MediaSource {
   // extension-based Cast MRP is removed.
   static MediaSource ForDesktop();
 
+  // Returns true if source outputs its content via tab mirroring and isn't a
+  // local file.
   bool IsTabMirroringSource() const;
+
+  // Returns true if source outputs its content via desktop mirroring.
   bool IsDesktopMirroringSource() const;
-  bool IsMirroringSource() const;
+
+  // Returns true if the source is a local file.
+  bool IsLocalFileSource() const;
 
   // Returns true if this is represents a Cast Presentation URL.
   bool IsCastPresentationUrl() const;
 
   // Parses the ID and returns the SessionTabHelper tab ID referencing a source
-  // tab. Returns a non-positive value on error.
+  // tab.  Don't rely on this method returning something useful without first
+  // calling IsTabMirroringSource(); it will return 0 for for ForLocalFile()
+  // source and -1 for non-tab sources or the ForAnyTab() source.
   int TabId() const;
 
   // When this source was created by ForDesktop(string), returns a stream ID
@@ -125,10 +134,6 @@ class MediaSource {
   // returns base::nullopt.
   base::Optional<std::string> DesktopStreamId() const;
 
-  // Checks that this is a parseable URN and is of a known type.
-  // Does not deeper protocol-level syntax checks.
-  bool IsValid() const;
-
   // Returns true this source outputs its content via DIAL.
   // TODO(crbug.com/804419): Move this to in-browser DIAL/Cast MRP when we have
   // one.

chrome/common/media_router/media_source_unittest.cc

@@ -58,10 +58,9 @@ TEST(MediaSourceTest, ForAnyTab) {
   auto source = MediaSource::ForAnyTab();
   EXPECT_EQ("urn:x-org.chromium.media:source:tab:*", source.id());
   EXPECT_EQ(-1, source.TabId());
-  EXPECT_TRUE(source.IsValid());
   EXPECT_FALSE(source.IsDesktopMirroringSource());
   EXPECT_TRUE(source.IsTabMirroringSource());
-  EXPECT_TRUE(source.IsMirroringSource());
+  EXPECT_FALSE(source.IsLocalFileSource());
   EXPECT_FALSE(source.IsCastPresentationUrl());
   EXPECT_FALSE(source.IsDialSource());
 }
@@ -70,10 +69,19 @@ TEST(MediaSourceTest, ForTab) {
   auto source = MediaSource::ForTab(123);
   EXPECT_EQ("urn:x-org.chromium.media:source:tab:123", source.id());
   EXPECT_EQ(123, source.TabId());
-  EXPECT_TRUE(source.IsValid());
   EXPECT_FALSE(source.IsDesktopMirroringSource());
   EXPECT_TRUE(source.IsTabMirroringSource());
-  EXPECT_TRUE(source.IsMirroringSource());
+  EXPECT_FALSE(source.IsLocalFileSource());
+  EXPECT_FALSE(source.IsCastPresentationUrl());
+  EXPECT_FALSE(source.IsDialSource());
+}
+
+TEST(MediaSourceTest, ForLocalFile) {
+  auto source = MediaSource::ForLocalFile();
+  EXPECT_EQ("urn:x-org.chromium.media:source:tab:0", source.id());
+  EXPECT_FALSE(source.IsDesktopMirroringSource());
+  EXPECT_FALSE(source.IsTabMirroringSource());
+  EXPECT_TRUE(source.IsLocalFileSource());
   EXPECT_FALSE(source.IsCastPresentationUrl());
   EXPECT_FALSE(source.IsDialSource());
 }
@@ -82,10 +90,9 @@ TEST(MediaSourceTest, ForDesktop) {
   std::string media_id = "fakeMediaId";
   auto source = MediaSource::ForDesktop(media_id);
   EXPECT_EQ("urn:x-org.chromium.media:source:desktop:" + media_id, source.id());
-  EXPECT_TRUE(source.IsValid());
   EXPECT_TRUE(source.IsDesktopMirroringSource());
   EXPECT_FALSE(source.IsTabMirroringSource());
-  EXPECT_TRUE(source.IsMirroringSource());
+  EXPECT_FALSE(source.IsLocalFileSource());
   EXPECT_FALSE(source.IsCastPresentationUrl());
   EXPECT_FALSE(source.IsDialSource());
 }
@@ -95,23 +102,13 @@ TEST(MediaSourceTest, ForPresentationUrl) {
       "https://www.example.com/presentation.html";
   auto source = MediaSource::ForPresentationUrl(GURL(kPresentationUrl));
   EXPECT_EQ(kPresentationUrl, source.id());
-  EXPECT_TRUE(source.IsValid());
   EXPECT_FALSE(source.IsDesktopMirroringSource());
   EXPECT_FALSE(source.IsTabMirroringSource());
-  EXPECT_FALSE(source.IsMirroringSource());
+  EXPECT_FALSE(source.IsLocalFileSource());
   EXPECT_FALSE(source.IsCastPresentationUrl());
   EXPECT_FALSE(source.IsDialSource());
 }
 
-TEST(MediaSourceTest, IsValid) {
-  // Disallowed scheme
-  EXPECT_FALSE(MediaSource::ForPresentationUrl(GURL("file:///some/local/path"))
-                   .IsValid());
-  // Not a URL
-  EXPECT_FALSE(
-      MediaSource::ForPresentationUrl(GURL("totally not a url")).IsValid());
-}
-
 TEST(MediaSourceTest, IsCastPresentationUrl) {
   EXPECT_TRUE(MediaSource(GURL("cast:233637DE")).IsCastPresentationUrl());
   EXPECT_TRUE(

chrome/common/media_router/providers/cast/cast_media_source.cc

@@ -353,7 +353,7 @@ CastAppInfo::CastAppInfo(const CastAppInfo& other) = default;
 // static
 std::unique_ptr<CastMediaSource> CastMediaSource::FromMediaSource(
     const MediaSource& source) {
-  if (source.IsTabMirroringSource())
+  if (source.IsTabMirroringSource() || source.IsLocalFileSource())
     return CastMediaSourceForTabMirroring(source.id());
 
   if (source.IsDesktopMirroringSource())