Break apart RenderWidgetHost::GetVisualProperties()

RenderWidgetHost::GetVisualProperties() was doing 3 separate things
  (1) Actually calculating the VisualProperties
  (2) Checking against the last stored VisualProperties to determine
      if the stored copy needs updating.
  (3) Determining if an ack should be expected if an update is eventually
      sent with the result of this function.

This CL breaks it out into 3 new functions:

GetVisualProperties() - Generates the VisualProperties. This should be
const but too much of the call chain underneath it did not annotate
their getters with const that annotating would be too annoying.

DidVisualPropertiesSizeChange() - Helper function that extracts a common
query used by DoesVisualPropertiesNeedAck() and
StoredVisualPropertiesNeedsUpdate().

DoesVisualPropertiesNeedAck() - Returns true if sending the given
VisualProperties update to the renderer will generate an ack.

StoredVisualPropertiesNeedsUpdate() - Returns true if the stored copy
of the VisualProperties is different enouhg from the incoming one that
it needs an update.

Bug: 998273
Change-Id: Ie8fee3ddedc1ea8b492765cb5b35e7bda99234de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1773878
Commit-Queue: Albert J. Wong <ajwong@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691879}

content/browser/renderer_host/render_view_host_impl.cc

@@ -374,9 +374,8 @@ bool RenderViewHostImpl::CreateRenderView(
       GetSiteInstance()->GetSiteURL().SchemeIs(kGuestScheme);
   params->inside_portal = delegate_->IsPortal();
 
-  bool needs_ack = false;
-  GetWidget()->GetVisualProperties(&params->visual_properties, &needs_ack);
-  GetWidget()->SetInitialVisualProperties(params->visual_properties, needs_ack);
+  params->visual_properties = GetWidget()->GetVisualProperties();
+  GetWidget()->SetInitialVisualProperties(params->visual_properties);
 
   // The RenderView is owned by this process. This call must be accompanied by a
   // DestroyView [see destructor] or else there will be a leak in the renderer

content/browser/renderer_host/render_widget_host_impl.h

@@ -594,15 +594,13 @@ class CONTENT_EXPORT RenderWidgetHostImpl
   // Allows the main frame's page scale state to be tracked.
   void SetPageScaleState(float page_scale_factor, bool is_pinch_gesture_active);
 
-  // Fills in the |visual_properties| struct.
-  // Returns |false| if the update is redundant, |true| otherwise.
-  bool GetVisualProperties(VisualProperties* visual_properties,
-                           bool* needs_ack);
+  // Generates a filled in VisualProperties struct representing the current
+  // properties of this widget.
+  VisualProperties GetVisualProperties();
 
   // 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,
-                                  bool needs_ack);
+  void SetInitialVisualProperties(const VisualProperties& visual_properties);
 
   // Pushes updated visual properties to the renderer as well as whether the
   // focused node should be scrolled into view.
@@ -897,6 +895,24 @@ class CONTENT_EXPORT RenderWidgetHostImpl
   // Called when visual properties have changed in the renderer.
   void DidUpdateVisualProperties(const cc::RenderFrameMetadata& metadata);
 
+  // Returns true if the |new_visual_properties| differs from
+  // |old_page_visual_properties| in a way that indicates a size changed.
+  static bool DidVisualPropertiesSizeChange(
+      const VisualProperties& old_visual_properties,
+      const VisualProperties& new_visual_properties);
+
+  // Returns true if the new visual properties requires an ack from a
+  // synchronization message.
+  static bool DoesVisualPropertiesNeedAck(
+      const std::unique_ptr<VisualProperties>& old_visual_properties,
+      const VisualProperties& new_visual_properties);
+
+  // Returns true if |old_visual_properties| is out of sync with
+  // |new_visual_properties|.
+  static bool StoredVisualPropertiesNeedsUpdate(
+      const std::unique_ptr<VisualProperties>& old_visual_properties,
+      const VisualProperties& new_visual_properties);
+
   // Give key press listeners a chance to handle this key press. This allow
   // widgets that don't have focus to still handle key presses.
   bool KeyPressListenersHandleEvent(const NativeWebKeyboardEvent& event);

content/browser/renderer_host/render_widget_host_unittest.cc

@@ -549,10 +549,8 @@ class RenderWidgetHostTest : public testing::Test {
   }
 
   void SetInitialVisualProperties() {
-    VisualProperties visual_properties;
-    bool needs_ack = false;
-    host_->GetVisualProperties(&visual_properties, &needs_ack);
-    host_->SetInitialVisualProperties(visual_properties, needs_ack);
+    VisualProperties visual_properties = host_->GetVisualProperties();
+    host_->SetInitialVisualProperties(visual_properties);
   }
 
   virtual void ConfigureView(TestView* view) {
@@ -1578,9 +1576,7 @@ TEST_F(RenderWidgetHostTest, VisualProperties) {
   view_->SetBounds(bounds);
   view_->SetMockCompositorViewportPixelSize(compositor_viewport_pixel_size);
 
-  VisualProperties visual_properties;
-  bool needs_ack = false;
-  host_->GetVisualProperties(&visual_properties, &needs_ack);
+  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);