Make OnFirstSurfaceActivation optional for OOPIFs

Don't send OnFirstSurfaceActivation to browser for OOPIFs when surface
sync is enabled.

Bug: 893850
Change-Id: I3afda38af2ade88424d28181c51294025c08dd6d
Reviewed-on: https://chromium-review.googlesource.com/c/1336051
Commit-Queue: Saman Sami <samans@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608181}

content/browser/frame_host/render_widget_host_view_guest.cc

@@ -241,9 +241,9 @@ gfx::Rect RenderWidgetHostViewGuest::GetBoundsInRootWindow() {
 
 gfx::PointF RenderWidgetHostViewGuest::TransformPointToRootCoordSpaceF(
     const gfx::PointF& point) {
+  viz::SurfaceId surface_id = GetCurrentSurfaceId();
   // LocalSurfaceId is not needed in Viz hit-test.
-  if (!guest_ ||
-      (!use_viz_hit_test_ && !last_activated_surface_info_.is_valid())) {
+  if (!guest_ || (!use_viz_hit_test_ && !surface_id.is_valid())) {
     return point;
   }
 
@@ -255,8 +255,8 @@ gfx::PointF RenderWidgetHostViewGuest::TransformPointToRootCoordSpaceF(
   // TODO(wjmaclean): If we knew that TransformPointToLocalCoordSpace would
   // guarantee not to change transformed_point on failure, then we could skip
   // checking the function return value and directly return transformed_point.
-  if (!root_rwhv->TransformPointToLocalCoordSpace(
-          point, last_activated_surface_info_.id(), &transformed_point)) {
+  if (!root_rwhv->TransformPointToLocalCoordSpace(point, surface_id,
+                                                  &transformed_point)) {
     return point;
   }
   return transformed_point;
@@ -266,19 +266,19 @@ bool RenderWidgetHostViewGuest::TransformPointToLocalCoordSpaceLegacy(
     const gfx::PointF& point,
     const viz::SurfaceId& original_surface,
     gfx::PointF* transformed_point) {
+  viz::SurfaceId surface_id = GetCurrentSurfaceId();
   *transformed_point = point;
-  if (!guest_ || !last_activated_surface_info_.is_valid())
+  if (!guest_ || !surface_id.is_valid())
     return false;
 
-  if (original_surface == last_activated_surface_info_.id())
+  if (original_surface == surface_id)
     return true;
 
   *transformed_point =
       gfx::ConvertPointToPixel(current_surface_scale_factor(), point);
   viz::SurfaceHittest hittest(nullptr,
                               GetFrameSinkManager()->surface_manager());
-  if (!hittest.TransformPointToTargetSurface(original_surface,
-                                             last_activated_surface_info_.id(),
+  if (!hittest.TransformPointToTargetSurface(original_surface, surface_id,
                                              transformed_point)) {
     return false;
   }

content/browser/renderer_host/render_widget_host_view_child_frame.cc

@@ -82,7 +82,10 @@ RenderWidgetHostViewChildFrame::RenderWidgetHostViewChildFrame(
     frame_sink_id_ = viz::FrameSinkId();
   } else {
     GetHostFrameSinkManager()->RegisterFrameSinkId(
-        frame_sink_id_, this, viz::ReportFirstSurfaceActivation::kYes);
+        frame_sink_id_, this,
+        enable_surface_synchronization_
+            ? viz::ReportFirstSurfaceActivation::kNo
+            : viz::ReportFirstSurfaceActivation::kYes);
     GetHostFrameSinkManager()->SetFrameSinkDebugLabel(
         frame_sink_id_, "RenderWidgetHostViewChildFrame");
     CreateCompositorFrameSinkSupport();
@@ -228,7 +231,7 @@ bool RenderWidgetHostViewChildFrame::HasFocus() const {
 }
 
 bool RenderWidgetHostViewChildFrame::IsSurfaceAvailableForCopy() const {
-  return has_frame_;
+  return GetLocalSurfaceIdAllocation().IsValid();
 }
 
 void RenderWidgetHostViewChildFrame::EnsureSurfaceSynchronizedForLayoutTest() {
@@ -587,7 +590,6 @@ void RenderWidgetHostViewChildFrame::DidCreateNewRendererCompositorFrameSink(
   ResetCompositorFrameSinkSupport();
   renderer_compositor_frame_sink_ = renderer_compositor_frame_sink;
   CreateCompositorFrameSinkSupport();
-  has_frame_ = false;
 }
 
 void RenderWidgetHostViewChildFrame::SetParentFrameSinkId(
@@ -616,6 +618,8 @@ void RenderWidgetHostViewChildFrame::SetParentFrameSinkId(
 void RenderWidgetHostViewChildFrame::SendSurfaceInfoToEmbedder() {
   if (features::IsMultiProcessMash())
     return;
+  if (enable_surface_synchronization_)
+    return;
   if (!last_activated_surface_info_.is_valid())
     return;
   FirstSurfaceActivation(last_activated_surface_info_);
@@ -644,15 +648,6 @@ void RenderWidgetHostViewChildFrame::OnDidNotProduceFrame(
   support_->DidNotProduceFrame(ack);
 }
 
-void RenderWidgetHostViewChildFrame::ProcessFrameSwappedCallbacks() {
-  std::vector<base::OnceClosure> process_callbacks;
-  // Swap the vectors to avoid re-entrancy issues due to calls to
-  // RegisterFrameSwappedCallback() while running the OnceClosures.
-  process_callbacks.swap(frame_swapped_callbacks_);
-  for (base::OnceClosure& callback : process_callbacks)
-    std::move(callback).Run();
-}
-
 void RenderWidgetHostViewChildFrame::TransformPointToRootSurface(
     gfx::PointF* point) {
   // This function is called by RenderWidgetHostInputEventRouter only for
@@ -742,14 +737,13 @@ bool RenderWidgetHostViewChildFrame::HasSize() const {
 
 gfx::PointF RenderWidgetHostViewChildFrame::TransformPointToRootCoordSpaceF(
     const gfx::PointF& point) {
+  viz::SurfaceId surface_id = GetCurrentSurfaceId();
   // LocalSurfaceId is not needed in Viz hit-test.
-  if (!frame_connector_ ||
-      (!use_viz_hit_test_ && !last_activated_surface_info_.is_valid())) {
+  if (!frame_connector_ || (!use_viz_hit_test_ && !surface_id.is_valid())) {
     return point;
   }
 
-  return frame_connector_->TransformPointToRootCoordSpace(
-      point, last_activated_surface_info_.id());
+  return frame_connector_->TransformPointToRootCoordSpace(point, surface_id);
 }
 
 bool RenderWidgetHostViewChildFrame::TransformPointToLocalCoordSpaceLegacy(
@@ -757,12 +751,12 @@ bool RenderWidgetHostViewChildFrame::TransformPointToLocalCoordSpaceLegacy(
     const viz::SurfaceId& original_surface,
     gfx::PointF* transformed_point) {
   *transformed_point = point;
-  if (!frame_connector_ || !last_activated_surface_info_.is_valid())
+  viz::SurfaceId surface_id = GetCurrentSurfaceId();
+  if (!frame_connector_ || !surface_id.is_valid())
     return false;
 
   return frame_connector_->TransformPointToLocalCoordSpaceLegacy(
-      point, original_surface, last_activated_surface_info_.id(),
-      transformed_point);
+      point, original_surface, surface_id, transformed_point);
 }
 
 bool RenderWidgetHostViewChildFrame::TransformPointToCoordSpaceForView(
@@ -770,9 +764,9 @@ bool RenderWidgetHostViewChildFrame::TransformPointToCoordSpaceForView(
     RenderWidgetHostViewBase* target_view,
     gfx::PointF* transformed_point,
     viz::EventSource source) {
+  viz::SurfaceId surface_id = GetCurrentSurfaceId();
   // LocalSurfaceId is not needed in Viz hit-test.
-  if (!frame_connector_ ||
-      (!use_viz_hit_test_ && !last_activated_surface_info_.is_valid())) {
+  if (!frame_connector_ || (!use_viz_hit_test_ && !surface_id.is_valid())) {
     return false;
   }
 
@@ -782,8 +776,7 @@ bool RenderWidgetHostViewChildFrame::TransformPointToCoordSpaceForView(
   }
 
   return frame_connector_->TransformPointToCoordSpaceForView(
-      point, target_view, last_activated_surface_info_.id(), transformed_point,
-      source);
+      point, target_view, surface_id, transformed_point, source);
 }
 
 gfx::PointF RenderWidgetHostViewChildFrame::TransformRootPointToViewCoordSpace(
@@ -840,21 +833,12 @@ void RenderWidgetHostViewChildFrame::ShowDefinitionForSelection() {
 void RenderWidgetHostViewChildFrame::SpeakSelection() {}
 #endif  // defined(OS_MACOSX)
 
-void RenderWidgetHostViewChildFrame::RegisterFrameSwappedCallback(
-    base::OnceClosure callback) {
-  frame_swapped_callbacks_.emplace_back(std::move(callback));
-}
-
 void RenderWidgetHostViewChildFrame::CopyFromSurface(
     const gfx::Rect& src_subrect,
     const gfx::Size& output_size,
     base::OnceCallback<void(const SkBitmap&)> callback) {
   if (!IsSurfaceAvailableForCopy()) {
-    // Defer submitting the copy request until after a frame is drawn, at which
-    // point we should be guaranteed that the surface is available.
-    RegisterFrameSwappedCallback(base::BindOnce(
-        &RenderWidgetHostViewChildFrame::CopyFromSurface, AsWeakPtr(),
-        src_subrect, output_size, std::move(callback)));
+    std::move(callback).Run(SkBitmap());
     return;
   }
 
@@ -869,11 +853,13 @@ void RenderWidgetHostViewChildFrame::CopyFromSurface(
               std::move(callback)));
 
   if (src_subrect.IsEmpty()) {
-    request->set_area(gfx::Rect(last_activated_surface_info_.size_in_pixels()));
+    request->set_area(gfx::Rect(GetCompositorViewportPixelSize()));
   } else {
+    ScreenInfo screen_info;
+    GetScreenInfo(&screen_info);
     // |src_subrect| is in DIP coordinates; convert to Surface coordinates.
-    request->set_area(gfx::ScaleToRoundedRect(
-        src_subrect, last_activated_surface_info_.device_scale_factor()));
+    request->set_area(
+        gfx::ScaleToRoundedRect(src_subrect, screen_info.device_scale_factor));
   }
 
   if (!output_size.IsEmpty()) {
@@ -889,8 +875,8 @@ void RenderWidgetHostViewChildFrame::CopyFromSurface(
         gfx::Vector2d(output_size.width(), output_size.height()));
   }
 
-  GetHostFrameSinkManager()->RequestCopyOfOutput(
-      last_activated_surface_info_.id(), std::move(request));
+  GetHostFrameSinkManager()->RequestCopyOfOutput(GetCurrentSurfaceId(),
+                                                 std::move(request));
 }
 
 void RenderWidgetHostViewChildFrame::ReclaimResources(
@@ -913,10 +899,12 @@ void RenderWidgetHostViewChildFrame::OnBeginFramePausedChanged(bool paused) {
 
 void RenderWidgetHostViewChildFrame::OnFirstSurfaceActivation(
     const viz::SurfaceInfo& surface_info) {
+  if (enable_surface_synchronization_) {
+    NOTREACHED();
+    return;
+  }
   last_activated_surface_info_ = surface_info;
-  has_frame_ = true;
   FirstSurfaceActivation(surface_info);
-  ProcessFrameSwappedCallbacks();
 }
 
 void RenderWidgetHostViewChildFrame::OnFrameTokenChanged(uint32_t frame_token) {

content/browser/renderer_host/render_widget_host_view_child_frame.h

@@ -234,8 +234,6 @@ class CONTENT_EXPORT RenderWidgetHostViewChildFrame
 
   void UpdateRenderThrottlingStatus();
 
-  bool has_frame() { return has_frame_; }
-
   ui::TextInputType GetTextInputType() const;
   bool GetSelectionRange(gfx::Range* range) const;
 
@@ -320,7 +318,6 @@ class CONTENT_EXPORT RenderWidgetHostViewChildFrame
 
   const bool enable_viz_;
   const bool enable_surface_synchronization_;
-  bool has_frame_ = false;
   viz::mojom::CompositorFrameSinkClient* renderer_compositor_frame_sink_ =
       nullptr;
 

content/browser/renderer_host/render_widget_host_view_child_frame_unittest.cc

@@ -198,6 +198,9 @@ TEST_F(RenderWidgetHostViewChildFrameTest, VisibilityTest) {
 // Verify that RenderWidgetHostViewChildFrame passes the child's SurfaceId to
 // FrameConnectorDelegate to be sent to the embedding renderer.
 TEST_F(RenderWidgetHostViewChildFrameTest, PassesSurfaceId) {
+  if (features::IsSurfaceSynchronizationEnabled())
+    return;
+
   gfx::Size view_size(100, 100);
   gfx::Rect view_rect(view_size);
   float scale_factor = 1.f;