Commit e3f9a900 authored by Jeremy Roman's avatar Jeremy Roman Committed by Commit Bot

Revert "Break apart RenderWidgetHost::GetVisualProperties()"

This reverts commit 84437412.

Reason for revert: Suspected cause of browser test failures on Linux CFI. Confirmed via local bisect on TouchSelectionForCrossProcessFramesTests/TouchSelectionControllerClientAuraSiteIsolationTest.BasicSelectionIsolatedIframe/0 which consistently times out after this CL.

Original change's description:
> 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}

TBR=ajwong@chromium.org,ccameron@chromium.org,erikchen@chromium.org

Change-Id: Ia0af4e2dd74bb16841464c2d2c23ed177edf0744
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 998273
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1778812Reviewed-by: default avatarJeremy Roman <jbroman@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#692083}
parent b8863c24
...@@ -374,8 +374,9 @@ bool RenderViewHostImpl::CreateRenderView( ...@@ -374,8 +374,9 @@ bool RenderViewHostImpl::CreateRenderView(
GetSiteInstance()->GetSiteURL().SchemeIs(kGuestScheme); GetSiteInstance()->GetSiteURL().SchemeIs(kGuestScheme);
params->inside_portal = delegate_->IsPortal(); params->inside_portal = delegate_->IsPortal();
params->visual_properties = GetWidget()->GetVisualProperties(); bool needs_ack = false;
GetWidget()->SetInitialVisualProperties(params->visual_properties); GetWidget()->GetVisualProperties(&params->visual_properties, &needs_ack);
GetWidget()->SetInitialVisualProperties(params->visual_properties, needs_ack);
// The RenderView is owned by this process. This call must be accompanied by a // 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 // DestroyView [see destructor] or else there will be a leak in the renderer
......
...@@ -594,13 +594,15 @@ class CONTENT_EXPORT RenderWidgetHostImpl ...@@ -594,13 +594,15 @@ class CONTENT_EXPORT RenderWidgetHostImpl
// Allows the main frame's page scale state to be tracked. // Allows the main frame's page scale state to be tracked.
void SetPageScaleState(float page_scale_factor, bool is_pinch_gesture_active); void SetPageScaleState(float page_scale_factor, bool is_pinch_gesture_active);
// Generates a filled in VisualProperties struct representing the current // Fills in the |visual_properties| struct.
// properties of this widget. // Returns |false| if the update is redundant, |true| otherwise.
VisualProperties GetVisualProperties(); bool GetVisualProperties(VisualProperties* visual_properties,
bool* needs_ack);
// Sets the |visual_properties| that were sent to the renderer bundled with // Sets the |visual_properties| that were sent to the renderer bundled with
// the request to create a new RenderWidget. // the request to create a new RenderWidget.
void SetInitialVisualProperties(const VisualProperties& visual_properties); void SetInitialVisualProperties(const VisualProperties& visual_properties,
bool needs_ack);
// Pushes updated visual properties to the renderer as well as whether the // Pushes updated visual properties to the renderer as well as whether the
// focused node should be scrolled into view. // focused node should be scrolled into view.
...@@ -895,24 +897,6 @@ class CONTENT_EXPORT RenderWidgetHostImpl ...@@ -895,24 +897,6 @@ class CONTENT_EXPORT RenderWidgetHostImpl
// Called when visual properties have changed in the renderer. // Called when visual properties have changed in the renderer.
void DidUpdateVisualProperties(const cc::RenderFrameMetadata& metadata); 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 // Give key press listeners a chance to handle this key press. This allow
// widgets that don't have focus to still handle key presses. // widgets that don't have focus to still handle key presses.
bool KeyPressListenersHandleEvent(const NativeWebKeyboardEvent& event); bool KeyPressListenersHandleEvent(const NativeWebKeyboardEvent& event);
......
...@@ -549,8 +549,10 @@ class RenderWidgetHostTest : public testing::Test { ...@@ -549,8 +549,10 @@ class RenderWidgetHostTest : public testing::Test {
} }
void SetInitialVisualProperties() { void SetInitialVisualProperties() {
VisualProperties visual_properties = host_->GetVisualProperties(); VisualProperties visual_properties;
host_->SetInitialVisualProperties(visual_properties); bool needs_ack = false;
host_->GetVisualProperties(&visual_properties, &needs_ack);
host_->SetInitialVisualProperties(visual_properties, needs_ack);
} }
virtual void ConfigureView(TestView* view) { virtual void ConfigureView(TestView* view) {
...@@ -1576,7 +1578,9 @@ TEST_F(RenderWidgetHostTest, VisualProperties) { ...@@ -1576,7 +1578,9 @@ TEST_F(RenderWidgetHostTest, VisualProperties) {
view_->SetBounds(bounds); view_->SetBounds(bounds);
view_->SetMockCompositorViewportPixelSize(compositor_viewport_pixel_size); view_->SetMockCompositorViewportPixelSize(compositor_viewport_pixel_size);
VisualProperties visual_properties = host_->GetVisualProperties(); VisualProperties visual_properties;
bool needs_ack = false;
host_->GetVisualProperties(&visual_properties, &needs_ack);
EXPECT_EQ(bounds.size(), visual_properties.new_size); EXPECT_EQ(bounds.size(), visual_properties.new_size);
EXPECT_EQ(compositor_viewport_pixel_size, EXPECT_EQ(compositor_viewport_pixel_size,
visual_properties.compositor_viewport_pixel_size); visual_properties.compositor_viewport_pixel_size);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment