Migrate MediaPlayerDelegateHostMsg_On{BufferUnderflow,MediaSizeChanged}

Migrate these legacy IPC message to the media.mojom.MediaPlayerObserver
mojo interface, implemented by MediaSessionController in the browser
process, and update their callers in the renderer process.

This CL migrates these messages together to simplify the review process
by avoiding unnecessary cycles, since the way they both get migrated is
nearly identical.

Relevant design document:
https://docs.google.com/document/d/1OLMNxLvGkRO6ju_WfHbRMrgaVnW7bsMnW7XlJQI0A2E

Bug: 1039252
Change-Id: I2e05e9fc39f17f3875fadcbaf65a28eff5170be0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2560621
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835683}

content/browser/media/media_web_contents_observer.cc

@@ -235,12 +235,8 @@ bool MediaWebContentsObserver::OnMessageReceived(
     IPC_MESSAGE_HANDLER(
         MediaPlayerDelegateHostMsg_OnMediaEffectivelyFullscreenChanged,
         OnMediaEffectivelyFullscreenChanged)
-    IPC_MESSAGE_HANDLER(MediaPlayerDelegateHostMsg_OnMediaSizeChanged,
-                        OnMediaSizeChanged)
     IPC_MESSAGE_HANDLER(MediaPlayerDelegateHostMsg_OnAudioOutputSinkChanged,
                         OnAudioOutputSinkChanged);
-    IPC_MESSAGE_HANDLER(MediaPlayerDelegateHostMsg_OnBufferUnderflow,
-                        OnBufferUnderflow)
     IPC_MESSAGE_HANDLER(MediaPlayerDelegateHostMsg_OnSeek, OnSeek)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
@@ -344,6 +340,12 @@ void MediaWebContentsObserver::MediaPlayerObserverHostImpl::
       ->OnMediaPositionStateChanged(media_player_id_, media_position);
 }
 
+void MediaWebContentsObserver::MediaPlayerObserverHostImpl::OnMediaSizeChanged(
+    const ::gfx::Size& size) {
+  media_web_contents_observer_->web_contents_impl()->MediaResized(
+      size, media_player_id_);
+}
+
 void MediaWebContentsObserver::MediaPlayerObserverHostImpl::
     OnPictureInPictureAvailabilityChanged(bool available) {
   media_web_contents_observer_->session_controllers_manager()
@@ -356,6 +358,12 @@ void MediaWebContentsObserver::MediaPlayerObserverHostImpl::
       ->OnAudioOutputSinkChangingDisabled(media_player_id_);
 }
 
