Picture-in-Picture: move UpdatePictureInPictureSurfaceId to //content…

… and IPC.

Before this change, the logic was in //chrome but would only rely on //content
concepts. Keeping it in //content reduces the level of plumbing. It's also
moving from frame.mojom to the media IPCs to be around the other PIP calls.

Bug: None
Change-Id: I401126871b48feaf551001ebdfc2f0f319ec75e0
Reviewed-on: https://chromium-review.googlesource.com/1065823
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Reviewed-by: apacible <apacible@chromium.org>
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560830}

chrome/browser/ui/browser.cc

@@ -1416,26 +1416,6 @@ void Browser::OnDidBlockFramebust(content::WebContents* web_contents,
       url, FramebustBlockTabHelper::ClickCallback());
 }
 
-void Browser::UpdatePictureInPictureSurfaceId(const viz::SurfaceId& surface_id,
-                                              const gfx::Size& natural_size) {
-  // TODO(mlamouri): update this to be only about updating the SurfaceId.
-  if (!pip_window_controller_ ||
-      pip_window_controller_->GetInitiatorWebContents() !=
-          tab_strip_model_->GetActiveWebContents()) {
-    // If there was already a controller, close the existing window before
-    // creating the next one.
-    if (pip_window_controller_)
-      pip_window_controller_->Close();
-
-    // Update |pip_window_controller_| for the current content::WebContents.
-    pip_window_controller_ =
-        content::PictureInPictureWindowController::GetOrCreateForWebContents(
-            tab_strip_model_->GetActiveWebContents());
-  }
-  pip_window_controller_->EmbedSurface(surface_id, natural_size);
-  pip_window_controller_->Show();
-}
-
 gfx::Size Browser::EnterPictureInPicture(const viz::SurfaceId& surface_id,
                                          const gfx::Size& natural_size) {
   if (!pip_window_controller_ ||
@@ -1451,6 +1431,7 @@ gfx::Size Browser::EnterPictureInPicture(const viz::SurfaceId& surface_id,
         content::PictureInPictureWindowController::GetOrCreateForWebContents(
             tab_strip_model_->GetActiveWebContents());
   }
+
   pip_window_controller_->EmbedSurface(surface_id, natural_size);
   return pip_window_controller_->Show();
 }

chrome/browser/ui/browser.h

@@ -502,8 +502,6 @@ class Browser : public TabStripModelObserver,
                            bool is_audible) override;
   void OnDidBlockFramebust(content::WebContents* web_contents,
                            const GURL& url) override;
-  void UpdatePictureInPictureSurfaceId(const viz::SurfaceId& surface_id,
-                                       const gfx::Size& natural_size) override;
   gfx::Size EnterPictureInPicture(const viz::SurfaceId&,
                                   const gfx::Size&) override;
   void ExitPictureInPicture() override;

content/browser/frame_host/render_frame_host_impl.cc

@@ -2386,13 +2386,6 @@ void RenderFrameHostImpl::FrameSizeChanged(const gfx::Size& frame_size) {
   frame_size_ = frame_size;
 }
 
-void RenderFrameHostImpl::OnUpdatePictureInPictureSurfaceId(
-    const viz::SurfaceId& surface_id,
-    const gfx::Size& natural_size) {
-  if (delegate_)
-    delegate_->UpdatePictureInPictureSurfaceId(surface_id, natural_size);
-}
-
 void RenderFrameHostImpl::OnDidBlockFramebust(const GURL& url) {
   delegate_->OnDidBlockFramebust(url);
 }

content/browser/frame_host/render_frame_host_impl.h

@@ -930,9 +930,6 @@ class CONTENT_EXPORT RenderFrameHostImpl
   void CancelInitialHistoryLoad() override;
   void UpdateEncoding(const std::string& encoding) override;
   void FrameSizeChanged(const gfx::Size& frame_size) override;
-  void OnUpdatePictureInPictureSurfaceId(
-      const viz::SurfaceId& surface_id,
-      const gfx::Size& natural_size) override;
 
   // Registers Mojo interfaces that this frame host makes available.
   void RegisterMojoInterfaces();

content/browser/media/media_web_contents_observer.cc

@@ -9,6 +9,7 @@
 #include "build/build_config.h"
 #include "content/browser/media/audible_metrics.h"
 #include "content/browser/media/audio_stream_monitor.h"
