Plumb proper OOPIF compositor viewport rect.

This CL converts OOPIFs from using a compositor viewport that is the
entire size of the content layer, to a much smaller one based on the
visible intersection of the OOPIF's content with its parent's viewport.

This should decrease compositor resources used by the OOPIF, and
prevent excessively large tiles being produced.

Bug: 852348
Change-Id: I27977e2513d727bc6ba1a414504157cef3436a03
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1778362
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694066}

content/browser/renderer_host/frame_connector_delegate.cc

@@ -48,6 +48,8 @@ void FrameConnectorDelegate::SynchronizeVisualProperties(
   render_widget_host->SetPageScaleState(
       visual_properties.page_scale_factor,
       visual_properties.is_pinch_gesture_active);
+  render_widget_host->SetCompositorViewport(
+      visual_properties.compositor_viewport);
 
   render_widget_host->SynchronizeVisualProperties();
 }

content/browser/renderer_host/render_widget_host_impl.cc

@@ -811,8 +811,16 @@ VisualProperties RenderWidgetHostImpl::GetVisualProperties() {
     visual_properties.new_size = view_->GetRequestedRendererSize();
     visual_properties.capture_sequence_number =
         view_->GetCaptureSequenceNumber();
-    visual_properties.compositor_viewport_pixel_size =
-        view_->GetCompositorViewportPixelSize();
+    // For OOPIFs, use the compositor viewport received from the FrameConnector.
+    visual_properties.compositor_viewport_pixel_rect =
+        view_->IsRenderWidgetHostViewChildFrame() &&
+                !view_->IsRenderWidgetHostViewGuest()
+            ? gfx::ScaleToEnclosingRect(
+                  compositor_viewport_,
+                  IsUseZoomForDSFEnabled()
+                      ? 1.f
+                      : visual_properties.screen_info.device_scale_factor)
+            : gfx::Rect(view_->GetCompositorViewportPixelSize());
     visual_properties.visible_viewport_size = view_->GetVisibleViewportSize();
     // TODO(ccameron): GetLocalSurfaceId is not synchronized with the device
     // scale factor of the surface. Fix this.
@@ -1991,6 +1999,11 @@ void RenderWidgetHostImpl::SetPageScaleState(float page_scale_factor,
   is_pinch_gesture_active_ = is_pinch_gesture_active;
 }
 
+void RenderWidgetHostImpl::SetCompositorViewport(
+    const gfx::Rect& compositor_viewport) {
+  compositor_viewport_ = compositor_viewport;
+}
+
 void RenderWidgetHostImpl::Destroy(bool also_delete) {
   DCHECK(!destroyed_);
   destroyed_ = true;
@@ -2247,8 +2260,8 @@ bool RenderWidgetHostImpl::DidVisualPropertiesSizeChange(
                new_visual_properties.max_size_for_auto_resize)) ||
          (!old_visual_properties.auto_resize_enabled &&
           (old_visual_properties.new_size != new_visual_properties.new_size ||
-           (old_visual_properties.compositor_viewport_pixel_size.IsEmpty() &&
-            !new_visual_properties.compositor_viewport_pixel_size.IsEmpty())));
+           (old_visual_properties.compositor_viewport_pixel_rect.IsEmpty() &&
+            !new_visual_properties.compositor_viewport_pixel_rect.IsEmpty())));
 }
 
 // static