+void MediaWebContentsObserver::MediaPlayerObserverHostImpl::
+    OnBufferUnderflow() {
+  media_web_contents_observer_->web_contents_impl()->MediaBufferUnderflow(
+      media_player_id_);
+}
+
 MediaWebContentsObserver::PlayerInfo* MediaWebContentsObserver::GetPlayerInfo(
     const MediaPlayerId& id) const {
   const auto it = player_info_map_.find(id);
@@ -459,14 +467,6 @@ void MediaWebContentsObserver::OnMediaEffectivelyFullscreenChanged(
   web_contents_impl()->MediaEffectivelyFullscreenChanged(is_fullscreen);
 }
 
-void MediaWebContentsObserver::OnMediaSizeChanged(
-    RenderFrameHost* render_frame_host,
-    int delegate_id,
-    const gfx::Size& size) {
-  const MediaPlayerId id(render_frame_host, delegate_id);
-  web_contents_impl()->MediaResized(size, id);
-}
-
 void MediaWebContentsObserver::OnAudioOutputSinkChanged(
     RenderFrameHost* render_frame_host,
     int delegate_id,
@@ -510,13 +510,6 @@ MediaWebContentsObserver::GetMediaPlayerRemote(const MediaPlayerId& player_id) {
   return media_player_remotes_[player_id];
 }
 
-void MediaWebContentsObserver::OnBufferUnderflow(
-    RenderFrameHost* render_frame_host,
-    int delegate_id) {
-  const MediaPlayerId id(render_frame_host, delegate_id);
-  web_contents_impl()->MediaBufferUnderflow(id);
-}
-
 void MediaWebContentsObserver::OnSeek(RenderFrameHost* render_frame_host,
                                       int delegate_id) {
   const MediaPlayerId id(render_frame_host, delegate_id);

content/browser/media/media_web_contents_observer.h

@@ -194,8 +194,10 @@ class CONTENT_EXPORT MediaWebContentsObserver : public WebContentsObserver {
     void OnMutedStatusChanged(bool muted) override;
     void OnMediaPositionStateChanged(
         const media_session::MediaPosition& media_position) override;
+    void OnMediaSizeChanged(const ::gfx::Size& size) override;
     void OnPictureInPictureAvailabilityChanged(bool available) override;
     void OnAudioOutputSinkChangingDisabled() override;
+    void OnBufferUnderflow() override;
 
    private:
     MediaPlayerId media_player_id_;
@@ -230,13 +232,9 @@ class CONTENT_EXPORT MediaWebContentsObserver : public WebContentsObserver {
       RenderFrameHost* render_frame_host,
       int delegate_id,
       blink::WebFullscreenVideoStatus fullscreen_status);
-  void OnMediaSizeChanged(RenderFrameHost* render_frame_host,
-                          int delegate_id,
-                          const gfx::Size& size);
   void OnAudioOutputSinkChanged(RenderFrameHost* render_frame_host,
                                 int delegate_id,
                                 std::string hashed_device_id);
-  void OnBufferUnderflow(RenderFrameHost* render_frame_host, int delegate_id);
   void OnSeek(RenderFrameHost* render_frame_host, int delegate_id);
 
   // Used to notify when the renderer -> browser mojo connection via the

content/common/media/media_player_delegate_messages.h

@@ -74,17 +74,10 @@ IPC_MESSAGE_ROUTED2(
     int /* delegate_id, distinguishes instances */,
     blink::WebFullscreenVideoStatus /* fullscreen_video_status */)
 
-IPC_MESSAGE_ROUTED2(MediaPlayerDelegateHostMsg_OnMediaSizeChanged,
-                    int /* delegate_id, distinguishes instances */,
-                    gfx::Size /* new size of video */)
-
 IPC_MESSAGE_ROUTED2(MediaPlayerDelegateHostMsg_OnAudioOutputSinkChanged,
                     int /* delegate_id, distinguishes instances */,
                     std::string /* hashed_device_id */)
 
-IPC_MESSAGE_ROUTED1(MediaPlayerDelegateHostMsg_OnBufferUnderflow,
-                    int /* delegate_id, distinguishes instances */)
-
 IPC_MESSAGE_ROUTED1(MediaPlayerDelegateHostMsg_OnSeek,
                     int /* delegate_id, distinguishes instances */)
 

content/renderer/media/renderer_webmediaplayer_delegate.cc

@@ -195,13 +195,6 @@ void RendererWebMediaPlayerDelegate::SetIsEffectivelyFullscreen(
       routing_id(), player_id, fullscreen_video_status));
 }
 
-void RendererWebMediaPlayerDelegate::DidPlayerSizeChange(
-    int delegate_id,
-    const gfx::Size& size) {
-  Send(new MediaPlayerDelegateHostMsg_OnMediaSizeChanged(routing_id(),
-                                                         delegate_id, size));
-}
-
 void RendererWebMediaPlayerDelegate::DidAudioOutputSinkChange(
     int delegate_id,
     const std::string& hashed_device_id) {
@@ -209,11 +202,6 @@ void RendererWebMediaPlayerDelegate::DidAudioOutputSinkChange(
       routing_id(), delegate_id, hashed_device_id));
 }
 
-void RendererWebMediaPlayerDelegate::DidBufferUnderflow(int player_id) {
-  Send(new MediaPlayerDelegateHostMsg_OnBufferUnderflow(routing_id(),
-                                                        player_id));
-}
-
 void RendererWebMediaPlayerDelegate::DidSeek(int player_id) {
   // Send the seek updates to delegate only once per second.
   if (last_seek_update_time_.is_null() ||

content/renderer/media/renderer_webmediaplayer_delegate.h

@@ -65,10 +65,8 @@ class CONTENT_EXPORT RendererWebMediaPlayerDelegate
   void SetIsEffectivelyFullscreen(
       int player_id,
       blink::WebFullscreenVideoStatus fullscreen_video_status) override;
-  void DidPlayerSizeChange(int delegate_id, const gfx::Size& size) override;
   void DidAudioOutputSinkChange(int delegate_id,
                                 const std::string& hashed_device_id) override;
-  void DidBufferUnderflow(int player_id) override;
   void DidSeek(int player_id) override;
 
   // content::RenderFrameObserver overrides.

content/renderer/media/renderer_webmediaplayer_delegate_browsertest.cc

@@ -166,22 +166,6 @@ TEST_F(RendererWebMediaPlayerDelegateTest, SendsMessagesCorrectly) {
         MediaPlayerDelegateHostMsg_OnMediaDestroyed::Read(msg, &result));
     EXPECT_EQ(delegate_id, std::get<0>(result));
   }
-
-  // Verify the resize message.
-  {
-    test_sink().ClearMessages();
-    delegate_manager_->DidPlayerSizeChange(delegate_id, gfx::Size(16, 9));
-    const IPC::Message* msg = test_sink().GetUniqueMessageMatching(
-        MediaPlayerDelegateHostMsg_OnMediaSizeChanged::ID);
-    ASSERT_TRUE(msg);
-
-    std::tuple<int, gfx::Size> result;
-    ASSERT_TRUE(
-        MediaPlayerDelegateHostMsg_OnMediaSizeChanged::Read(msg, &result));
-    EXPECT_EQ(delegate_id, std::get<0>(result));
-    EXPECT_EQ(16, std::get<1>(result).width());
-    EXPECT_EQ(9, std::get<1>(result).height());
-  }
 }
 
 TEST_F(RendererWebMediaPlayerDelegateTest, DeliversObserverNotifications) {

media/blink/webmediaplayer_impl.cc

@@ -2271,7 +2271,7 @@ void WebMediaPlayerImpl::OnBufferingStateChangeInternal(
         !seeking_) {
       underflow_timer_ = std::make_unique<base::ElapsedTimer>();
       watch_time_reporter_->OnUnderflow();
-      delegate_->DidBufferUnderflow(delegate_id_);
+      client_->DidBufferUnderflow();
 
       if (playback_events_recorder_)
         playback_events_recorder_->OnBuffering();
@@ -2406,7 +2406,7 @@ void WebMediaPlayerImpl::OnVideoNaturalSizeChange(const gfx::Size& size) {
   if (observer_)
     observer_->OnMetadataChanged(pipeline_metadata_);
 
-  delegate_->DidPlayerSizeChange(delegate_id_, NaturalSize());
+  client_->DidPlayerSizeChange(NaturalSize());
 }
 
 void WebMediaPlayerImpl::OnVideoOpacityChange(bool opaque) {
@@ -3047,7 +3047,7 @@ void WebMediaPlayerImpl::SetDelegateState(DelegateState new_state,
       break;
     case DelegateState::PLAYING: {
       if (HasVideo())
-        delegate_->DidPlayerSizeChange(delegate_id_, NaturalSize());
+        client_->DidPlayerSizeChange(NaturalSize());
       delegate_->DidPlay(delegate_id_);
       break;
     }

media/blink/webmediaplayer_impl_unittest.cc

@@ -69,6 +69,7 @@
 #include "third_party/blink/public/web/web_testing_support.h"
 #include "third_party/blink/public/web/web_view.h"
 #include "third_party/blink/public/web/web_widget.h"
+#include "ui/gfx/geometry/size.h"
 #include "url/gurl.h"
 
 using ::base::test::RunClosure;
@@ -159,6 +160,8 @@ class MockWebMediaPlayerClient : public blink::WebMediaPlayerClient {
   MOCK_METHOD3(DidPlayerMediaPositionStateChange,
                void(double, base::TimeDelta, base::TimeDelta position));
   MOCK_METHOD0(DidDisableAudioOutputSinkChanges, void());
+  MOCK_METHOD1(DidPlayerSizeChange, void(const gfx::Size&));
+  MOCK_METHOD0(DidBufferUnderflow, void());
   MOCK_METHOD0(GetFeatures, Features(void));
   MOCK_METHOD0(OnRequestVideoFrameCallback, void());
   MOCK_METHOD0(GetTextTrackMetadata, std::vector<blink::TextTrackMetadata>());
@@ -237,14 +240,6 @@ class MockWebMediaPlayerDelegate : public blink::WebMediaPlayerDelegate {
     DCHECK_EQ(player_id_, player_id);
   }
 
-  void DidPlayerSizeChange(int player_id, const gfx::Size& size) override {
-    DCHECK_EQ(player_id_, player_id);
-  }
-
-  void DidBufferUnderflow(int player_id) override {
-    DCHECK_EQ(player_id_, player_id);
-  }
-
   void DidSeek(int player_id) override { DCHECK_EQ(player_id_, player_id); }
 
   bool IsFrameHidden() override { return is_hidden_; }

media/mojo/mojom/media_player.mojom

@@ -6,6 +6,7 @@ module media.mojom;
 
 import "mojo/public/mojom/base/time.mojom";
 import "services/media_session/public/mojom/media_session.mojom";
+import "ui/gfx/geometry/mojom/geometry.mojom";
 
 // Implemented by HTMLMediaElement in the renderer process.
 interface MediaPlayer {
@@ -44,12 +45,18 @@ interface MediaPlayerObserver {
   // and reports the new current position via |media_position|.
   OnMediaPositionStateChanged(media_session.mojom.MediaPosition media_position);
 
+  // Notifies that the size of the media player has changed.
+  OnMediaSizeChanged(gfx.mojom.Size size);
+
   // Notifies the browser process of PictureinPicture playback's availability.
   OnPictureInPictureAvailabilityChanged(bool available);
 
   // Notifies the browser process that the ability to switch audio output
   // devices for the associated media player has been disabled.
   OnAudioOutputSinkChangingDisabled();
+
+  // Notifies that a buffer underflow event happened for the media player.
+  OnBufferUnderflow();
 };
 
 // Implemented by MediaWebContentsObserver::MediaPlayerHostImpl in the browser

third_party/blink/public/platform/media/webmediaplayer_delegate.h

@@ -8,10 +8,6 @@
 #include "third_party/blink/public/platform/web_common.h"
 #include "third_party/blink/public/platform/web_media_player.h"
 
-namespace gfx {
-class Size;
-}  // namespace gfx
-
 namespace media {
 enum class MediaContentType;
 }  // namespace media
@@ -105,17 +101,11 @@ class BLINK_PLATFORM_EXPORT WebMediaPlayerDelegate {
   // so that we can do the right thing with media that starts paused.
   virtual void DidPause(int player_id, bool reached_end_of_stream) = 0;
 
-  // Notify that the size of the media player is changed.
-  virtual void DidPlayerSizeChange(int delegate_id, const gfx::Size& size) = 0;
-
   // Notify that the audio output sink has changed
   virtual void DidAudioOutputSinkChange(
       int delegate_id,
       const std::string& hashed_device_id) = 0;
 
-  // Notify that a buffer underflow event happened for the media player.
-  virtual void DidBufferUnderflow(int player_id) = 0;
-
   // Notify that a playback seek event happened for the media player.
   virtual void DidSeek(int player_id) = 0;
 

third_party/blink/public/platform/web_media_player_client.h

@@ -177,6 +177,13 @@ class BLINK_PLATFORM_EXPORT WebMediaPlayerClient {
   // Notify the client that the audio sink cannot be changed.
   virtual void DidDisableAudioOutputSinkChanges() = 0;
 
+  // Notify the client that the size of the media player has changed.
+  // TODO(crbug.com/1039252): Remove by merging this method into SizeChanged().
+  virtual void DidPlayerSizeChange(const gfx::Size& size) = 0;
+
+  // Notify the client that a buffer underflow happened for the media player.
+  virtual void DidBufferUnderflow() = 0;
+
   // Notify the client that one of the state used by Picture-in-Picture has
   // changed. The client will then have to poll the states from the associated
   // WebMediaPlayer.

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

@@ -4413,6 +4413,22 @@ void HTMLMediaElement::DidDisableAudioOutputSinkChanges() {
   media_player_observer_remote_->OnAudioOutputSinkChangingDisabled();
 }
 
+void HTMLMediaElement::DidPlayerSizeChange(const gfx::Size& size) {
+  // The remote to the MediaPlayerObserver could be not set yet.
+  if (!media_player_observer_remote_.is_bound())
+    return;
+
+  media_player_observer_remote_->OnMediaSizeChanged(size);
+}
+
+void HTMLMediaElement::DidBufferUnderflow() {
+  // The remote to the MediaPlayerObserver could be not set yet.
+  if (!media_player_observer_remote_.is_bound())
+    return;
+
+  media_player_observer_remote_->OnBufferUnderflow();
+}
+
 media::mojom::blink::MediaPlayerHost&
 HTMLMediaElement::GetMediaPlayerHostRemote() {
   // It is an error to call this before having access to the document's frame.

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

@@ -475,6 +475,8 @@ class CORE_EXPORT HTMLMediaElement
                                          base::TimeDelta duration,
                                          base::TimeDelta position) override;
   void DidDisableAudioOutputSinkChanges() override;
+  void DidPlayerSizeChange(const gfx::Size& size) override;
+  void DidBufferUnderflow() override;
 
   // Returns a reference to the mojo remote for the MediaPlayerHost interface,
   // requesting it first from the BrowserInterfaceBroker if needed. It is an

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

@@ -28,6 +28,7 @@
 #include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
 #include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
 #include "third_party/blink/renderer/platform/wtf/vector.h"
+#include "ui/gfx/geometry/size.h"
 
 using ::testing::_;
 using ::testing::AnyNumber;
@@ -117,19 +118,35 @@ class MockMediaPlayerObserverReceiverForTesting
   void OnMediaPositionStateChanged(
       ::media_session::mojom::blink::MediaPositionPtr) override {}
 
+  void OnMediaSizeChanged(const gfx::Size& size) override {
+    received_media_size_ = size;
+    run_loop_->Quit();
+  }
+
   void OnPictureInPictureAvailabilityChanged(bool available) override {}
 
   void OnAudioOutputSinkChangingDisabled() override {}
 
+  void OnBufferUnderflow() override {
+    received_buffer_underflow_ = true;
+    run_loop_->Quit();
+  }
+
   // Getters used from HTMLMediaElementTest.
   const base::Optional<bool>& received_muted_status() const {
     return received_muted_status_type_;
   }
 
+  gfx::Size received_media_size() const { return received_media_size_; }
+
+  bool received_buffer_underflow() const { return received_buffer_underflow_; }
+
  private:
   std::unique_ptr<base::RunLoop> run_loop_;
   mojo::Receiver<media::mojom::blink::MediaPlayerObserver> receiver_{this};
   base::Optional<bool> received_muted_status_type_;
+  gfx::Size received_media_size_{0, 0};
+  bool received_buffer_underflow_{false};
 };
 
 enum class MediaTestParam { kAudio, kVideo };
@@ -230,6 +247,24 @@ class HTMLMediaElementTest : public testing::TestWithParam<MediaTestParam> {
     return media_player_observer_receiver_->received_muted_status() == muted;
   }
 
+  void NotifyMediaSizeChange(const gfx::Size& size) {
+    media_->DidPlayerSizeChange(size);
+    media_player_observer_receiver_->WaitUntilReceivedMessage();
+  }
+
+  bool ReceivedMessageMediaSizeChange(const gfx::Size& size) {
+    return media_player_observer_receiver_->received_media_size() == size;
+  }
+
+  void NotifyBufferUnderflowEvent() {
+    media_->DidBufferUnderflow();
+    media_player_observer_receiver_->WaitUntilReceivedMessage();
+  }
+
+  bool ReceivedMessageBufferUnderflowEvent() {
+    return media_player_observer_receiver_->received_buffer_underflow();
+  }
+
  private:
   std::unique_ptr<DummyPageHolder> dummy_page_holder_;
   Persistent<HTMLMediaElement> media_;
@@ -874,4 +909,15 @@ TEST_P(HTMLMediaElementTest, SendMutedStatusChangeToObserver) {
   EXPECT_TRUE(ReceivedMessageMutedStatusChange(false));
 }
 
+TEST_P(HTMLMediaElementTest, SendMediaSizeChangeToObserver) {
+  const gfx::Size kTestMediaSizeChangedValue(16, 9);
+  NotifyMediaSizeChange(kTestMediaSizeChangedValue);
+  EXPECT_TRUE(ReceivedMessageMediaSizeChange(kTestMediaSizeChangedValue));
+}
+
+TEST_P(HTMLMediaElementTest, SendBufferOverflowToObserver) {
+  NotifyBufferUnderflowEvent();
+  EXPECT_TRUE(ReceivedMessageBufferUnderflowEvent());
+}
+
 }  // namespace blink

third_party/blink/renderer/modules/mediastream/webmediaplayer_ms.cc

@@ -661,7 +661,7 @@ void WebMediaPlayerMS::ReloadVideo() {
 
   DCHECK_NE(renderer_action, RendererReloadAction::KEEP_RENDERER);
   if (!paused_)
-    delegate_->DidPlayerSizeChange(delegate_id_, NaturalSize());
+    client_->DidPlayerSizeChange(NaturalSize());
 
   // TODO(perkj, magjed): We use OneShot focus type here so that it takes
   // audio focus once it starts, and then will not respond to further audio
@@ -745,7 +745,7 @@ void WebMediaPlayerMS::Play() {
     audio_renderer_->Play();
 
   if (HasVideo())
-    delegate_->DidPlayerSizeChange(delegate_id_, NaturalSize());
+    client_->DidPlayerSizeChange(NaturalSize());
 
   delegate_->DidPlay(delegate_id_);
 
@@ -1321,7 +1321,7 @@ void WebMediaPlayerMS::TriggerResize() {
   if (HasVideo())
     get_client()->SizeChanged();
 
-  delegate_->DidPlayerSizeChange(delegate_id_, NaturalSize());
+  client_->DidPlayerSizeChange(NaturalSize());
 }
 
 void WebMediaPlayerMS::SetGpuMemoryBufferVideoForTesting(

third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_test.cc

@@ -123,14 +123,6 @@ class FakeWebMediaPlayerDelegate
     playing_ = false;
   }
 
-  void DidPlayerSizeChange(int delegate_id, const gfx::Size& size) override {
-    EXPECT_EQ(delegate_id_, delegate_id);
-  }
-
-  void DidBufferUnderflow(int delegate_id) override {
-    EXPECT_EQ(delegate_id_, delegate_id);
-  }
-
   void DidSeek(int delegate_id) override {
     EXPECT_EQ(delegate_id_, delegate_id);
   }
@@ -602,6 +594,9 @@ class WebMediaPlayerMSTest
                                          base::TimeDelta duration,
                                          base::TimeDelta position) override {}
   void DidDisableAudioOutputSinkChanges() override {}
+  void DidPlayerSizeChange(const gfx::Size& size) override {}
+  void DidBufferUnderflow() override {}
+
   Features GetFeatures() override { return Features(); }
 
   // Implementation of cc::VideoFrameProvider::Client