[91] HTMLMediaElement: Disconnect mojo pipes when moving to new document

This resets the |media_player_host_remote_| when the document changes.
A bound MediaPlayerHost is associated with the current frame. Failing
to reset the host will cause duplicate MediaPlayerIds in
MediaWebContentsObserver because the delegate ids will restart at 1
while the RFH id remains the same.

(cherry picked from commit 9e023ff)

Bug: 1195769, 1187160
Change-Id: I89e37814c440e3616bc129f0a1a026b365ecdd8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2808277
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#870883}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2822331
Auto-Submit: Chrome Cunningham <chcunningham@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/branch-heads/4472@{#28}
Cr-Branched-From: 3d60439-refs/heads/master@{#870763}

third_party/blink/renderer/core/html/media/html_media_element.cc

@@ -538,9 +538,20 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tag_name,
       media_controls_(nullptr),
       controls_list_(MakeGarbageCollected<HTMLMediaElementControlsList>(this)),
       lazy_load_intersection_observer_(nullptr),
-      media_player_host_remote_(GetExecutionContext()),
-      media_player_observer_remote_set_(GetExecutionContext()),
-      media_player_receiver_set_(this, GetExecutionContext()) {
+      media_player_host_remote_(
+          MakeGarbageCollected<DisallowNewWrapper<
+              HeapMojoAssociatedRemote<media::mojom::blink::MediaPlayerHost>>>(
+              GetExecutionContext())),
+      media_player_observer_remote_set_(
+          MakeGarbageCollected<DisallowNewWrapper<HeapMojoAssociatedRemoteSet<
+              media::mojom::blink::MediaPlayerObserver>>>(
+              GetExecutionContext())),
+      media_player_receiver_set_(
+          MakeGarbageCollected<DisallowNewWrapper<
+              HeapMojoAssociatedReceiverSet<media::mojom::blink::MediaPlayer,
+                                            HTMLMediaElement>>>(
+              this,
+              GetExecutionContext())) {
   DVLOG(1) << "HTMLMediaElement(" << *this << ")";
 
   LocalFrame* frame = document.GetFrame();
@@ -605,6 +616,20 @@ void HTMLMediaElement::DidMoveToNewDocument(Document& old_document) {
 
   RemoveElementFromDocumentMap(this, &old_document);
   AddElementToDocumentMap(this, &GetDocument());
+  SetExecutionContext(GetExecutionContext());
+
+  // Reset mojo state that is coupled to |old_document|'s execution context.
+  // NOTE: |media_player_host_remote_| is also coupled to |old_document|'s frame
+  media_player_host_remote_ = MakeGarbageCollected<DisallowNewWrapper<
+      HeapMojoAssociatedRemote<media::mojom::blink::MediaPlayerHost>>>(
+      GetExecutionContext());
+  media_player_observer_remote_set_ = MakeGarbageCollected<DisallowNewWrapper<
+      HeapMojoAssociatedRemoteSet<media::mojom::blink::MediaPlayerObserver>>>(
+      GetExecutionContext());
+  media_player_receiver_set_ =
+      MakeGarbageCollected<DisallowNewWrapper<HeapMojoAssociatedReceiverSet<
+          media::mojom::blink::MediaPlayer, HTMLMediaElement>>>(
+          this, GetExecutionContext());
 
   // FIXME: This is a temporary fix to prevent this object from causing the
   // MediaPlayer to dereference LocalFrame and FrameLoader pointers from the
@@ -620,7 +645,6 @@ void HTMLMediaElement::DidMoveToNewDocument(Document& old_document) {
   // load event from within the destructor.
   old_document.DecrementLoadEventDelayCount();
 
-  SetExecutionContext(GetExecutionContext());
   HTMLElement::DidMoveToNewDocument(old_document);
 }
 
@@ -1330,7 +1354,7 @@ void HTMLMediaElement::StartPlayerLoad() {
 
   // Setup the communication channels between the renderer and browser processes
   // via the MediaPlayer and MediaPlayerObserver mojo interfaces.
-  DCHECK(media_player_receiver_set_.empty());
+  DCHECK(media_player_receiver_set_->Value().empty());
   mojo::PendingAssociatedRemote<media::mojom::blink::MediaPlayer>
       media_player_remote;
   BindMediaPlayerReceiver(
@@ -1470,13 +1494,13 @@ bool HTMLMediaElement::PausedWhenVisible() const {
 
 void HTMLMediaElement::DidAudioOutputSinkChanged(
     const String& hashed_device_id) {
-  for (auto& observer : media_player_observer_remote_set_)
+  for (auto& observer : media_player_observer_remote_set_->Value())
     observer->OnAudioOutputSinkChanged(hashed_device_id);
 }
 
 void HTMLMediaElement::SetMediaPlayerHostForTesting(
     mojo::PendingAssociatedRemote<media::mojom::blink::MediaPlayerHost> host) {
-  media_player_host_remote_.Bind(
+  media_player_host_remote_->Value().Bind(
       std::move(host), GetDocument().GetTaskRunner(TaskType::kInternalMedia));
 }
 
@@ -3681,8 +3705,8 @@ void HTMLMediaElement::
 
     // The lifetime of the mojo endpoints are tied to the WebMediaPlayer's, so
     // we need to reset those as well.
-    media_player_receiver_set_.Clear();
-    media_player_observer_remote_set_.Clear();
+    media_player_receiver_set_->Value().Clear();
+    media_player_observer_remote_set_->Value().Clear();
   }
   OnWebMediaPlayerCleared();
 }
@@ -4125,7 +4149,7 @@ bool HTMLMediaElement::IsInteractiveContent() const {
 void HTMLMediaElement::BindMediaPlayerReceiver(
     mojo::PendingAssociatedReceiver<media::mojom::blink::MediaPlayer>
         receiver) {
-  media_player_receiver_set_.Add(
+  media_player_receiver_set_->Value().Add(
       std::move(receiver),
       GetDocument().GetTaskRunner(TaskType::kInternalMedia));
 }
@@ -4405,51 +4429,51 @@ void HTMLMediaElement::PausePlayback() {
 }
 
 void HTMLMediaElement::DidPlayerStartPlaying() {
-  for (auto& observer : media_player_observer_remote_set_)
+  for (auto& observer : media_player_observer_remote_set_->Value())
     observer->OnMediaPlaying();
 }
 
 void HTMLMediaElement::DidPlayerPaused(bool stream_ended) {
-  for (auto& observer : media_player_observer_remote_set_)
+  for (auto& observer : media_player_observer_remote_set_->Value())
     observer->OnMediaPaused(stream_ended);
 }
 
 void HTMLMediaElement::DidPlayerMutedStatusChange(bool muted) {
-  for (auto& observer : media_player_observer_remote_set_)
+  for (auto& observer : media_player_observer_remote_set_->Value())
     observer->OnMutedStatusChanged(muted);
 }
 
 void HTMLMediaElement::DidMediaMetadataChange(
     bool has_audio,
     bool has_video,
     media::MediaContentType media_content_type) {
-  for (auto& observer : media_player_observer_remote_set_)
+  for (auto& observer : media_player_observer_remote_set_->Value())
     observer->OnMediaMetadataChanged(has_audio, has_video, media_content_type);
 }
 
 void HTMLMediaElement::DidPlayerMediaPositionStateChange(
     double playback_rate,
     base::TimeDelta duration,
     base::TimeDelta position) {
-  for (auto& observer : media_player_observer_remote_set_) {
+  for (auto& observer : media_player_observer_remote_set_->Value()) {
     observer->OnMediaPositionStateChanged(
         media_session::mojom::blink::MediaPosition::New(
             playback_rate, duration, position, base::TimeTicks::Now()));
   }
 }
 
 void HTMLMediaElement::DidDisableAudioOutputSinkChanges() {
-  for (auto& observer : media_player_observer_remote_set_)
+  for (auto& observer : media_player_observer_remote_set_->Value())
     observer->OnAudioOutputSinkChangingDisabled();
 }
 
 void HTMLMediaElement::DidPlayerSizeChange(const gfx::Size& size) {
-  for (auto& observer : media_player_observer_remote_set_)
+  for (auto& observer : media_player_observer_remote_set_->Value())
     observer->OnMediaSizeChanged(size);
 }
 
 void HTMLMediaElement::DidBufferUnderflow() {
-  for (auto& observer : media_player_observer_remote_set_)
+  for (auto& observer : media_player_observer_remote_set_->Value())
     observer->OnBufferUnderflow();
 }
 
@@ -4459,7 +4483,7 @@ void HTMLMediaElement::DidSeek() {
       (base::TimeTicks::Now() - last_seek_update_time_ >=
        base::TimeDelta::FromSeconds(1))) {
     last_seek_update_time_ = base::TimeTicks::Now();
-    for (auto& observer : media_player_observer_remote_set_)
+    for (auto& observer : media_player_observer_remote_set_->Value())
       observer->OnSeek();
   }
 }
@@ -4468,22 +4492,23 @@ media::mojom::blink::MediaPlayerHost&
 HTMLMediaElement::GetMediaPlayerHostRemote() {
   // It is an error to call this before having access to the document's frame.
   DCHECK(GetDocument().GetFrame());
-  if (!media_player_host_remote_.is_bound()) {
+  if (!media_player_host_remote_->Value().is_bound()) {
     GetDocument()
         .GetFrame()
         ->GetRemoteNavigationAssociatedInterfaces()
-        ->GetInterface(media_player_host_remote_.BindNewEndpointAndPassReceiver(
-            GetDocument().GetTaskRunner(TaskType::kInternalMedia)));
+        ->GetInterface(
+            media_player_host_remote_->Value().BindNewEndpointAndPassReceiver(
+                GetDocument().GetTaskRunner(TaskType::kInternalMedia)));
   }
-  return *media_player_host_remote_.get();
+  return *media_player_host_remote_->Value().get();
 }
 
 mojo::PendingAssociatedReceiver<media::mojom::blink::MediaPlayerObserver>
 HTMLMediaElement::AddMediaPlayerObserverAndPassReceiver() {
   mojo::PendingAssociatedRemote<media::mojom::blink::MediaPlayerObserver>
       observer;
   auto observer_receiver = observer.InitWithNewEndpointAndPassReceiver();
-  media_player_observer_remote_set_.Add(
+  media_player_observer_remote_set_->Value().Add(
       std::move(observer),
       GetDocument().GetTaskRunner(TaskType::kInternalMedia));
   return observer_receiver;

third_party/blink/renderer/core/html/media/html_media_element.h

@@ -45,6 +45,7 @@
 #include "third_party/blink/renderer/core/intersection_observer/intersection_observer.h"
 #include "third_party/blink/renderer/platform/audio/audio_source_provider.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
+#include "third_party/blink/renderer/platform/heap/disallow_new_wrapper.h"
 #include "third_party/blink/renderer/platform/media/web_audio_source_provider_client.h"
 #include "third_party/blink/renderer/platform/mojo/heap_mojo_associated_receiver_set.h"
 #include "third_party/blink/renderer/platform/mojo/heap_mojo_associated_remote.h"
@@ -377,7 +378,7 @@ class CORE_EXPORT HTMLMediaElement
   // HTMLMediaElement's subclasses.
   const HeapMojoAssociatedRemoteSet<media::mojom::blink::MediaPlayerObserver>&
   GetMediaPlayerObserverRemoteSet() {
-    return media_player_observer_remote_set_;
+    return media_player_observer_remote_set_->Value();
   }
 
   void ParseAttribute(const AttributeModificationParams&) override;
@@ -843,20 +844,23 @@ class CORE_EXPORT HTMLMediaElement
 
   Member<IntersectionObserver> lazy_load_intersection_observer_;
 
-  HeapMojoAssociatedRemote<media::mojom::blink::MediaPlayerHost>
+  Member<DisallowNewWrapper<
+      HeapMojoAssociatedRemote<media::mojom::blink::MediaPlayerHost>>>
       media_player_host_remote_;
 
   // Multiple objects outside of the renderer process can register as observers,
   // so we need to store the remotes in a set here.
-  HeapMojoAssociatedRemoteSet<media::mojom::blink::MediaPlayerObserver>
+  Member<DisallowNewWrapper<
+      HeapMojoAssociatedRemoteSet<media::mojom::blink::MediaPlayerObserver>>>
       media_player_observer_remote_set_;
 
   // A receiver set is needed here as there will be different objects in the
   // browser communicating with this object. This is done this way to avoid
   // routing everything through a single class (e.g. RFHI) and to keep this
   // logic contained inside MediaPlayer-related classes.
-  HeapMojoAssociatedReceiverSet<media::mojom::blink::MediaPlayer,
-                                HTMLMediaElement>
+  Member<DisallowNewWrapper<
+      HeapMojoAssociatedReceiverSet<media::mojom::blink::MediaPlayer,
+                                    HTMLMediaElement>>>
       media_player_receiver_set_;
 };