+#include "content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/media/media_player_delegate_messages.h"
 #include "content/public/browser/render_frame_host.h"
@@ -138,6 +139,9 @@ bool MediaWebContentsObserver::OnMessageReceived(
         OnPictureInPictureModeStarted)
     IPC_MESSAGE_HANDLER(MediaPlayerDelegateHostMsg_OnPictureInPictureModeEnded,
                         OnPictureInPictureModeEnded)
+    IPC_MESSAGE_HANDLER(
+        MediaPlayerDelegateHostMsg_OnPictureInPictureSurfaceChanged,
+        OnPictureInPictureSurfaceChanged)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -315,6 +319,22 @@ void MediaWebContentsObserver::OnPictureInPictureModeEnded(
           render_frame_host->GetRoutingID(), delegate_id, request_id));
 }
 
+void MediaWebContentsObserver::OnPictureInPictureSurfaceChanged(
+    RenderFrameHost* render_frame_host,
+    int delegate_id,
+    const viz::SurfaceId& surface_id,
+    const gfx::Size& natural_size) {
+  DCHECK(surface_id.is_valid());
+  DCHECK(pip_player_);
+
+  PictureInPictureWindowControllerImpl* pip_controller =
+      PictureInPictureWindowControllerImpl::FromWebContents(
+          web_contents_impl());
+  DCHECK(pip_controller);
+
+  pip_controller->EmbedSurface(surface_id, natural_size);
+}
+
 void MediaWebContentsObserver::ClearWakeLocks(
     RenderFrameHost* render_frame_host) {
   std::set<MediaPlayerId> video_players;

content/browser/media/media_web_contents_observer.h

@@ -129,6 +129,10 @@ class CONTENT_EXPORT MediaWebContentsObserver : public WebContentsObserver {
   void OnPictureInPictureModeEnded(RenderFrameHost* render_frame_host,
                                    int delegate_id,
                                    int request_id);
+  void OnPictureInPictureSurfaceChanged(RenderFrameHost*,
+                                        int delegate_id,
+                                        const viz::SurfaceId&,
+                                        const gfx::Size&);
 
   // Clear |render_frame_host|'s tracking entry for its WakeLocks.
   void ClearWakeLocks(RenderFrameHost* render_frame_host);

content/browser/web_contents/web_contents_impl.cc

@@ -4198,13 +4198,6 @@ void WebContentsImpl::PrintCrossProcessSubframe(
                                        subframe_host);
 }
 
-void WebContentsImpl::UpdatePictureInPictureSurfaceId(
-    const viz::SurfaceId& surface_id,
-    const gfx::Size& natural_size) {
-  if (delegate_)
-    delegate_->UpdatePictureInPictureSurfaceId(surface_id, natural_size);
-}
-
 #if defined(OS_ANDROID)
 base::android::ScopedJavaLocalRef<jobject>
 WebContentsImpl::GetJavaRenderFrameHostDelegate() {

content/browser/web_contents/web_contents_impl.h

@@ -588,8 +588,6 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
   void ResourceLoadComplete(
       RenderFrameHost* render_frame_host,
       mojom::ResourceLoadInfoPtr resource_load_information) override;
-  void UpdatePictureInPictureSurfaceId(const viz::SurfaceId& surface_id,
-                                       const gfx::Size& natural_size) override;
   // RenderViewHostDelegate ----------------------------------------------------
   RenderViewHostDelegateView* GetDelegateView() override;
   bool OnMessageReceived(RenderViewHostImpl* render_view_host,

content/common/frame.mojom

@@ -324,10 +324,4 @@ interface FrameHost {
   // correctly set the initial size of the frame in case of a cross-process
   // navigation.
   FrameSizeChanged(gfx.mojom.Size size);
-
-  // Sent by the renderer to update Picture-in-Picture with SurfaceId and video
-  // size to be used to show content in the Picture-in-Picture window.
-  OnUpdatePictureInPictureSurfaceId(
-      viz.mojom.SurfaceId surface_id,
-      gfx.mojom.Size natural_size);
 };

content/common/media/media_player_delegate_messages.h

@@ -113,4 +113,9 @@ IPC_MESSAGE_ROUTED2(MediaPlayerDelegateHostMsg_OnPictureInPictureModeEnded,
                     int /* delegate id */,
                     int /* request_id */)
 
+IPC_MESSAGE_ROUTED3(MediaPlayerDelegateHostMsg_OnPictureInPictureSurfaceChanged,
+                    int /* delegate id */,
+                    viz::SurfaceId /* surface_id */,
+                    gfx::Size /* natural_size */)
+
 #endif  // CONTENT_COMMON_MEDIA_MEDIA_PLAYER_DELEGATE_MESSAGES_H_

content/public/browser/web_contents_delegate.cc

@@ -269,10 +269,6 @@ bool WebContentsDelegate::DoBrowserControlsShrinkBlinkSize() const {
   return false;
 }
 
-void WebContentsDelegate::UpdatePictureInPictureSurfaceId(
-    const viz::SurfaceId& surface_id,
-    const gfx::Size& natural_size) {}
-
 gfx::Size WebContentsDelegate::EnterPictureInPicture(const viz::SurfaceId&,
                                                      const gfx::Size&) {
   return gfx::Size();

content/public/browser/web_contents_delegate.h

@@ -583,11 +583,6 @@ class CONTENT_EXPORT WebContentsDelegate {
                                          RenderFrameHost* subframe_host) const {
   }
 
-  // Updates the Picture-in-Picture controller with the relevant viz::SurfaceId
-  // and natural size of the video to be in Picture-in-Picture mode.
-  virtual void UpdatePictureInPictureSurfaceId(const viz::SurfaceId& surface_id,
-                                               const gfx::Size& natural_size);
-
   // Notifies the Picture-in-Picture controller that there is a new player
   // entering Picture-in-Picture.
   // Returns the size of the Picture-in-Picture window.

content/renderer/media/media_factory.cc

@@ -316,10 +316,7 @@ blink::WebMediaPlayer* MediaFactory::CreateMediaPlayer(
           std::move(metrics_provider),
           base::Bind(&blink::WebSurfaceLayerBridge::Create, layer_tree_view),
           RenderThreadImpl::current()->SharedMainThreadContextProvider(),
-          use_surface_layer_for_video,
-          base::BindRepeating(
-              &RenderFrameImpl::OnPictureInPictureSurfaceIdUpdated,
-              base::Unretained(render_frame_))));
+          use_surface_layer_for_video));
 
   std::unique_ptr<media::VideoFrameCompositor> vfc =
       std::make_unique<media::VideoFrameCompositor>(

content/renderer/media/renderer_webmediaplayer_delegate.cc

@@ -140,6 +140,14 @@ void RendererWebMediaPlayerDelegate::DidPictureInPictureModeEnd(
       routing_id(), delegate_id, request_id));
 }
 