@@ -2263,7 +2276,7 @@ bool RenderWidgetHostImpl::DoesVisualPropertiesNeedAck(
       g_check_for_pending_visual_properties_ack &&
       !new_visual_properties.auto_resize_enabled &&
       !new_visual_properties.new_size.IsEmpty() &&
-      !new_visual_properties.compositor_viewport_pixel_size.IsEmpty() &&
+      !new_visual_properties.compositor_viewport_pixel_rect.IsEmpty() &&
       new_visual_properties.local_surface_id_allocation;
 
   // If acking is applicable, then check if there has been an
@@ -2311,8 +2324,10 @@ bool RenderWidgetHostImpl::StoredVisualPropertiesNeedsUpdate(
   return zoom_changed || size_changed || parent_local_surface_id_changed ||
          old_visual_properties->screen_info !=
              new_visual_properties.screen_info ||
-         old_visual_properties->compositor_viewport_pixel_size !=
-             new_visual_properties.compositor_viewport_pixel_size ||
+         old_visual_properties->compositor_viewport_pixel_rect !=
+             new_visual_properties.compositor_viewport_pixel_rect ||
+         old_visual_properties->compositor_viewport_pixel_rect !=
+             new_visual_properties.compositor_viewport_pixel_rect ||
          old_visual_properties->is_fullscreen_granted !=
              new_visual_properties.is_fullscreen_granted ||
          old_visual_properties->display_mode !=

content/browser/renderer_host/render_widget_host_impl.h

@@ -598,6 +598,9 @@ class CONTENT_EXPORT RenderWidgetHostImpl
   // properties of this widget.
   VisualProperties GetVisualProperties();
 
+  // Tracks the compositor viewport requested for an OOPIF subframe.
+  void SetCompositorViewport(const gfx::Rect& compositor_viewport);
+
   // Sets the |visual_properties| that were sent to the renderer bundled with
   // the request to create a new RenderWidget.
   void SetInitialVisualProperties(const VisualProperties& visual_properties);
@@ -1079,6 +1082,8 @@ class CONTENT_EXPORT RenderWidgetHostImpl
   // True when the renderer is currently undergoing a pinch-zoom gesture.
   bool is_pinch_gesture_active_ = false;
 
+  gfx::Rect compositor_viewport_;
+
   bool waiting_for_screen_rects_ack_ = false;
   gfx::Rect last_view_screen_rect_;
   gfx::Rect last_window_screen_rect_;

content/browser/renderer_host/render_widget_host_unittest.cc

@@ -1572,14 +1572,15 @@ TEST_F(RenderWidgetHostTest, RendererExitedResetsIsHidden) {
 
 TEST_F(RenderWidgetHostTest, VisualProperties) {
   gfx::Rect bounds(0, 0, 100, 100);
-  gfx::Size compositor_viewport_pixel_size(40, 50);
+  gfx::Rect compositor_viewport_pixel_rect(40, 50);
   view_->SetBounds(bounds);
-  view_->SetMockCompositorViewportPixelSize(compositor_viewport_pixel_size);
+  view_->SetMockCompositorViewportPixelSize(
+      compositor_viewport_pixel_rect.size());
 
   VisualProperties visual_properties = host_->GetVisualProperties();
   EXPECT_EQ(bounds.size(), visual_properties.new_size);
-  EXPECT_EQ(compositor_viewport_pixel_size,
-            visual_properties.compositor_viewport_pixel_size);
+  EXPECT_EQ(compositor_viewport_pixel_rect,
+            visual_properties.compositor_viewport_pixel_rect);
 }
 
 // Make sure no dragging occurs after renderer exited. See crbug.com/704832.

content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

@@ -2603,7 +2603,8 @@ TEST_F(RenderWidgetHostViewAuraTest, CompositorViewportPixelSizeWithScale) {
     EXPECT_EQ("100x100", std::get<0>(params).new_size.ToString());  // dip size
     EXPECT_EQ("100x100",
               std::get<0>(params)
-                  .compositor_viewport_pixel_size.ToString());  // backing size
+                  .compositor_viewport_pixel_rect.size()
+                  .ToString());  // backing size
   }
 
   widget_host_->ResetSentVisualProperties();

content/browser/renderer_host/render_widget_host_view_child_frame_unittest.cc

@@ -275,8 +275,8 @@ TEST_F(RenderWidgetHostViewChildFrameTest,
 
   widget_host_->Init();
 
-  constexpr gfx::Size compositor_viewport_pixel_size(100, 100);
-  constexpr gfx::Rect screen_space_rect(compositor_viewport_pixel_size);
+  constexpr gfx::Rect compositor_viewport_pixel_rect(100, 100);
+  constexpr gfx::Rect screen_space_rect(compositor_viewport_pixel_rect);
   viz::ParentLocalSurfaceIdAllocator allocator;
   allocator.GenerateId();
   viz::LocalSurfaceIdAllocation local_surface_id_allocation =
@@ -287,7 +287,8 @@ TEST_F(RenderWidgetHostViewChildFrameTest,
 
   FrameVisualProperties visual_properties;
   visual_properties.screen_space_rect = screen_space_rect;
-  visual_properties.local_frame_size = compositor_viewport_pixel_size;
+  visual_properties.compositor_viewport = compositor_viewport_pixel_rect;
+  visual_properties.local_frame_size = compositor_viewport_pixel_rect.size();
   visual_properties.capture_sequence_number = 123u;
   visual_properties.local_surface_id_allocation = local_surface_id_allocation;
   test_frame_connector_->SynchronizeVisualProperties(frame_sink_id,
@@ -300,8 +301,8 @@ TEST_F(RenderWidgetHostViewChildFrameTest,
   ASSERT_NE(nullptr, resize_msg);
   WidgetMsg_SynchronizeVisualProperties::Param params;
   WidgetMsg_SynchronizeVisualProperties::Read(resize_msg, &params);
-  EXPECT_EQ(compositor_viewport_pixel_size,
-            std::get<0>(params).compositor_viewport_pixel_size);
+  EXPECT_EQ(compositor_viewport_pixel_rect,
+            std::get<0>(params).compositor_viewport_pixel_rect);
   EXPECT_EQ(screen_space_rect.size(), std::get<0>(params).new_size);
   EXPECT_EQ(local_surface_id_allocation,
             std::get<0>(params).local_surface_id_allocation);

content/browser/site_per_process_browsertest.cc

@@ -15253,4 +15253,86 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
   }
 }
 
+class SitePerProcessCompositorViewportBrowserTest
+    : public SitePerProcessBrowserTest,
+      public testing::WithParamInterface<double> {
+ public:
+  SitePerProcessCompositorViewportBrowserTest() {}
+
+ protected:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    SitePerProcessBrowserTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitchASCII(switches::kForceDeviceScaleFactor,
+                                    base::StringPrintf("%f", GetParam()));
+  }
+};
+
+IN_PROC_BROWSER_TEST_P(SitePerProcessCompositorViewportBrowserTest,
+                       OopifCompositorViewportSizeRelativeToParent) {
+  // Load page with very tall OOPIF.
+  GURL main_url(
+      embedded_test_server()->GetURL("a.com", "/super_tall_parent.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+
+  // It is safe to obtain the root frame tree node here, as it doesn't change.
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()
+                            ->root();
+  ASSERT_EQ(1U, root->child_count());
+
+  FrameTreeNode* child = root->child_at(0);
+
+  GURL nested_site_url(
+      embedded_test_server()->GetURL("b.com", "/super_tall_page.html"));
+  NavigateFrameToURL(child, nested_site_url);
+
+  EXPECT_EQ(
+      " Site A ------------ proxies for B\n"
+      "   +--Site B ------- proxies for A\n"
+      "Where A = http://a.com/\n"
+      "      B = http://b.com/",
+      DepictFrameTree(root));
+
+  // Observe frame submission from parent.
+  RenderFrameSubmissionObserver parent_observer(
+      root->current_frame_host()
+          ->GetRenderWidgetHost()
+          ->render_frame_metadata_provider());
+  parent_observer.WaitForAnyFrameSubmission();
+  gfx::Size parent_viewport_size =
+      parent_observer.LastRenderFrameMetadata().viewport_size_in_pixels;
+
+  // Observe frame submission from child.
+  RenderFrameSubmissionObserver child_observer(
+      child->current_frame_host()
+          ->GetRenderWidgetHost()
+          ->render_frame_metadata_provider());
+  child_observer.WaitForAnyFrameSubmission();
+  gfx::Size child_viewport_size =
+      child_observer.LastRenderFrameMetadata().viewport_size_in_pixels;
+
+  // Verify child's compositor viewport is no more than about 30% larger than
+  // the parent's. See RemoteFrameView::GetCompositingRect() for explanation of
+  // the choice of 30%. Add +1 to child viewport height to account for rounding.
+  EXPECT_GE(1.3f * parent_viewport_size.height(),
+            1.f * (child_viewport_size.height() - 1));
+
+  // Verify the child's ViewBounds are much larger.
+  RenderWidgetHostViewBase* child_rwhv = static_cast<RenderWidgetHostViewBase*>(
+      child->current_frame_host()->GetRenderWidgetHost()->GetView());
+  // 30,000 is based on div/iframe sizes in the test HTML files.
+  EXPECT_LT(30000, child_rwhv->GetViewBounds().height());
+}
+
+#if defined(OS_ANDROID)
+// Android doesn't support forcing device scale factor in tests.
+INSTANTIATE_TEST_SUITE_P(SitePerProcess,
+                         SitePerProcessCompositorViewportBrowserTest,
+                         testing::Values(1.0));
+#else
+INSTANTIATE_TEST_SUITE_P(SitePerProcess,
+                         SitePerProcessCompositorViewportBrowserTest,
+                         testing::Values(1.0, 1.5, 2.0));
+#endif
+
 }  // namespace content

content/common/frame_messages.h

@@ -268,6 +268,7 @@ IPC_STRUCT_TRAITS_BEGIN(content::FrameVisualProperties)
   IPC_STRUCT_TRAITS_MEMBER(max_size_for_auto_resize)
   IPC_STRUCT_TRAITS_MEMBER(screen_space_rect)
   IPC_STRUCT_TRAITS_MEMBER(local_frame_size)
+  IPC_STRUCT_TRAITS_MEMBER(compositor_viewport)
   IPC_STRUCT_TRAITS_MEMBER(capture_sequence_number)
   IPC_STRUCT_TRAITS_MEMBER(zoom_level)
   IPC_STRUCT_TRAITS_MEMBER(page_scale_factor)

content/common/frame_visual_properties.h

@@ -38,6 +38,9 @@ struct CONTENT_EXPORT FrameVisualProperties {
 
   gfx::Size local_frame_size;
 
+  // The size of the compositor viewport, to match the sub-frame's surface.
+  gfx::Rect compositor_viewport;
+
   uint32_t capture_sequence_number = 0u;
 
   // This represents the page zoom level for a WebContents.

content/common/visual_properties.h

@@ -37,10 +37,10 @@ struct CONTENT_EXPORT VisualProperties {
   // The size for the widget in DIPs.
   gfx::Size new_size;
 
-  // The size of compositor's viewport in pixels. Note that this may differ
-  // from a ScaleToCeiledSize of |new_size| due to Android's keyboard or due
-  // to rounding particulars.
-  gfx::Size compositor_viewport_pixel_size;
+  // The rect of compositor's viewport in pixels. Note that this may differ in
+  // size from a ScaleToCeiledSize of |new_size| due to Android's keyboard or
+  // due to rounding particulars.
+  gfx::Rect compositor_viewport_pixel_rect;
 
   // Whether or not Blink's viewport size should be shrunk by the height of the
   // URL-bar (always false on platforms where URL-bar hiding isn't supported).

content/common/widget_messages.h

@@ -55,7 +55,7 @@ IPC_STRUCT_TRAITS_BEGIN(content::VisualProperties)
   IPC_STRUCT_TRAITS_MEMBER(min_size_for_auto_resize)
   IPC_STRUCT_TRAITS_MEMBER(max_size_for_auto_resize)
   IPC_STRUCT_TRAITS_MEMBER(new_size)
-  IPC_STRUCT_TRAITS_MEMBER(compositor_viewport_pixel_size)
+  IPC_STRUCT_TRAITS_MEMBER(compositor_viewport_pixel_rect)
   IPC_STRUCT_TRAITS_MEMBER(browser_controls_shrink_blink_size)
   IPC_STRUCT_TRAITS_MEMBER(scroll_focused_node_into_view)
   IPC_STRUCT_TRAITS_MEMBER(top_controls_height)

content/public/test/render_view_test.cc

@@ -691,7 +691,7 @@ void RenderViewTest::Resize(gfx::Size new_size,
   VisualProperties visual_properties;
   visual_properties.screen_info = ScreenInfo();
   visual_properties.new_size = new_size;
-  visual_properties.compositor_viewport_pixel_size = new_size;
+  visual_properties.compositor_viewport_pixel_rect = gfx::Rect(new_size);
   visual_properties.top_controls_height = 0.f;
   visual_properties.browser_controls_shrink_blink_size = false;
   visual_properties.is_fullscreen_granted = is_fullscreen_granted;

content/renderer/render_frame_impl_browsertest.cc

@@ -222,7 +222,7 @@ TEST_F(RenderFrameImplTest, FrameResize) {
   gfx::Size size(200, 200);
   visual_properties.screen_info = ScreenInfo();
   visual_properties.new_size = size;
-  visual_properties.compositor_viewport_pixel_size = size;
+  visual_properties.compositor_viewport_pixel_rect = gfx::Rect(size);
   visual_properties.visible_viewport_size = size;
   visual_properties.top_controls_height = 0.f;
   visual_properties.browser_controls_shrink_blink_size = false;

content/renderer/render_frame_proxy.cc

@@ -617,6 +617,9 @@ void RenderFrameProxy::SynchronizeVisualProperties() {
       sent_visual_properties_->capture_sequence_number !=
           pending_visual_properties_.capture_sequence_number;
 
+  pending_visual_properties_.compositor_viewport =
+      web_frame_->GetCompositingRect();
+
   bool synchronized_props_changed =
       !sent_visual_properties_ ||
       sent_visual_properties_->auto_resize_enabled !=
@@ -637,6 +640,8 @@ void RenderFrameProxy::SynchronizeVisualProperties() {
           pending_visual_properties_.page_scale_factor ||
       sent_visual_properties_->is_pinch_gesture_active !=
           pending_visual_properties_.is_pinch_gesture_active ||
+      sent_visual_properties_->compositor_viewport !=
+          pending_visual_properties_.compositor_viewport ||
       capture_sequence_number_changed;
 
   if (synchronized_props_changed) {
@@ -652,7 +657,9 @@ void RenderFrameProxy::SynchronizeVisualProperties() {
                                     ? cc::DeadlinePolicy::UseInfiniteDeadline()
                                     : cc::DeadlinePolicy::UseDefaultDeadline();
   viz::SurfaceId surface_id(frame_sink_id_, GetLocalSurfaceId());
-  compositing_helper_->SetSurfaceId(surface_id, local_frame_size(), deadline);
+  compositing_helper_->SetSurfaceId(
+      surface_id, pending_visual_properties_.compositor_viewport.size(),
+      deadline);
 
   bool rect_changed = !sent_visual_properties_ ||
                       sent_visual_properties_->screen_space_rect !=
@@ -677,14 +684,6 @@ void RenderFrameProxy::SynchronizeVisualProperties() {
       "FrameHostMsg_SynchronizeVisualProperties", "local_surface_id",
       pending_visual_properties_.local_surface_id_allocation.local_surface_id()
           .ToString());
-
-  // The visible rect that the OOPIF needs to raster depends partially on
-  // parameters that might have changed. If they affect the raster area, resend
-  // the intersection rects.
-  gfx::Rect new_compositor_visible_rect = web_frame_->GetCompositingRect();
-  if (new_compositor_visible_rect != last_compositor_visible_rect_)
-    UpdateRemoteViewportIntersection(last_intersection_rect_,
-                                     last_occlusion_state_);
 }
 
 void RenderFrameProxy::OnSetHasReceivedUserGestureBeforeNavigation(bool value) {
@@ -822,12 +821,22 @@ void RenderFrameProxy::FrameRectsChanged(
 void RenderFrameProxy::UpdateRemoteViewportIntersection(
     const blink::WebRect& viewport_intersection,
     blink::FrameOcclusionState occlusion_state) {
-  last_intersection_rect_ = viewport_intersection;
-  last_compositor_visible_rect_ = web_frame_->GetCompositingRect();
-  last_occlusion_state_ = occlusion_state;
+  // If the remote viewport intersection has changed, then we should check if
+  // the compositing rect has also changed: if it has, then we should update the
+  // visible properties.
+  // TODO(wjmaclean): Maybe we should always call SynchronizeVisualProperties()
+  // here? If nothing has changed, it will early out, and it avoids duplicate
+  // checks here.
+  gfx::Rect compositor_visible_rect = web_frame_->GetCompositingRect();
+  bool compositor_visible_rect_changed =
+      compositor_visible_rect != pending_visual_properties_.compositor_viewport;
+
+  if (compositor_visible_rect_changed)
+    SynchronizeVisualProperties();
+
   Send(new FrameHostMsg_UpdateViewportIntersection(
-      routing_id_, gfx::Rect(viewport_intersection),
-      last_compositor_visible_rect_, last_occlusion_state_));
+      routing_id_, gfx::Rect(viewport_intersection), compositor_visible_rect,
+      occlusion_state));
 }
 
 void RenderFrameProxy::VisibilityChanged(

content/renderer/render_frame_proxy.h

@@ -322,11 +322,6 @@ class CONTENT_EXPORT RenderFrameProxy : public IPC::Listener,
   std::unique_ptr<viz::ParentLocalSurfaceIdAllocator>
       parent_local_surface_id_allocator_;
 
-  gfx::Rect last_intersection_rect_;
-  gfx::Rect last_compositor_visible_rect_;
-  blink::FrameOcclusionState last_occlusion_state_ =
-      blink::FrameOcclusionState::kUnknown;
-
   // The layer used to embed the out-of-process content.
   scoped_refptr<cc::Layer> embedded_layer_;
 

content/renderer/render_view_browsertest.cc

@@ -471,7 +471,7 @@ class RenderViewImplScaleFactorTest : public RenderViewImplTest {
     VisualProperties visual_properties;
     visual_properties.screen_info.device_scale_factor = dsf;
     visual_properties.new_size = gfx::Size(100, 100);
-    visual_properties.compositor_viewport_pixel_size = gfx::Size(200, 200);
+    visual_properties.compositor_viewport_pixel_rect = gfx::Rect(200, 200);
     visual_properties.visible_viewport_size = visual_properties.new_size;
     visual_properties.auto_resize_enabled =
         view()->GetWidget()->auto_resize_mode();