GMC: Remove item when tab is closed

This CL makes |MediaToolbarButtonController::Session| a
|WebContentsObserver| so that we get notified when a tab is closed.
This allows us to immediately remove a notification and hide the
toolbar icon when a tab is closed instead of freezing it.

Bug: 994509
Change-Id: I751e3c8a8c0cc696984a9581b7f9dbfe39ed69e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1793235Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695377}

chrome/browser/ui/global_media_controls/media_toolbar_button_controller.cc

@@ -20,6 +20,11 @@
 
 namespace {
 
+// Here we check to see if the WebContents is focused. Note that since Session
+// is a WebContentsObserver, we could in theory listen for
+// |OnWebContentsFocused()| and |OnWebContentsLostFocus()|. However, this won't
+// actually work since focusing the MediaDialogView causes the WebContents to
+// "lose focus", so we'd never be focused.
 bool IsWebContentsFocused(content::WebContents* web_contents) {
   DCHECK(web_contents);
   Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
@@ -38,12 +43,26 @@ bool IsWebContentsFocused(content::WebContents* web_contents) {
 }  // anonymous namespace
 
 MediaToolbarButtonController::Session::Session(
+    MediaToolbarButtonController* owner,
+    const std::string& id,
     std::unique_ptr<media_message_center::MediaNotificationItem> item,
     content::WebContents* web_contents)
-    : item_(std::move(item)), web_contents_(web_contents) {}
+    : content::WebContentsObserver(web_contents),
+      owner_(owner),
+      id_(id),
+      item_(std::move(item)) {
+  DCHECK(owner_);
+  DCHECK(item_);
+}
 
 MediaToolbarButtonController::Session::~Session() = default;
 
+void MediaToolbarButtonController::Session::WebContentsDestroyed() {
+  // If the WebContents is destroyed, then we should just remove the item
+  // instead of freezing it.
+  owner_->RemoveItem(id_);
+}
+
 MediaToolbarButtonController::MediaToolbarButtonController(
     const base::UnguessableToken& source_id,
     service_manager::Connector* connector,
@@ -106,6 +125,7 @@ void MediaToolbarButtonController::OnFocusGained(
     sessions_.emplace(
         std::piecewise_construct, std::forward_as_tuple(id),
         std::forward_as_tuple(
+            this, id,
             std::make_unique<media_message_center::MediaNotificationItem>(
                 this, id, session->source_name.value_or(std::string()),
                 std::move(controller), std::move(session->session_info)),

chrome/browser/ui/global_media_controls/media_toolbar_button_controller.h

@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "components/media_message_center/media_notification_controller.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "services/media_session/public/mojom/audio_focus.mojom.h"
@@ -69,18 +70,23 @@ class MediaToolbarButtonController
     kHidden,
   };
 
-  class Session {
+  class Session : public content::WebContentsObserver {
    public:
-    Session(std::unique_ptr<media_message_center::MediaNotificationItem> item,
+    Session(MediaToolbarButtonController* owner,
+            const std::string& id,
+            std::unique_ptr<media_message_center::MediaNotificationItem> item,
             content::WebContents* web_contents);
-    ~Session();
+    ~Session() override;
+
+    // content::WebContentsObserver implementation.
+    void WebContentsDestroyed() override;
 
     media_message_center::MediaNotificationItem* item() { return item_.get(); }
-    content::WebContents* web_contents() { return web_contents_; }
 
    private:
+    MediaToolbarButtonController* owner_;
+    const std::string id_;
     std::unique_ptr<media_message_center::MediaNotificationItem> item_;
-    content::WebContents* web_contents_;
 
     DISALLOW_COPY_AND_ASSIGN(Session);
   };

chrome/browser/ui/global_media_controls/media_toolbar_button_controller_unittest.cc

@@ -152,6 +152,18 @@ class MediaToolbarButtonControllerTest : public testing::Test {
     delegate->Open(controller_.get());
   }
 
+  void SimulateTabClosed(const base::UnguessableToken& id) {
+    // When a tab is closing, audio focus will be lost before the WebContents is
+    // destroyed, so to simulate closer to reality we will also simulate audio
+    // focus lost here.
+    SimulateFocusLost(id);
+
+    // Now, close the tab.
+    auto item_itr = controller_->sessions_.find(id.ToString());
+    EXPECT_NE(controller_->sessions_.end(), item_itr);
+    item_itr->second.WebContentsDestroyed();
+  }
+
   void ExpectHistogramCountRecorded(int count, int size) {
     histogram_tester_.ExpectBucketCount(
         media_message_center::kCountHistogramName, count, size);
@@ -352,3 +364,16 @@ TEST_F(MediaToolbarButtonControllerTest, NewMediaSessionWhileDialogOpen) {
   ExpectHistogramCountRecorded(1, 1);
   ExpectHistogramCountRecorded(2, 1);
 }
+
+TEST_F(MediaToolbarButtonControllerTest,
+       SessionIsRemovedImmediatelyWhenATabCloses) {
+  // First, show the button.
+  EXPECT_CALL(delegate(), Show());
+  base::UnguessableToken id = SimulatePlayingControllableMedia();
+  testing::Mock::VerifyAndClearExpectations(&delegate());
+
+  // Then, close the tab. The button should immediately hide.
+  EXPECT_CALL(delegate(), Hide());
+  SimulateTabClosed(id);
+  testing::Mock::VerifyAndClearExpectations(&delegate());
+}