Avoid IPC spam and reentrancy when sending initial frame properties

When a site-isolated iframe moves to a new process, we need to ensure
that the browser sends it an initial set of properties that are
generated by the parent frame, even if those properties have not
changed. Previously, we redundantly re-sent the properties every time
frame rects for the iframe changed. With this patch, we send the
initial properties only once, after the first time VisualProperties
is sent to the frame.

This is a follow-up to:

https://chromium-review.googlesource.com/c/chromium/src/+/2628746

Bug: 1135714
Change-Id: I72e2f30d12525ab3f8cfb97a178b5185a9f6ca13
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2640893Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845685}

content/browser/renderer_host/cross_process_frame_connector.cc

@@ -329,7 +329,6 @@ void CrossProcessFrameConnector::OnSynchronizeVisualProperties(
 void CrossProcessFrameConnector::UpdateViewportIntersection(
     const blink::mojom::ViewportIntersectionState& intersection_state,
     const base::Optional<blink::FrameVisualProperties>& visual_properties) {
-  base::AutoReset<bool>(&is_processing_viewport_intersection_, true);
   if (visual_properties.has_value())
     SynchronizeVisualProperties(visual_properties.value(), false);
   UpdateViewportIntersectionInternal(intersection_state);

content/browser/renderer_host/cross_process_frame_connector.h

@@ -334,14 +334,6 @@ class CONTENT_EXPORT CrossProcessFrameConnector {
     use_zoom_for_device_scale_factor_ = use_zoom_for_device_scale_factor;
   }
 
-  // TODO(szager): This is a hack piled on top of a hack; see
-  // RenderWidgetHostViewChildFrame::WillSendScreenRects. We need a better way
-  // to initialize renderer process state after a frame migrates to a different
-  // process due to navigation.
-  bool IsProcessingViewportIntersection() const {
-    return is_processing_viewport_intersection_;
-  }
-
  protected:
   friend class MockCrossProcessFrameConnector;
   friend class SitePerProcessBrowserTestBase;
@@ -422,9 +414,6 @@ class CONTENT_EXPORT CrossProcessFrameConnector {
   // shown after a crash. This is only used when recording renderer crashes.
   bool delegate_was_shown_after_crash_ = false;
 
-  // This is used to prevent re-entry into UpdateViewportIntersection.
-  bool is_processing_viewport_intersection_ = false;
-
   // The last pre-transform frame size received from the parent renderer.
   // |last_received_local_frame_size_| may be in DIP if use zoom for DSF is
   // off.

content/browser/renderer_host/render_widget_host_impl.cc

@@ -558,7 +558,6 @@ void RenderWidgetHostImpl::SendScreenRects() {
 
   last_view_screen_rect_ = view_->GetViewBounds();
   last_window_screen_rect_ = view_->GetBoundsInRootWindow();
-  view_->WillSendScreenRects();
   blink_widget_->UpdateScreenRects(
       last_view_screen_rect_, last_window_screen_rect_,
       base::BindOnce(&RenderWidgetHostImpl::OnUpdateScreenRectsAck,
@@ -2393,6 +2392,8 @@ void RenderWidgetHostImpl::OnUpdateScreenRectsAck() {
   if (!view_)
     return;
 
+  view_->SendInitialPropertiesIfNeeded();
+
   if (view_->GetViewBounds() == last_view_screen_rect_ &&
       view_->GetBoundsInRootWindow() == last_window_screen_rect_) {
     return;

content/browser/renderer_host/render_widget_host_view_base.h

@@ -146,6 +146,8 @@ class CONTENT_EXPORT RenderWidgetHostViewBase : public RenderWidgetHostView {
 
   WidgetType GetWidgetType();
 
+  virtual void SendInitialPropertiesIfNeeded() {}
+
   // Notification that a resize or move session ended on the native widget.
   void UpdateScreenInfo(gfx::NativeView view);
 
@@ -324,10 +326,6 @@ class CONTENT_EXPORT RenderWidgetHostViewBase : public RenderWidgetHostView {
   // need to also be resolved.
   virtual bool IsRenderWidgetHostViewChildFrame();
 
-  // Notify the View that a screen rect update is being sent to the
-  // RenderWidget. Related platform-specific updates can be sent from here.
-  virtual void WillSendScreenRects() {}
-
   // Returns true if the current view is in virtual reality mode.
   virtual bool IsInVR() const;
 

content/browser/renderer_host/render_widget_host_view_child_frame.cc

@@ -349,6 +349,17 @@ void RenderWidgetHostViewChildFrame::UpdateCursor(const WebCursor& cursor) {
     frame_connector_->UpdateCursor(cursor);
 }
 
+void RenderWidgetHostViewChildFrame::SendInitialPropertiesIfNeeded() {
+  if (initial_properties_sent_ || !frame_connector_)
+    return;
+  UpdateViewportIntersection(frame_connector_->intersection_state(),
+                             base::nullopt);
+  SetIsInert();
+  UpdateInheritedEffectiveTouchAction();
+  UpdateRenderThrottlingStatus();
+  initial_properties_sent_ = true;
+}
+
 void RenderWidgetHostViewChildFrame::SetIsLoading(bool is_loading) {
   // It is valid for an inner WebContents's SetIsLoading() to end up here.
   // This is because an inner WebContents's main frame's RenderWidgetHostView
@@ -730,26 +741,6 @@ bool RenderWidgetHostViewChildFrame::IsRenderWidgetHostViewChildFrame() {
   return true;
 }
 
-void RenderWidgetHostViewChildFrame::WillSendScreenRects() {
-  // TODO(kenrb): These represent post-initialization state updates that are
-  // needed by the renderer. During normal OOPIF setup these are unnecessary,
-  // as the parent renderer will send the information and it will be
-  // immediately propagated to the OOPIF. However when an OOPIF navigates from
-  // one process to another, the parent doesn't know that, and certain
-  // browser-side state needs to be sent again. There is probably a less
-  // spammy way to do this, but triggering on SendScreenRects() is reasonable
-  // until somebody figures that out. RWHVCF::Init() is too early.
-  if (frame_connector_) {
-    if (!frame_connector_->IsProcessingViewportIntersection()) {
-      UpdateViewportIntersection(frame_connector_->intersection_state(),
-                                 base::nullopt);
-    }
-    SetIsInert();
-    UpdateInheritedEffectiveTouchAction();
-    UpdateRenderThrottlingStatus();
-  }
-}
-
 #if defined(OS_MAC)
 void RenderWidgetHostViewChildFrame::SetActive(bool active) {}
 

content/browser/renderer_host/render_widget_host_view_child_frame.h

@@ -96,6 +96,7 @@ class CONTENT_EXPORT RenderWidgetHostViewChildFrame
   void InitAsPopup(RenderWidgetHostView* parent_host_view,
                    const gfx::Rect& bounds) override;
   void UpdateCursor(const WebCursor& cursor) override;
+  void SendInitialPropertiesIfNeeded() override;
   void SetIsLoading(bool is_loading) override;
   void RenderProcessGone() override;
   void Destroy() override;
@@ -138,7 +139,6 @@ class CONTENT_EXPORT RenderWidgetHostViewChildFrame
   std::unique_ptr<SyntheticGestureTarget> CreateSyntheticGestureTarget()
       override;
   bool IsRenderWidgetHostViewChildFrame() override;
-  void WillSendScreenRects() override;
 
 #if defined(OS_MAC)
   // RenderWidgetHostView implementation.
@@ -286,6 +286,11 @@ class CONTENT_EXPORT RenderWidgetHostViewChildFrame
   // True if there is currently a scroll sequence being bubbled to our parent.
   bool is_scroll_sequence_bubbling_ = false;
 
+  // If a new RWHVCF is created for a cross-origin navigation, the parent
+  // will typically not notice and will not transmit a full complement of
+  // properties.
+  bool initial_properties_sent_ = false;
+
   // The ScreenInfo information from the parent at the time this class is
   // created, to be used before this view is connected to its FrameDelegate.
   // This is kept up to date anytime GetScreenInfo() is called and we have

content/browser/renderer_host/render_widget_host_view_child_frame_unittest.cc

@@ -244,7 +244,8 @@ TEST_F(RenderWidgetHostViewChildFrameTest, ViewportIntersectionUpdated) {
   FakeFrameWidget fake_frame_widget(std::move(blink_frame_widget_receiver));
 
   widget_host_->RendererWidgetCreated(/*for_frame_widget=*/true);
-
+  base::RunLoop().RunUntilIdle();
+  widget_.ClearScreenRects();
   base::RunLoop().RunUntilIdle();
 
   auto& intersection_state = fake_frame_widget.GetIntersectionState();