+void RendererWebMediaPlayerDelegate::DidPictureInPictureSurfaceChange(
+    int delegate_id,
+    const viz::SurfaceId& surface_id,
+    const gfx::Size& natural_size) {
+  Send(new MediaPlayerDelegateHostMsg_OnPictureInPictureSurfaceChanged(
+      routing_id(), delegate_id, surface_id, natural_size));
+}
+
 void RendererWebMediaPlayerDelegate::DidPause(int player_id) {
   DVLOG(2) << __func__ << "(" << player_id << ")";
   DCHECK(id_map_.Lookup(player_id));

content/renderer/media/renderer_webmediaplayer_delegate.h

@@ -71,6 +71,9 @@ class CONTENT_EXPORT RendererWebMediaPlayerDelegate
       const gfx::Size&,
       blink::WebMediaPlayer::PipWindowSizeCallback) override;
   void DidPictureInPictureModeEnd(int delegate_id, base::OnceClosure) override;
+  void DidPictureInPictureSurfaceChange(int delegate_id,
+                                        const viz::SurfaceId&,
+                                        const gfx::Size&) override;
 
   // content::RenderFrameObserver overrides.
   void WasHidden() override;

content/renderer/media/stream/webmediaplayer_ms_unittest.cc

@@ -90,6 +90,12 @@ class FakeWebMediaPlayerDelegate
     EXPECT_EQ(delegate_id_, delegate_id);
   }
 
