[Media Router] Fixed problems with dummy notifications.

This change fixes two related problems, one causing notifications to
appear when they shouldn't, and another causing a notification to
appear in the media dialog after it has been deleted, causing a crash
if it is clicked.

Change-Id: Ia78ae95af2afe6d1024e86c12ebacdf0720d84cb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2424629Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Commit-Queue: John Williams <jrw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813964}

chrome/browser/ui/global_media_controls/media_notification_service.cc

@@ -629,8 +629,10 @@ void MediaNotificationService::Shutdown() {
 void MediaNotificationService::AddSupplementalNotification(
     const std::string& id,
     content::WebContents* web_contents) {
+  DCHECK(web_contents);
   supplemental_notifications_.emplace(id, web_contents);
-  ShowNotification(id);
+  if (!HasSessionForWebContents(web_contents))
+    ShowNotification(id);
 }
 
 void MediaNotificationService::OnOverlayNotificationClosed(
@@ -872,7 +874,7 @@ void MediaNotificationService::OnNotificationChanged(
   for (auto& observer : observers_)
     observer.OnNotificationListChanged();
 
-  // Avoid re-examining the supplemental notifications as a side-effect or
+  // Avoid re-examining the supplemental notifications as a side-effect of
   // showing or hiding a supplemental notification.
   if (!changed_notification_id ||
       base::Contains(supplemental_notifications_, *changed_notification_id))
@@ -880,20 +882,23 @@ void MediaNotificationService::OnNotificationChanged(
 
   // Show or hide supplemental notifications as necessary.
   for (const auto& pair : supplemental_notifications_) {
-    auto* web_contents = pair.second;
-    const bool should_hide = std::any_of(
-        sessions_.begin(), sessions_.end(),
-        [web_contents, this](const auto& pair) {
-          return pair.second.web_contents() == web_contents &&
-                 base::Contains(active_controllable_session_ids_, pair.first);
-        });
-
     // If there is an active session associated with the same web contents as
     // this supplemental notification, hide it; if not, show it.
-    if (should_hide) {
+    if (HasSessionForWebContents(pair.second)) {
       HideNotification(pair.first);
     } else {
       ShowNotification(pair.first);
     }
   }
 }
+
+bool MediaNotificationService::HasSessionForWebContents(
+    content::WebContents* web_contents) const {
+  DCHECK(web_contents);
+  return std::any_of(sessions_.begin(), sessions_.end(),
+                     [web_contents, this](const auto& pair) {
+                       return pair.second.web_contents() == web_contents &&
+                              base::Contains(active_controllable_session_ids_,
+                                             pair.first);
+                     });
+}

chrome/browser/ui/global_media_controls/media_notification_service.h

@@ -299,12 +299,6 @@ class MediaNotificationService
   base::WeakPtr<media_message_center::MediaNotificationItem>
   GetNotificationItem(const std::string& id);
 
-  // Called after changing anything about a notification to notify any observers
-  // and update the visibility of supplemental notifications.  If the change is
-  // associated with a particular notification ID, that ID should be passed as
-  // the argument, otherwise the argument should be nullptr.
-  void OnNotificationChanged(const std::string* changed_notification_id);
-
   // Looks up a Session object by its ID.  Returns null if not found.
   Session* GetSession(const std::string& id);
 
@@ -315,6 +309,14 @@ class MediaNotificationService
   base::WeakPtr<media_message_center::MediaNotificationItem>
   GetNonSessionNotificationItem(const std::string& id);
 
+  // Called after changing anything about a notification to notify any observers
+  // and update the visibility of supplemental notifications.  If the change is
+  // associated with a particular notification ID, that ID should be passed as
+  // the argument, otherwise the argument should be nullptr.
+  void OnNotificationChanged(const std::string* changed_notification_id);
+
+  bool HasSessionForWebContents(content::WebContents* web_contents) const;
+
   MediaDialogDelegate* dialog_delegate_ = nullptr;
 
   OverlayMediaNotificationsManagerImpl overlay_media_notifications_manager_;

chrome/browser/ui/global_media_controls/presentation_request_notification_provider.cc

@@ -16,6 +16,8 @@
 #include "components/media_router/common/providers/cast/cast_media_source.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/web_contents.h"
 
 namespace {
 
@@ -103,8 +105,8 @@ void PresentationRequestNotificationProvider::AfterMediaDialogOpened(
   // request.
   if (presentation_manager->HasDefaultPresentationRequest() &&
       notification_service_->HasOpenDialog()) {
-    CreateItemForPresentationRequest(
-        presentation_manager->GetDefaultPresentationRequest(), nullptr);
+    OnDefaultPresentationChanged(
+        &presentation_manager->GetDefaultPresentationRequest());
   }
 }
 
@@ -138,5 +140,10 @@ void PresentationRequestNotificationProvider::CreateItemForPresentationRequest(
   // This may replace an existing item, which is the right thing to do if we've
   // reached this point.
   item_.emplace(notification_service_, request, std::move(context));
-  notification_service_->ShowNotification(item_->id());
+
+  auto* rfh = content::RenderFrameHost::FromID(request.render_frame_host_id);
+  DCHECK(rfh);
+  auto* web_contents = content::WebContents::FromRenderFrameHost(rfh);
+  DCHECK(web_contents);
+  notification_service_->AddSupplementalNotification(item_->id(), web_contents);
 }