Fix frozen PiP video spawned from extension background page

We should register the PiP window as the parent of the video in
FrameSink hierarchy to make sure it can receive BeginFrames from the
window's BeginFrameSource if the original embedder of the video (the
extension background page) is not connected to any BeginFrameSource.

Bug: 912874
Change-Id: I1b60487ddc29900351507398b083a62d351970f9
Reviewed-on: https://chromium-review.googlesource.com/c/1403642
Commit-Queue: Saman Sami <samans@chromium.org>
Reviewed-by: François Beaufort <beaufort.francois@gmail.com>
Reviewed-by: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632225}

components/viz/host/host_frame_sink_manager.cc

@@ -223,7 +223,6 @@ bool HostFrameSinkManager::RegisterFrameSinkHierarchy(
                                                   child_frame_sink_id);
 
   FrameSinkData& child_data = frame_sink_data_map_[child_frame_sink_id];
-  DCHECK(child_data.IsFrameSinkRegistered());
   DCHECK(!base::ContainsValue(child_data.parents, parent_frame_sink_id));
   child_data.parents.push_back(parent_frame_sink_id);
 

components/viz/host/host_frame_sink_manager.h

@@ -137,10 +137,7 @@ class VIZ_HOST_EXPORT HostFrameSinkManager
   // the child. If |parent_frame_sink_id| is registered then it will be added as
   // a parent of |child_frame_sink_id| and the function will return true. If
   // |parent_frame_sink_id| is not registered then the function will return
-  // false.
-  //
-  // |child_frame_sink_id| must be registered before calling. A frame sink
-  // can have multiple parents.
+  // false. A frame sink can have multiple parents.
   bool RegisterFrameSinkHierarchy(const FrameSinkId& parent_frame_sink_id,
                                   const FrameSinkId& child_frame_sink_id);
 

content/browser/picture_in_picture/picture_in_picture_window_controller_impl.cc

@@ -19,6 +19,7 @@
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/content_client.h"
 #include "media/base/media_switches.h"
+#include "ui/compositor/layer.h"
 
 namespace content {
 
@@ -126,6 +127,17 @@ void PictureInPictureWindowControllerImpl::EmbedSurface(
   DCHECK(window_);
 
   DCHECK(surface_id.is_valid());
+
+  // TODO(https://crbug.com/925346): We also want to unregister the page that
+  // used to embed the video as its parent.
+  ui::Compositor* compositor = window_->GetLayer()->GetCompositor();
+  if (!surface_id_.is_valid()) {
+    compositor->AddChildFrameSink(surface_id.frame_sink_id());
+  } else if (surface_id_.frame_sink_id() != surface_id.frame_sink_id()) {
+    compositor->RemoveChildFrameSink(surface_id_.frame_sink_id());
+    compositor->AddChildFrameSink(surface_id.frame_sink_id());
+  }
+
   surface_id_ = surface_id;
 
   // Update the media player id in step with the video surface id. If the
@@ -305,6 +317,11 @@ void PictureInPictureWindowControllerImpl::MediaStoppedPlaying(
 void PictureInPictureWindowControllerImpl::OnLeavingPictureInPicture(
     bool should_pause_video,
     bool should_reset_pip_player) {
+  if (window_ && surface_id_.is_valid()) {
+    window_->GetLayer()->GetCompositor()->RemoveChildFrameSink(
+        surface_id_.frame_sink_id());
+  }
+
   if (IsPlayerActive() && should_pause_video) {
     // Pause the current video so there is only one video playing at a time.
     media_player_id_->render_frame_host->Send(new MediaPlayerDelegateMsg_Pause(
@@ -332,10 +349,9 @@ void PictureInPictureWindowControllerImpl::CloseInternal(
   if (initiator_->IsBeingDestroyed())
     return;
 
-  surface_id_ = viz::SurfaceId();
-
   initiator_->SetHasPictureInPictureVideo(false);
   OnLeavingPictureInPicture(should_pause_video, should_reset_pip_player);
+  surface_id_ = viz::SurfaceId();
 }
 
 void PictureInPictureWindowControllerImpl::EnsureWindow() {