+  void DidPictureInPictureSurfaceChange(int delegate_id,
+                                        const viz::SurfaceId&,
+                                        const gfx::Size&) override {
+    EXPECT_EQ(delegate_id_, delegate_id);
+  }
+
   void DidPause(int delegate_id) override {
     EXPECT_EQ(delegate_id_, delegate_id);
     EXPECT_TRUE(playing_);

content/renderer/render_frame_impl.cc

@@ -7034,12 +7034,6 @@ void RenderFrameImpl::FrameDidCallFocus() {
   Send(new FrameHostMsg_FrameDidCallFocus(routing_id_));
 }
 
-void RenderFrameImpl::OnPictureInPictureSurfaceIdUpdated(
-    const viz::SurfaceId& surface_id,
-    const gfx::Size& natural_size) {
-  GetFrameHost()->OnUpdatePictureInPictureSurfaceId(surface_id, natural_size);
-}
-
 void RenderFrameImpl::SetAccessibilityModeForTest(ui::AXMode new_mode) {
   OnSetAccessibilityMode(new_mode);
 }

content/renderer/render_frame_impl.h

@@ -867,11 +867,6 @@ class CONTENT_EXPORT RenderFrameImpl
   // frame.
   void FrameDidCallFocus();
 
-  // Send viz::SurfaceId and video information to FrameHost to use for
-  // Picture-in-Picture.
-  void OnPictureInPictureSurfaceIdUpdated(const viz::SurfaceId& surface_id,
-                                          const gfx::Size& natural_size);
-
  protected:
   explicit RenderFrameImpl(CreateParams params);
 

content/test/test_render_frame.cc

@@ -109,10 +109,6 @@ class MockFrameHost : public mojom::FrameHost {
 
   void FrameSizeChanged(const gfx::Size& frame_size) override {}
 
-  void OnUpdatePictureInPictureSurfaceId(
-      const viz::SurfaceId& surface_id,
-      const gfx::Size& natural_size) override {}
-
  private:
   std::unique_ptr<FrameHostMsg_DidCommitProvisionalLoad_Params>
       last_commit_params_;

media/blink/webmediaplayer_delegate.h

@@ -123,6 +123,11 @@ class WebMediaPlayerDelegate {
   virtual void DidPictureInPictureModeEnd(int delegate_id,
                                           base::OnceClosure) = 0;
 
+  // Notify that the media player in Picture-in-Picture had a change of surface.
+  virtual void DidPictureInPictureSurfaceChange(int delegate_id,
+                                                const viz::SurfaceId&,
+                                                const gfx::Size&) = 0;
+
   // Notify that playback is stopped. This will drop wake locks and remove any
   // external controls.
   //

media/blink/webmediaplayer_impl.cc

@@ -249,8 +249,7 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
       surface_layer_for_video_enabled_(params->use_surface_layer_for_video()),
       request_routing_token_cb_(params->request_routing_token_cb()),
       overlay_routing_token_(OverlayInfo::RoutingToken()),
-      media_metrics_provider_(params->take_metrics_provider()),
-      pip_surface_info_cb_(params->pip_surface_info_cb()) {
+      media_metrics_provider_(params->take_metrics_provider()) {
   DVLOG(1) << __func__;
   DCHECK(!adjust_allocated_memory_cb_.is_null());
   DCHECK(renderer_factory_selector_);
@@ -416,8 +415,10 @@ void WebMediaPlayerImpl::OnSurfaceIdUpdated(viz::SurfaceId surface_id) {
   // disabled.
   // The viz::SurfaceId may be updated when the video begins playback or when
   // the size of the video changes.
-  if (client_ && client_->IsInPictureInPictureMode())
-    pip_surface_info_cb_.Run(pip_surface_id_, pipeline_metadata_.natural_size);
+  if (client_ && client_->IsInPictureInPictureMode()) {
+    delegate_->DidPictureInPictureSurfaceChange(
+        delegate_id_, surface_id, pipeline_metadata_.natural_size);
+  }
 }
 
 bool WebMediaPlayerImpl::SupportsOverlayFullscreenVideo() {

media/blink/webmediaplayer_impl.h

@@ -921,9 +921,6 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   // route the video to be shown in the Picture-in-Picture window.
   viz::SurfaceId pip_surface_id_;
 
-  // Callback to pass updated information about the current surface info.
-  WebMediaPlayerParams::PipSurfaceInfoCB pip_surface_info_cb_;
-
   DISALLOW_COPY_AND_ASSIGN(WebMediaPlayerImpl);
 };
 

media/blink/webmediaplayer_impl_unittest.cc

@@ -227,6 +227,8 @@ class MockWebMediaPlayerDelegate : public WebMediaPlayerDelegate {
                     blink::WebMediaPlayer::PipWindowSizeCallback));
   MOCK_METHOD2(DidPictureInPictureModeEnd,
                void(int, blink::WebMediaPlayer::PipWindowClosedCallback));
+  MOCK_METHOD3(DidPictureInPictureSurfaceChange,
+               void(int, const viz::SurfaceId&, const gfx::Size&));
 
   void ClearStaleFlag(int player_id) override {
     DCHECK_EQ(player_id_, player_id);
@@ -373,8 +375,7 @@ class WebMediaPlayerImplTest : public testing::Test {
             &WebMediaPlayerImplTest::CreateMockSurfaceLayerBridge,
             base::Unretained(this)),
         viz::TestContextProvider::Create(),
-        base::FeatureList::IsEnabled(media::kUseSurfaceLayerForVideo),
-        base::BindRepeating(pip_surface_info_cb_.Get()));
+        base::FeatureList::IsEnabled(media::kUseSurfaceLayerForVideo));
 
     auto compositor = std::make_unique<StrictMock<MockVideoFrameCompositor>>(
         params->video_frame_compositor_task_runner());
@@ -706,10 +707,6 @@ class WebMediaPlayerImplTest : public testing::Test {
   // The WebMediaPlayerImpl instance under test.
   std::unique_ptr<WebMediaPlayerImpl> wmpi_;
 
-  // Callback used for updating Picture-in-Picture about new Surface info.
-  base::MockCallback<WebMediaPlayerParams::PipSurfaceInfoCB>
-      pip_surface_info_cb_;
-
  private:
   DISALLOW_COPY_AND_ASSIGN(WebMediaPlayerImplTest);
 };
@@ -1356,13 +1353,16 @@ TEST_F(WebMediaPlayerImplTest, PictureInPictureTriggerCallback) {
   InitializeWebMediaPlayerImpl();
 
   EXPECT_CALL(client_, IsInPictureInPictureMode()).WillRepeatedly(Return(true));
+  EXPECT_CALL(delegate_,
+              DidPictureInPictureSurfaceChange(delegate_.player_id(),
+                                               surface_id_, GetNaturalSize()))
+      .Times(2);
 
   wmpi_->OnSurfaceIdUpdated(surface_id_);
 
   EXPECT_CALL(delegate_,
               DidPictureInPictureModeStart(delegate_.player_id(), surface_id_,
                                            GetNaturalSize(), _));
-  EXPECT_CALL(pip_surface_info_cb_, Run(surface_id_, GetNaturalSize()));
 
   wmpi_->EnterPictureInPicture(base::DoNothing());
   wmpi_->OnSurfaceIdUpdated(surface_id_);

media/blink/webmediaplayer_params.cc

@@ -32,8 +32,7 @@ WebMediaPlayerParams::WebMediaPlayerParams(
     base::Callback<std::unique_ptr<blink::WebSurfaceLayerBridge>(
         blink::WebSurfaceLayerBridgeObserver*)> create_bridge_callback,
     scoped_refptr<viz::ContextProvider> context_provider,
-    bool use_surface_layer_for_video,
-    const PipSurfaceInfoCB& pip_surface_info_cb)
+    bool use_surface_layer_for_video)
     : defer_load_cb_(defer_load_cb),
       audio_renderer_sink_(audio_renderer_sink),
       media_log_(std::move(media_log)),
@@ -55,8 +54,7 @@ WebMediaPlayerParams::WebMediaPlayerParams(
       metrics_provider_(std::move(metrics_provider)),
       create_bridge_callback_(create_bridge_callback),
       context_provider_(std::move(context_provider)),
-      use_surface_layer_for_video_(use_surface_layer_for_video),
-      pip_surface_info_cb_(pip_surface_info_cb) {}
+      use_surface_layer_for_video_(use_surface_layer_for_video) {}
 
 WebMediaPlayerParams::~WebMediaPlayerParams() = default;