Support multiple MediaPlayerHost connections per frame

With the previous set-up,
MediaWebContentsObserver::BindMediaPlayerHost() contained a hidden
assumption that a RenderFrameHost uniquely identifies a media element.
When another element was created within the same frame,
BindMediaPlayerHostReceiver() would reset the connection to the renderer
that was set up previously. As a result, MediaPlayerHost messages for
the media elements created earlier were not reaching the browser if a
frame created multiple media elements.

To fix this, we add support for multiple MediaPlayerHost connections
from a single frame, one for each HTMLMediaElement, with each
HTMLMediaElement calling mojom::MediaPlayerHost::OnMediaPlayerAdded()
through a dedicated connection.

The disconnect handler for MediaPlayerHost receivers is gone. It could
have been reimplemented as a handler on the ReceiverSet, removing
MediaPlayerHostImpl from MediaWebContentsObserver when the receiver set
becomes empty. However, this removal is effectively a 'delete this'
operation and it doesn't seem necessary. By the time there are no
receivers, the MediaPlayerHostImpl object is just an empty ReceiverSet
and two pointers. It is cleaned up upon render frame deletion.

Bug: 1167154
Change-Id: Idd7873da91ee452c91b2f4667f29a1aa80831678
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2632691
Commit-Queue: Wojciech Dzierżanowski <wdzierzanowski@opera.com>
Reviewed-by: Mario Sanchez Prada <mario@igalia.com>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845371}

content/browser/media/media_web_contents_observer.cc

@@ -313,15 +313,7 @@ MediaWebContentsObserver::MediaPlayerHostImpl::~MediaPlayerHostImpl() = default;
 
 void MediaWebContentsObserver::MediaPlayerHostImpl::BindMediaPlayerHostReceiver(
     mojo::PendingReceiver<media::mojom::MediaPlayerHost> receiver) {
-  receiver_.reset();
-  receiver_.Bind(std::move(receiver));
-
-  // Both |media_web_contents_observer_| and |render_frame_host_| outlive
-  // MediaPlayerHostImpl, so it's safe to use base::Unretained().
-  receiver_.set_disconnect_handler(
-      base::BindOnce(&MediaWebContentsObserver::OnMediaPlayerHostDisconnected,
-                     base::Unretained(media_web_contents_observer_),
-                     base::Unretained(render_frame_host_)));
+  receivers_.Add(this, std::move(receiver));
 }
 
 void MediaWebContentsObserver::MediaPlayerHostImpl::OnMediaPlayerAdded(
@@ -538,12 +530,6 @@ media::mojom::MediaPlayer* MediaWebContentsObserver::GetMediaPlayerRemote(
   return nullptr;
 }
 
-void MediaWebContentsObserver::OnMediaPlayerHostDisconnected(
-    RenderFrameHost* host) {
-  DCHECK(media_player_hosts_.contains(host));
-  media_player_hosts_.erase(host);
-}
-
 void MediaWebContentsObserver::OnMediaPlayerObserverDisconnected(
     const MediaPlayerId& player_id) {
   DCHECK(media_player_observer_hosts_.contains(player_id));

content/browser/media/media_web_contents_observer.h

@@ -26,6 +26,7 @@
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver.h"
+#include "mojo/public/cpp/bindings/receiver_set.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "services/device/public/mojom/wake_lock.mojom.h"
 
@@ -160,7 +161,7 @@ class CONTENT_EXPORT MediaWebContentsObserver : public WebContentsObserver {
                         MediaWebContentsObserver* media_web_contents_observer);
     ~MediaPlayerHostImpl() override;
 
-    // Used to bind the receiver via the BrowserInterfaceBroker.
+    // Used to bind receivers via the BrowserInterfaceBroker.
     void BindMediaPlayerHostReceiver(
         mojo::PendingReceiver<media::mojom::MediaPlayerHost> receiver);
 
@@ -172,7 +173,7 @@ class CONTENT_EXPORT MediaWebContentsObserver : public WebContentsObserver {
    private:
     RenderFrameHost* render_frame_host_;
     MediaWebContentsObserver* media_web_contents_observer_;
-    mojo::Receiver<media::mojom::MediaPlayerHost> receiver_{this};
+    mojo::ReceiverSet<media::mojom::MediaPlayerHost> receivers_;
   };
 
   // Helper class providing a per-MediaPlayerId object implementing the
@@ -234,10 +235,6 @@ class CONTENT_EXPORT MediaWebContentsObserver : public WebContentsObserver {
                                 int delegate_id,
                                 std::string hashed_device_id);
 
-  // Used to notify when the renderer -> browser mojo connection via the
-  // interface media::mojom::MediaPlayerHost gets disconnected.
-  void OnMediaPlayerHostDisconnected(RenderFrameHost* host);
-
   // Used to notify when the renderer -> browser mojo connection via the
   // interface media::mojom::MediaPlayerObserver gets disconnected.
   void OnMediaPlayerObserverDisconnected(const MediaPlayerId& player_id);