diff --git a/content/browser/BUILD.gn b/content/browser/BUILD.gn index 9b57178a86e61..f9c2f54584574 100644 --- a/content/browser/BUILD.gn +++ b/content/browser/BUILD.gn @@ -1591,6 +1591,8 @@ jumbo_source_set("browser") { "renderer_host/text_input_manager.h", "renderer_host/ui_events_helper.cc", "renderer_host/ui_events_helper.h", + "renderer_host/visual_properties_manager.cc", + "renderer_host/visual_properties_manager.h", "renderer_host/web_database_host_impl.cc", "renderer_host/web_database_host_impl.h", "renderer_host/webmenurunner_mac.h", diff --git a/content/browser/frame_host/interstitial_page_impl.cc b/content/browser/frame_host/interstitial_page_impl.cc index 208f65431c4f7..115d2c486953c 100644 --- a/content/browser/frame_host/interstitial_page_impl.cc +++ b/content/browser/frame_host/interstitial_page_impl.cc @@ -795,14 +795,16 @@ Visibility InterstitialPageImpl::GetVisibility() { void InterstitialPageImpl::CreateNewWidget( int32_t render_process_id, int32_t route_id, - mojo::PendingRemote<mojom::Widget> widget) { + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host) { NOTREACHED() << "InterstitialPage does not support showing drop-downs."; } void InterstitialPageImpl::CreateNewFullscreenWidget( int32_t render_process_id, int32_t route_id, - mojo::PendingRemote<mojom::Widget> widget) { + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host) { NOTREACHED() << "InterstitialPage does not support showing full screen popups."; } diff --git a/content/browser/frame_host/interstitial_page_impl.h b/content/browser/frame_host/interstitial_page_impl.h index 9445711b3031d..239ae0f78e72b 100644 --- a/content/browser/frame_host/interstitial_page_impl.h +++ b/content/browser/frame_host/interstitial_page_impl.h @@ -161,11 +161,12 @@ class CONTENT_EXPORT InterstitialPageImpl : public InterstitialPage, BrowserContext* browser_context) const override; void CreateNewWidget(int32_t render_process_id, int32_t route_id, - mojo::PendingRemote<mojom::Widget> widget) override; - void CreateNewFullscreenWidget( - int32_t render_process_id, - int32_t route_id, - mojo::PendingRemote<mojom::Widget> widget) override; + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host) override; + void CreateNewFullscreenWidget(int32_t render_process_id, + int32_t route_id, + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host) override; void ShowCreatedWidget(int process_id, int route_id, const gfx::Rect& initial_rect) override; diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc index d9df69d57a2fb..a99c699bd0610 100644 --- a/content/browser/frame_host/render_frame_host_impl.cc +++ b/content/browser/frame_host/render_frame_host_impl.cc @@ -923,6 +923,8 @@ RenderFrameHostImpl::RenderFrameHostImpl( owned_render_widget_host_ = RenderWidgetHostFactory::Create( frame_tree_->render_widget_delegate(), GetProcess(), widget_routing_id, std::move(widget), /*hidden=*/true); + owned_render_widget_host_->BindVisualPropertiesManager( + render_view_host_->GetVisualPropertiesManager()); owned_render_widget_host_->set_owned_by_render_frame_host(true); } diff --git a/content/browser/renderer_host/render_view_host_delegate.h b/content/browser/renderer_host/render_view_host_delegate.h index 8aafa18a66684..aca63d149a8cd 100644 --- a/content/browser/renderer_host/render_view_host_delegate.h +++ b/content/browser/renderer_host/render_view_host_delegate.h @@ -133,13 +133,15 @@ class CONTENT_EXPORT RenderViewHostDelegate { // happen in response to ShowCreatedWidget. virtual void CreateNewWidget(int32_t render_process_id, int32_t widget_route_id, - mojo::PendingRemote<mojom::Widget> widget) {} + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host) {} // Creates a full screen RenderWidget. Similar to above. virtual void CreateNewFullscreenWidget( int32_t render_process_id, int32_t widget_route_id, - mojo::PendingRemote<mojom::Widget> widget) {} + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host) {} // Show the newly created widget with the specified bounds. // The widget is identified by the route_id passed to CreateNewWidget. diff --git a/content/browser/renderer_host/render_view_host_factory.cc b/content/browser/renderer_host/render_view_host_factory.cc index dde3ad4443851..11d568ac5d712 100644 --- a/content/browser/renderer_host/render_view_host_factory.cc +++ b/content/browser/renderer_host/render_view_host_factory.cc @@ -51,13 +51,17 @@ RenderViewHost* RenderViewHostFactory::Create( routing_id, main_frame_routing_id, widget_routing_id, swapped_out); } - return new RenderViewHostImpl( + + RenderViewHostImpl* view_host = new RenderViewHostImpl( instance, RenderWidgetHostFactory::Create(widget_delegate, instance->GetProcess(), widget_routing_id, mojo::NullRemote(), /*hidden=*/true), delegate, routing_id, main_frame_routing_id, swapped_out, true /* has_initialized_audio_host */); + view_host->GetWidget()->BindVisualPropertiesManager( + view_host->GetVisualPropertiesManager()); + return view_host; } // static diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc index ff4a64aeadb71..9ab5d0fad3e62 100644 --- a/content/browser/renderer_host/render_view_host_impl.cc +++ b/content/browser/renderer_host/render_view_host_impl.cc @@ -226,7 +226,8 @@ RenderViewHostImpl::RenderViewHostImpl( is_waiting_for_close_ack_(false), sudden_termination_allowed_(false), updating_web_preferences_(false), - has_notified_about_creation_(false) { + has_notified_about_creation_(false), + visual_properties_manager_(this) { DCHECK(instance_.get()); CHECK(delegate_); // http://crbug.com/82827 DCHECK_NE(GetRoutingID(), render_widget_host_->GetRoutingID()); @@ -432,12 +433,6 @@ FrameTreeNode* RenderViewHostImpl::GetFocusedFrame() { return GetDelegate()->GetFrameTree()->GetFocusedFrame(); } -void RenderViewHostImpl::UpdatePageVisualProperties( - const VisualProperties& visual_properties) { - Send(new ViewMsg_UpdateLocalMainFrameVisualProperties(GetRoutingID(), - visual_properties)); -} - void RenderViewHostImpl::ShowContextMenu(RenderFrameHost* render_frame_host, const ContextMenuParams& params) { GetDelegate()->GetDelegateView()->ShowContextMenu(render_frame_host, params); @@ -859,14 +854,14 @@ void RenderViewHostImpl::CreateNewWidget( int32_t widget_route_id, mojo::PendingRemote<mojom::Widget> widget) { delegate_->CreateNewWidget(GetProcess()->GetID(), widget_route_id, - std::move(widget)); + std::move(widget), this); } void RenderViewHostImpl::CreateNewFullscreenWidget( int32_t widget_route_id, mojo::PendingRemote<mojom::Widget> widget) { delegate_->CreateNewFullscreenWidget(GetProcess()->GetID(), widget_route_id, - std::move(widget)); + std::move(widget), this); } void RenderViewHostImpl::OnShowWidget(int widget_route_id, diff --git a/content/browser/renderer_host/render_view_host_impl.h b/content/browser/renderer_host/render_view_host_impl.h index 29c552386d00e..622c7a65f2cea 100644 --- a/content/browser/renderer_host/render_view_host_impl.h +++ b/content/browser/renderer_host/render_view_host_impl.h @@ -24,6 +24,7 @@ #include "content/browser/renderer_host/input/input_device_change_observer.h" #include "content/browser/renderer_host/render_widget_host_impl.h" #include "content/browser/renderer_host/render_widget_host_owner_delegate.h" +#include "content/browser/renderer_host/visual_properties_manager.h" #include "content/browser/site_instance_impl.h" #include "content/common/render_message_filter.mojom.h" #include "content/public/browser/notification_observer.h" @@ -132,6 +133,10 @@ class CONTENT_EXPORT RenderViewHostImpl const FrameReplicationState& replicated_frame_state, bool window_was_created_with_opener); + base::WeakPtr<VisualPropertiesManager> GetVisualPropertiesManager() { + return visual_properties_manager_.GetWeakPtr(); + } + // Tracks whether this RenderViewHost is in an active state (rather than // pending swap out or swapped out), according to its main frame // RenderFrameHost. @@ -235,8 +240,6 @@ class CONTENT_EXPORT RenderViewHostImpl bool IsNeverVisible() override; WebPreferences GetWebkitPreferencesForWidget() override; FrameTreeNode* GetFocusedFrame() override; - void UpdatePageVisualProperties( - const VisualProperties& visual_properties) override; void ShowContextMenu(RenderFrameHost* render_frame_host, const ContextMenuParams& params) override; @@ -354,6 +357,9 @@ class CONTENT_EXPORT RenderViewHostImpl // duplicate RenderViewCreated events. bool has_notified_about_creation_; + // Used to send Page and Widget visual properties to Renderers. + VisualPropertiesManager visual_properties_manager_; + base::WeakPtrFactory<RenderViewHostImpl> weak_factory_{this}; DISALLOW_COPY_AND_ASSIGN(RenderViewHostImpl); diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc index ee620963f25dc..a8f29674dd39a 100644 --- a/content/browser/renderer_host/render_widget_host_impl.cc +++ b/content/browser/renderer_host/render_widget_host_impl.cc @@ -66,6 +66,7 @@ #include "content/browser/renderer_host/render_widget_host_owner_delegate.h" #include "content/browser/renderer_host/render_widget_host_view_base.h" #include "content/browser/renderer_host/render_widget_host_view_child_frame.h" +#include "content/browser/renderer_host/visual_properties_manager.h" #include "content/common/content_constants_internal.h" #include "content/common/cursors/webcursor.h" #include "content/common/drag_messages.h" @@ -430,6 +431,13 @@ RenderWidgetHostImpl::~RenderWidgetHostImpl() { Destroy(false); } +void RenderWidgetHostImpl::BindVisualPropertiesManager( + base::WeakPtr<VisualPropertiesManager> visual_properties_manager) { + DCHECK(visual_properties_manager); + DCHECK(!visual_properties_manager_); + visual_properties_manager_ = std::move(visual_properties_manager); +} + // static RenderWidgetHost* RenderWidgetHost::FromID( int32_t process_id, @@ -927,21 +935,15 @@ bool RenderWidgetHostImpl::SynchronizeVisualProperties( visual_properties->visible_viewport_size; bool sent_visual_properties = false; - - // If the RenderWidget is associated with a RenderView, then we send the - // visual properties to the RenderView instead of the RenderWidget. The - // RenderView will pass along the relevant properties to the RenderWidget, - // which will send back an ACK. - if (owner_delegate_) { - owner_delegate_->UpdatePageVisualProperties(*visual_properties); - - // TODO(erikchen): Remove sent_visual_properties. It's unused and doesn't - // even make sense, since we're potentially sending multiple IPC messages. + if (visual_properties_manager_) { + visual_properties_manager_->SendVisualProperties(*visual_properties, + GetRoutingID()); sent_visual_properties = true; - } else { - sent_visual_properties = Send(new WidgetMsg_SynchronizeVisualProperties( - routing_id_, *visual_properties)); } + + // Ideally, page visual properties and widget visual properties would be sent + // synchronously. As they become decoupled, members should be moved from the + // VisualPropertiesManager::SendVisualProperties into this one. if (delegate() && visible_viewport_size_changed) { delegate()->NotifyVisibleViewportSizeChanged( visual_properties->visible_viewport_size); diff --git a/content/browser/renderer_host/render_widget_host_impl.h b/content/browser/renderer_host/render_widget_host_impl.h index 9257f1af8b6bc..9fdfbce61531b 100644 --- a/content/browser/renderer_host/render_widget_host_impl.h +++ b/content/browser/renderer_host/render_widget_host_impl.h @@ -114,6 +114,7 @@ class RenderWidgetHostOwnerDelegate; class SyntheticGestureController; class TimeoutMonitor; class TouchEmulator; +class VisualPropertiesManager; class WebCursor; struct EditCommand; struct VisualProperties; @@ -173,6 +174,20 @@ class CONTENT_EXPORT RenderWidgetHostImpl ~RenderWidgetHostImpl() override; + // The visual properties manager is used to send visual property updates to + // Renderers. Visual properties include both page and widget-specific + // properties. Both must be sent in lock-step, and the widget may not exist, + // whereas the page always will. Therefore, this class cannot directly send + // visual property updates, it must use the manager, a member of + // RenderViewHostImpl. Eventually, page state should be stored on the visual + // properties manager as well. + // This method must be called immediately after construction of the + // RenderWidgetHostImpl. The only reason it isn't a member of the constructor + // is due to ordering constraints of construction of RenderViewHostImpl [which + // requires an instance of RenderWidgetHostImpl]. + void BindVisualPropertiesManager( + base::WeakPtr<VisualPropertiesManager> visual_properties_manager); + // Similar to RenderWidgetHost::FromID, but returning the Impl object. static RenderWidgetHostImpl* FromID(int32_t process_id, int32_t routing_id); @@ -1287,6 +1302,12 @@ class CONTENT_EXPORT RenderWidgetHostImpl // See comments on Add/ClearPendingUserActivation(). base::OneShotTimer pending_user_activation_timer_; + // VisualPropertiesManager is owned by the RenderViewHost. For popup or pepper + // fullscreen widgets, the widget might outlive the RenderViewHost. + // TODO(https://crbug.com/1004009): Assert that this never happens and change + // this to be a raw pointer. + base::WeakPtr<VisualPropertiesManager> visual_properties_manager_; + base::WeakPtrFactory<RenderWidgetHostImpl> weak_factory_{this}; DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostImpl); diff --git a/content/browser/renderer_host/render_widget_host_owner_delegate.h b/content/browser/renderer_host/render_widget_host_owner_delegate.h index c714a4c365e22..a2eca4b2b4428 100644 --- a/content/browser/renderer_host/render_widget_host_owner_delegate.h +++ b/content/browser/renderer_host/render_widget_host_owner_delegate.h @@ -87,11 +87,6 @@ class CONTENT_EXPORT RenderWidgetHostOwnerDelegate { virtual void ShowContextMenu(RenderFrameHost* render_frame_host, const ContextMenuParams& params) = 0; - // Update VisualProperties for the page. For now, this only updates the main - // frame renderer. - virtual void UpdatePageVisualProperties( - const VisualProperties& visual_properties) = 0; - protected: virtual ~RenderWidgetHostOwnerDelegate() {} }; diff --git a/content/browser/renderer_host/render_widget_host_unittest.cc b/content/browser/renderer_host/render_widget_host_unittest.cc index b855d87361769..2bc014ff02f14 100644 --- a/content/browser/renderer_host/render_widget_host_unittest.cc +++ b/content/browser/renderer_host/render_widget_host_unittest.cc @@ -35,6 +35,7 @@ #include "content/browser/renderer_host/render_view_host_delegate_view.h" #include "content/browser/renderer_host/render_widget_host_delegate.h" #include "content/browser/renderer_host/render_widget_host_view_base.h" +#include "content/browser/renderer_host/visual_properties_manager.h" #include "content/common/edit_command.h" #include "content/common/input/synthetic_web_input_event_builders.h" #include "content/common/input_messages.h" @@ -443,6 +444,16 @@ class MockRenderWidgetHostOwnerDelegate void(const VisualProperties& visual_properties)); }; +class MockRenderWidgetHostVisualPropertiesManager + : public VisualPropertiesManager { + public: + MockRenderWidgetHostVisualPropertiesManager() + : VisualPropertiesManager(nullptr) {} + MOCK_METHOD2(SendVisualProperties, + void(const VisualProperties& visual_properties, + int widget_routing_id)); +}; + // RenderWidgetHostTest -------------------------------------------------------- class RenderWidgetHostTest : public testing::Test { @@ -493,6 +504,8 @@ class RenderWidgetHostTest : public testing::Test { process_->GetNextRoutingID())); // Set up the RenderWidgetHost as being for a main frame. host_->set_owner_delegate(&mock_owner_delegate_); + host_->BindVisualPropertiesManager( + mock_visual_properties_manager_.GetWeakPtr()); view_.reset(new TestView(host_.get())); ConfigureView(view_.get()); host_->SetView(view_.get()); @@ -702,6 +715,8 @@ class RenderWidgetHostTest : public testing::Test { RenderWidgetHostProcess* process_; // Deleted automatically by the widget. std::unique_ptr<MockRenderWidgetHostDelegate> delegate_; testing::NiceMock<MockRenderWidgetHostOwnerDelegate> mock_owner_delegate_; + testing::NiceMock<MockRenderWidgetHostVisualPropertiesManager> + mock_visual_properties_manager_; std::unique_ptr<MockRenderWidgetHost> host_; std::unique_ptr<TestView> view_; std::unique_ptr<display::Screen> screen_; @@ -737,28 +752,31 @@ class RenderWidgetHostWithSourceTest TEST_F(RenderWidgetHostTest, SynchronizeVisualProperties) { // The initial zoom is 0 so host should not send a sync message - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(0); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(0); delegate_->SetZoomLevel(0); EXPECT_FALSE(host_->SynchronizeVisualProperties()); EXPECT_FALSE(host_->visual_properties_ack_pending_); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); // The zoom has changed so host should send out a sync message - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(1); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(1); double new_zoom_level = content::ZoomFactorToZoomLevel(0.25); delegate_->SetZoomLevel(new_zoom_level); EXPECT_TRUE(host_->SynchronizeVisualProperties()); EXPECT_FALSE(host_->visual_properties_ack_pending_); EXPECT_NEAR(new_zoom_level, host_->old_visual_properties_->zoom_level, 0.01); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); // The initial bounds is the empty rect, so setting it to the same thing // shouldn't send the resize message. - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(0); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(0); view_->SetBounds(gfx::Rect()); host_->SynchronizeVisualProperties(); EXPECT_FALSE(host_->visual_properties_ack_pending_); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); // No visual properties ACK if the physical backing gets set, but the view // bounds are zero. @@ -768,18 +786,20 @@ TEST_F(RenderWidgetHostTest, SynchronizeVisualProperties) { // Setting the view bounds to nonzero should send out the notification. // but should not expect ack for empty physical backing size. - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(1); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(1); gfx::Rect original_size(0, 0, 100, 100); view_->SetBounds(original_size); view_->SetMockCompositorViewportPixelSize(gfx::Size()); host_->SynchronizeVisualProperties(); EXPECT_FALSE(host_->visual_properties_ack_pending_); EXPECT_EQ(original_size.size(), host_->old_visual_properties_->new_size); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); // Setting the bounds and physical backing size to nonzero should send out // the notification and expect an ack. - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(1); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(1); view_->ClearMockCompositorViewportPixelSize(); host_->SynchronizeVisualProperties(); EXPECT_TRUE(host_->visual_properties_ack_pending_); @@ -789,7 +809,7 @@ TEST_F(RenderWidgetHostTest, SynchronizeVisualProperties) { metadata.local_surface_id_allocation = base::nullopt; host_->DidUpdateVisualProperties(metadata); EXPECT_FALSE(host_->visual_properties_ack_pending_); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); gfx::Rect second_size(0, 0, 110, 110); EXPECT_FALSE(host_->visual_properties_ack_pending_); @@ -799,77 +819,85 @@ TEST_F(RenderWidgetHostTest, SynchronizeVisualProperties) { // Sending out a new notification should NOT send out a new IPC message since // a visual properties ACK is pending. - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(0); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(0); gfx::Rect third_size(0, 0, 120, 120); process_->sink().ClearMessages(); view_->SetBounds(third_size); EXPECT_FALSE(host_->SynchronizeVisualProperties()); EXPECT_TRUE(host_->visual_properties_ack_pending_); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); // Send a update that's a visual properties ACK, but for the original_size we // sent. Since this isn't the second_size, the message handler should // immediately send a new resize message for the new size to the renderer. - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(1); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(1); metadata.viewport_size_in_pixels = original_size.size(); metadata.local_surface_id_allocation = base::nullopt; host_->DidUpdateVisualProperties(metadata); EXPECT_TRUE(host_->visual_properties_ack_pending_); EXPECT_EQ(third_size.size(), host_->old_visual_properties_->new_size); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); // Send the visual properties ACK for the latest size. - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(0); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(0); metadata.viewport_size_in_pixels = third_size.size(); metadata.local_surface_id_allocation = base::nullopt; host_->DidUpdateVisualProperties(metadata); EXPECT_FALSE(host_->visual_properties_ack_pending_); EXPECT_EQ(third_size.size(), host_->old_visual_properties_->new_size); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); // Now clearing the bounds should send out a notification but we shouldn't // expect a visual properties ACK (since the renderer won't ack empty sizes). // The message should contain the new size (0x0) and not the previous one that // we skipped. - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(1); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(1); view_->SetBounds(gfx::Rect()); host_->SynchronizeVisualProperties(); EXPECT_FALSE(host_->visual_properties_ack_pending_); EXPECT_EQ(gfx::Size(), host_->old_visual_properties_->new_size); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); // Send a rect that has no area but has either width or height set. - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(1); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(1); view_->SetBounds(gfx::Rect(0, 0, 0, 30)); host_->SynchronizeVisualProperties(); EXPECT_FALSE(host_->visual_properties_ack_pending_); EXPECT_EQ(gfx::Size(0, 30), host_->old_visual_properties_->new_size); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); // Set the same size again. It should not be sent again. - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(0); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(0); host_->SynchronizeVisualProperties(); EXPECT_FALSE(host_->visual_properties_ack_pending_); EXPECT_EQ(gfx::Size(0, 30), host_->old_visual_properties_->new_size); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); // A different size should be sent again, however. - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(1); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(1); view_->SetBounds(gfx::Rect(0, 0, 0, 31)); host_->SynchronizeVisualProperties(); EXPECT_FALSE(host_->visual_properties_ack_pending_); EXPECT_EQ(gfx::Size(0, 31), host_->old_visual_properties_->new_size); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); // An invalid LocalSurfaceId should result in no change to the // |visual_properties_ack_pending_| bit. - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(1); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(1); view_->SetBounds(gfx::Rect(25, 25)); view_->InvalidateLocalSurfaceId(); host_->SynchronizeVisualProperties(); EXPECT_FALSE(host_->visual_properties_ack_pending_); EXPECT_EQ(gfx::Size(25, 25), host_->old_visual_properties_->new_size); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); } // Test that a resize event is sent if SynchronizeVisualProperties() is called @@ -882,35 +910,39 @@ TEST_F(RenderWidgetHostTest, ResizeScreenInfo) { screen_info.orientation_angle = 0; screen_info.orientation_type = SCREEN_ORIENTATION_VALUES_PORTRAIT_PRIMARY; - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(1); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(1); view_->SetScreenInfo(screen_info); host_->SynchronizeVisualProperties(); EXPECT_FALSE(host_->visual_properties_ack_pending_); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); screen_info.orientation_angle = 180; screen_info.orientation_type = SCREEN_ORIENTATION_VALUES_LANDSCAPE_PRIMARY; - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(1); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(1); view_->SetScreenInfo(screen_info); host_->SynchronizeVisualProperties(); EXPECT_FALSE(host_->visual_properties_ack_pending_); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); screen_info.device_scale_factor = 2.f; - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(1); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(1); view_->SetScreenInfo(screen_info); host_->SynchronizeVisualProperties(); EXPECT_FALSE(host_->visual_properties_ack_pending_); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); // No screen change. - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(0); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(0); view_->SetScreenInfo(screen_info); host_->SynchronizeVisualProperties(); EXPECT_FALSE(host_->visual_properties_ack_pending_); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); } // Test for crbug.com/25097. If a renderer crashes between a resize and the @@ -918,13 +950,14 @@ TEST_F(RenderWidgetHostTest, ResizeScreenInfo) { // ACK logic. TEST_F(RenderWidgetHostTest, ResizeThenCrash) { // Setting the bounds to a "real" rect should send out the notification. - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(1); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(1); gfx::Rect original_size(0, 0, 100, 100); view_->SetBounds(original_size); host_->SynchronizeVisualProperties(); EXPECT_TRUE(host_->visual_properties_ack_pending_); EXPECT_EQ(original_size.size(), host_->old_visual_properties_->new_size); - ::testing::Mock::VerifyAndClearExpectations(&mock_owner_delegate_); + ::testing::Mock::VerifyAndClearExpectations(&mock_visual_properties_manager_); // Simulate a renderer crash before the update message. Ensure all the visual // properties ACK logic is cleared. Must clear the view first so it doesn't @@ -1622,7 +1655,8 @@ TEST_F(RenderWidgetHostInitialSizeTest, InitialSize) { TEST_F(RenderWidgetHostTest, HideUnthrottlesResize) { gfx::Size original_size(100, 100); view_->SetBounds(gfx::Rect(original_size)); - EXPECT_CALL(mock_owner_delegate_, UpdatePageVisualProperties(_)).Times(1); + EXPECT_CALL(mock_visual_properties_manager_, SendVisualProperties(_, _)) + .Times(1); EXPECT_TRUE(host_->SynchronizeVisualProperties()); EXPECT_EQ(original_size, host_->old_visual_properties_->new_size); EXPECT_TRUE(host_->visual_properties_ack_pending_); diff --git a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc index 1baecece488cc..bc18634ecee97 100644 --- a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc +++ b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc @@ -59,6 +59,7 @@ #include "content/browser/renderer_host/render_widget_host_impl.h" #include "content/browser/renderer_host/render_widget_host_view_event_handler.h" #include "content/browser/renderer_host/text_input_manager.h" +#include "content/browser/renderer_host/visual_properties_manager.h" #include "content/browser/web_contents/web_contents_view_aura.h" #include "content/common/input/synthetic_web_input_event_builders.h" #include "content/common/input_messages.h" @@ -155,6 +156,23 @@ using viz::FrameEvictionManager; namespace content { +namespace { +class MockRenderWidgetHostVisualPropertiesManager + : public VisualPropertiesManager { + public: + MockRenderWidgetHostVisualPropertiesManager() + : VisualPropertiesManager(nullptr) {} + void SendVisualProperties(const VisualProperties& visual_properties, + int widget_routing_id) override { + ++sent_visual_properties_count_; + sent_visual_properties_ = visual_properties; + } + + int sent_visual_properties_count_ = 0; + VisualProperties sent_visual_properties_; +}; +} // namespace + void InstallDelegatedFrameHostClient( RenderWidgetHostViewAura* render_widget_host_view, std::unique_ptr<DelegatedFrameHostClient> delegated_frame_host_client); @@ -557,6 +575,8 @@ class RenderWidgetHostViewAuraTest : public testing::Test { gfx::Rect()); view_ = CreateView(is_guest_view_hack_); widget_host_ = static_cast<MockRenderWidgetHostImpl*>(view_->host()); + widget_host_->BindVisualPropertiesManager( + visual_properties_manager_.GetWeakPtr()); // Set the mouse_wheel_phase_handler_ timer timeout to 100ms. view_->event_handler()->set_mouse_wheel_wheel_phase_handler_timeout( base::TimeDelta::FromMilliseconds(100)); @@ -687,6 +707,7 @@ class RenderWidgetHostViewAuraTest : public testing::Test { base::test::ScopedFeatureList feature_list_; viz::ParentLocalSurfaceIdAllocator parent_local_surface_id_allocator_; + MockRenderWidgetHostVisualPropertiesManager visual_properties_manager_; private: DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostViewAuraTest); @@ -2590,52 +2611,40 @@ TEST_F(RenderWidgetHostViewAuraTest, CompositorViewportPixelSizeWithScale) { view_->GetNativeView(), parent_view_->GetNativeView()->GetRootWindow(), gfx::Rect()); - sink_->ClearMessages(); + EXPECT_EQ(visual_properties_manager_.sent_visual_properties_count_, 1); view_->SetSize(gfx::Size(100, 100)); EXPECT_EQ("100x100", view_->GetCompositorViewportPixelSize().ToString()); - EXPECT_EQ(1u, sink_->message_count()); - EXPECT_EQ(static_cast<uint32_t>(WidgetMsg_SynchronizeVisualProperties::ID), - sink_->GetMessageAt(0)->type()); + EXPECT_EQ(visual_properties_manager_.sent_visual_properties_count_, 2); { - const IPC::Message* msg = sink_->GetMessageAt(0); - EXPECT_EQ(static_cast<uint32_t>(WidgetMsg_SynchronizeVisualProperties::ID), - msg->type()); - WidgetMsg_SynchronizeVisualProperties::Param params; - WidgetMsg_SynchronizeVisualProperties::Read(msg, ¶ms); - EXPECT_EQ("100x100", std::get<0>(params).new_size.ToString()); // dip size + VisualProperties visual_properties = + visual_properties_manager_.sent_visual_properties_; + EXPECT_EQ("100x100", visual_properties.new_size.ToString()); // dip size EXPECT_EQ("100x100", - std::get<0>(params) - .compositor_viewport_pixel_rect.size() + visual_properties.compositor_viewport_pixel_rect.size() .ToString()); // backing size } widget_host_->ResetSentVisualProperties(); - sink_->ClearMessages(); aura_test_helper_->test_screen()->SetDeviceScaleFactor(2.0f); EXPECT_EQ("200x200", view_->GetCompositorViewportPixelSize().ToString()); // Extra ScreenInfoChanged message for |parent_view_|. // Changing the device scale factor triggers the // RenderWidgetHostViewAura::OnDisplayMetricsChanged() observer callback, - // which sends a WidgetMsg_SynchronizeVisualProperties::ID message to the + // which sends a ViewMsg_UpdateVisualProperties::ID message to the // renderer. - EXPECT_EQ(1u, sink_->message_count()); - EXPECT_EQ(static_cast<uint32_t>(WidgetMsg_SynchronizeVisualProperties::ID), - sink_->GetMessageAt(0)->type()); + EXPECT_EQ(visual_properties_manager_.sent_visual_properties_count_, 3); widget_host_->ResetSentVisualProperties(); - sink_->ClearMessages(); aura_test_helper_->test_screen()->SetDeviceScaleFactor(1.0f); // Extra ScreenInfoChanged message for |parent_view_|. - EXPECT_EQ(1u, sink_->message_count()); - EXPECT_EQ(static_cast<uint32_t>(WidgetMsg_SynchronizeVisualProperties::ID), - sink_->GetMessageAt(0)->type()); + EXPECT_EQ(visual_properties_manager_.sent_visual_properties_count_, 4); EXPECT_EQ("100x100", view_->GetCompositorViewportPixelSize().ToString()); } // This test verifies that in AutoResize mode a new -// WidgetMsg_SynchronizeVisualProperties message is sent when ScreenInfo +// ViewMsg_UpdateVisualProperties message is sent when ScreenInfo // changes and that message contains the latest ScreenInfo. TEST_F(RenderWidgetHostViewAuraTest, AutoResizeWithScale) { view_->InitAsChild(nullptr); @@ -2646,18 +2655,14 @@ TEST_F(RenderWidgetHostViewAuraTest, AutoResizeWithScale) { view_->GetLocalSurfaceIdAllocation()); EXPECT_TRUE(local_surface_id_allocation1.IsValid()); - sink_->ClearMessages(); + EXPECT_EQ(visual_properties_manager_.sent_visual_properties_count_, 1); view_->EnableAutoResize(gfx::Size(50, 50), gfx::Size(100, 100)); viz::LocalSurfaceIdAllocation local_surface_id_allocation2; - ASSERT_EQ(1u, sink_->message_count()); + EXPECT_EQ(visual_properties_manager_.sent_visual_properties_count_, 2); { - const IPC::Message* msg = sink_->GetMessageAt(0); - EXPECT_EQ(static_cast<uint32_t>(WidgetMsg_SynchronizeVisualProperties::ID), - msg->type()); - WidgetMsg_SynchronizeVisualProperties::Param params; - WidgetMsg_SynchronizeVisualProperties::Read(msg, ¶ms); - VisualProperties visual_properties = std::get<0>(params); + VisualProperties visual_properties = + visual_properties_manager_.sent_visual_properties_; EXPECT_EQ("50x50", visual_properties.min_size_for_auto_resize.ToString()); EXPECT_EQ("100x100", visual_properties.max_size_for_auto_resize.ToString()); EXPECT_EQ(1, visual_properties.screen_info.device_scale_factor); @@ -2684,18 +2689,14 @@ TEST_F(RenderWidgetHostViewAuraTest, AutoResizeWithScale) { widget_host_->DidUpdateVisualProperties(metadata); } - sink_->ClearMessages(); aura_test_helper_->test_screen()->SetDeviceScaleFactor(2.0f); + EXPECT_EQ(visual_properties_manager_.sent_visual_properties_count_, 3); { // TODO(samans): There should be only one message in the sink, but some // testers are seeing two (crrev.com/c/839580). Investigate why. - const IPC::Message* msg = sink_->GetFirstMessageMatching( - WidgetMsg_SynchronizeVisualProperties::ID); - ASSERT_TRUE(msg); - WidgetMsg_SynchronizeVisualProperties::Param params; - WidgetMsg_SynchronizeVisualProperties::Read(msg, ¶ms); - VisualProperties visual_properties = std::get<0>(params); + VisualProperties visual_properties = + visual_properties_manager_.sent_visual_properties_; EXPECT_EQ("50x50", visual_properties.min_size_for_auto_resize.ToString()); EXPECT_EQ("100x100", visual_properties.max_size_for_auto_resize.ToString()); EXPECT_EQ(2, visual_properties.screen_info.device_scale_factor); @@ -2709,7 +2710,7 @@ TEST_F(RenderWidgetHostViewAuraTest, AutoResizeWithScale) { } // This test verifies that in AutoResize mode a new -// WidgetMsg_SynchronizeVisualProperties message is sent when size changes. +// ViewMsg_UpdateVisualProperties message is sent when size changes. TEST_F(RenderWidgetHostViewAuraTest, AutoResizeWithBrowserInitiatedResize) { view_->InitAsChild(nullptr); aura::client::ParentWindowWithContext( @@ -2719,18 +2720,14 @@ TEST_F(RenderWidgetHostViewAuraTest, AutoResizeWithBrowserInitiatedResize) { view_->GetLocalSurfaceIdAllocation()); EXPECT_TRUE(local_surface_id_allocation1.IsValid()); - sink_->ClearMessages(); + EXPECT_EQ(visual_properties_manager_.sent_visual_properties_count_, 1); view_->EnableAutoResize(gfx::Size(50, 50), gfx::Size(100, 100)); viz::LocalSurfaceIdAllocation local_surface_id_allocation2; - ASSERT_EQ(1u, sink_->message_count()); + EXPECT_EQ(visual_properties_manager_.sent_visual_properties_count_, 2); { - const IPC::Message* msg = sink_->GetMessageAt(0); - EXPECT_EQ(static_cast<uint32_t>(WidgetMsg_SynchronizeVisualProperties::ID), - msg->type()); - WidgetMsg_SynchronizeVisualProperties::Param params; - WidgetMsg_SynchronizeVisualProperties::Read(msg, ¶ms); - VisualProperties visual_properties = std::get<0>(params); + VisualProperties visual_properties = + visual_properties_manager_.sent_visual_properties_; EXPECT_EQ("50x50", visual_properties.min_size_for_auto_resize.ToString()); EXPECT_EQ("100x100", visual_properties.max_size_for_auto_resize.ToString()); EXPECT_EQ(1, visual_properties.screen_info.device_scale_factor); @@ -2757,18 +2754,12 @@ TEST_F(RenderWidgetHostViewAuraTest, AutoResizeWithBrowserInitiatedResize) { widget_host_->DidUpdateVisualProperties(metadata); } - sink_->ClearMessages(); - view_->SetSize(gfx::Size(120, 120)); viz::LocalSurfaceIdAllocation local_surface_id_allocation3; - ASSERT_EQ(1u, sink_->message_count()); + EXPECT_EQ(visual_properties_manager_.sent_visual_properties_count_, 3); { - const IPC::Message* msg = sink_->GetMessageAt(0); - EXPECT_EQ(static_cast<uint32_t>(WidgetMsg_SynchronizeVisualProperties::ID), - msg->type()); - WidgetMsg_SynchronizeVisualProperties::Param params; - WidgetMsg_SynchronizeVisualProperties::Read(msg, ¶ms); - VisualProperties visual_properties = std::get<0>(params); + VisualProperties visual_properties = + visual_properties_manager_.sent_visual_properties_; EXPECT_EQ("50x50", visual_properties.min_size_for_auto_resize.ToString()); EXPECT_EQ("100x100", visual_properties.max_size_for_auto_resize.ToString()); EXPECT_EQ(1, visual_properties.screen_info.device_scale_factor); @@ -2815,7 +2806,7 @@ TEST_F(RenderWidgetHostViewAuraTest, ChildAllocationAcceptedInParent) { // This test verifies that if the parent is hidden when the child sends a // child-allocated viz::LocalSurfaceId, the parent will store it and it will -// not send a WidgetMsg_SynchronizeVisualProperties back to the child. +// not send a ViewMsg_UpdateVisualProperties back to the child. TEST_F(RenderWidgetHostViewAuraTest, ChildAllocationAcceptedInParentWhileHidden) { view_->InitAsChild(nullptr); @@ -2849,8 +2840,8 @@ TEST_F(RenderWidgetHostViewAuraTest, EXPECT_NE(local_surface_id_allocation1, local_surface_id_allocation3); EXPECT_EQ(local_surface_id_allocation2, local_surface_id_allocation3); - EXPECT_FALSE(sink_->GetUniqueMessageMatching( - WidgetMsg_SynchronizeVisualProperties::ID)); + EXPECT_FALSE( + sink_->GetUniqueMessageMatching(ViewMsg_UpdateVisualProperties::ID)); } // This test verifies that when the child and parent both allocate their own @@ -3133,10 +3124,10 @@ TEST_F(RenderWidgetHostViewAuraTest, DISABLED_FullscreenResize) { { // 0 is CreatingNew message. const IPC::Message* msg = sink_->GetMessageAt(0); - EXPECT_EQ(static_cast<uint32_t>(WidgetMsg_SynchronizeVisualProperties::ID), + EXPECT_EQ(static_cast<uint32_t>(ViewMsg_UpdateVisualProperties::ID), msg->type()); - WidgetMsg_SynchronizeVisualProperties::Param params; - WidgetMsg_SynchronizeVisualProperties::Read(msg, ¶ms); + ViewMsg_UpdateVisualProperties::Param params; + ViewMsg_UpdateVisualProperties::Read(msg, ¶ms); EXPECT_EQ( "0,0 800x600", std::get<0>(params).screen_info.available_rect.ToString()); @@ -3161,10 +3152,10 @@ TEST_F(RenderWidgetHostViewAuraTest, DISABLED_FullscreenResize) { EXPECT_EQ(1u, sink_->message_count()); { const IPC::Message* msg = sink_->GetMessageAt(0); - EXPECT_EQ(static_cast<uint32_t>(WidgetMsg_SynchronizeVisualProperties::ID), + EXPECT_EQ(static_cast<uint32_t>(ViewMsg_UpdateVisualProperties::ID), msg->type()); - WidgetMsg_SynchronizeVisualProperties::Param params; - WidgetMsg_SynchronizeVisualProperties::Read(msg, ¶ms); + ViewMsg_UpdateVisualProperties::Param params; + ViewMsg_UpdateVisualProperties::Read(msg, ¶ms); EXPECT_EQ( "0,0 1600x1200", std::get<0>(params).screen_info.available_rect.ToString()); @@ -3195,16 +3186,13 @@ TEST_F(RenderWidgetHostViewAuraTest, ZeroSizeStillGetsLocalSurfaceId) { ui::Layer* parent_layer = view_->GetNativeView()->layer(); EXPECT_EQ(gfx::Rect(), parent_layer->bounds()); - EXPECT_EQ(2u, sink_->message_count()); + EXPECT_EQ(visual_properties_manager_.sent_visual_properties_count_, 1); { - const IPC::Message* msg = sink_->GetMessageAt(1); - EXPECT_EQ(static_cast<uint32_t>(WidgetMsg_SynchronizeVisualProperties::ID), - msg->type()); - WidgetMsg_SynchronizeVisualProperties::Param params; - WidgetMsg_SynchronizeVisualProperties::Read(msg, ¶ms); - EXPECT_EQ(frame_size.ToString(), std::get<0>(params).new_size.ToString()); - ASSERT_TRUE(std::get<0>(params).local_surface_id_allocation.has_value()); - EXPECT_TRUE(std::get<0>(params).local_surface_id_allocation->IsValid()); + VisualProperties visual_properties = + visual_properties_manager_.sent_visual_properties_; + EXPECT_EQ(frame_size.ToString(), visual_properties.new_size.ToString()); + ASSERT_TRUE(visual_properties.local_surface_id_allocation.has_value()); + EXPECT_TRUE(visual_properties.local_surface_id_allocation->IsValid()); } } @@ -3299,10 +3287,10 @@ TEST_F(RenderWidgetHostViewAuraTest, DISABLED_Resize) { EXPECT_EQ(1u, sink_->message_count()); { const IPC::Message* msg = sink_->GetMessageAt(0); - EXPECT_EQ(static_cast<uint32_t>(WidgetMsg_SynchronizeVisualProperties::ID), + EXPECT_EQ(static_cast<uint32_t>(ViewMsg_UpdateVisualProperties::ID), msg->type()); - WidgetMsg_SynchronizeVisualProperties::Param params; - WidgetMsg_SynchronizeVisualProperties::Read(msg, ¶ms); + ViewMsg_UpdateVisualProperties::Param params; + ViewMsg_UpdateVisualProperties::Read(msg, ¶ms); EXPECT_EQ(size2.ToString(), std::get<0>(params).new_size.ToString()); } // Send resize ack to observe new Resize messages. @@ -3355,10 +3343,10 @@ TEST_F(RenderWidgetHostViewAuraTest, DISABLED_Resize) { for (uint32_t i = 0; i < sink_->message_count(); ++i) { const IPC::Message* msg = sink_->GetMessageAt(i); switch (msg->type()) { - case WidgetMsg_SynchronizeVisualProperties::ID: { + case ViewMsg_UpdateVisualProperties::ID: { EXPECT_FALSE(has_resize); - WidgetMsg_SynchronizeVisualProperties::Param params; - WidgetMsg_SynchronizeVisualProperties::Read(msg, ¶ms); + ViewMsg_UpdateVisualProperties::Param params; + ViewMsg_UpdateVisualProperties::Read(msg, ¶ms); EXPECT_EQ(size3.ToString(), std::get<0>(params).new_size.ToString()); has_resize = true; break; @@ -3718,18 +3706,15 @@ TEST_F(RenderWidgetHostViewAuraTest, VisibleViewportTest) { EXPECT_EQ(100, view_->GetVisibleViewportSize().height()); widget_host_->ResetSentVisualProperties(); - sink_->ClearMessages(); + EXPECT_EQ(visual_properties_manager_.sent_visual_properties_count_, 2); view_->SetInsets(gfx::Insets(0, 0, 40, 0)); EXPECT_EQ(60, view_->GetVisibleViewportSize().height()); - const IPC::Message* message = - sink_->GetFirstMessageMatching(WidgetMsg_SynchronizeVisualProperties::ID); - ASSERT_TRUE(message != nullptr); - - WidgetMsg_SynchronizeVisualProperties::Param params; - WidgetMsg_SynchronizeVisualProperties::Read(message, ¶ms); - EXPECT_EQ(60, std::get<0>(params).visible_viewport_size.height()); + EXPECT_EQ(visual_properties_manager_.sent_visual_properties_count_, 3); + VisualProperties visual_properties = + visual_properties_manager_.sent_visual_properties_; + EXPECT_EQ(60, visual_properties.visible_viewport_size.height()); } // Ensures that touch event positions are never truncated to integers. diff --git a/content/browser/renderer_host/render_widget_host_view_child_frame_unittest.cc b/content/browser/renderer_host/render_widget_host_view_child_frame_unittest.cc index 1170977406c18..715e00f02c4c2 100644 --- a/content/browser/renderer_host/render_widget_host_view_child_frame_unittest.cc +++ b/content/browser/renderer_host/render_widget_host_view_child_frame_unittest.cc @@ -28,7 +28,9 @@ #include "content/browser/renderer_host/frame_connector_delegate.h" #include "content/browser/renderer_host/render_widget_host_delegate.h" #include "content/browser/renderer_host/render_widget_host_impl.h" +#include "content/browser/renderer_host/visual_properties_manager.h" #include "content/common/frame_visual_properties.h" +#include "content/common/view_messages.h" #include "content/common/widget_messages.h" #include "content/public/browser/render_widget_host_view.h" #include "content/public/test/browser_task_environment.h" @@ -47,6 +49,21 @@ namespace content { namespace { +class MockRenderWidgetHostVisualPropertiesManager + : public VisualPropertiesManager { + public: + MockRenderWidgetHostVisualPropertiesManager() + : VisualPropertiesManager(nullptr) {} + void SendVisualProperties(const VisualProperties& visual_properties, + int widget_routing_id) override { + ++sent_visual_properties_count_; + sent_visual_properties_ = visual_properties; + } + + int sent_visual_properties_count_ = 0; + VisualProperties sent_visual_properties_; +}; + const viz::LocalSurfaceId kArbitraryLocalSurfaceId( 1, base::UnguessableToken::Deserialize(2, 3)); @@ -119,6 +136,8 @@ class RenderWidgetHostViewChildFrameTest : public testing::Test { widget.InitWithNewPipeAndPassReceiver()); widget_host_ = new RenderWidgetHostImpl( &delegate_, process_host, routing_id, std::move(widget), false); + widget_host_->BindVisualPropertiesManager( + visual_properties_manager_.GetWeakPtr()); view_ = RenderWidgetHostViewChildFrame::Create(widget_host_); test_frame_connector_ = @@ -176,6 +195,7 @@ class RenderWidgetHostViewChildFrameTest : public testing::Test { MockFrameConnectorDelegate* test_frame_connector_; std::unique_ptr<FakeRendererCompositorFrameSink> renderer_compositor_frame_sink_; + MockRenderWidgetHostVisualPropertiesManager visual_properties_manager_; private: viz::mojom::CompositorFrameSinkClientPtr renderer_compositor_frame_sink_ptr_; @@ -285,8 +305,7 @@ TEST_F(RenderWidgetHostViewChildFrameTest, allocator.GetCurrentLocalSurfaceIdAllocation(); constexpr viz::FrameSinkId frame_sink_id(1, 1); - process->sink().ClearMessages(); - + EXPECT_EQ(visual_properties_manager_.sent_visual_properties_count_, 0); FrameVisualProperties visual_properties; visual_properties.screen_space_rect = screen_space_rect; visual_properties.compositor_viewport = compositor_viewport_pixel_rect; @@ -296,19 +315,16 @@ TEST_F(RenderWidgetHostViewChildFrameTest, test_frame_connector_->SynchronizeVisualProperties(frame_sink_id, visual_properties); - ASSERT_EQ(1u, process->sink().message_count()); + EXPECT_EQ(visual_properties_manager_.sent_visual_properties_count_, 1); - const IPC::Message* resize_msg = process->sink().GetUniqueMessageMatching( - WidgetMsg_SynchronizeVisualProperties::ID); - ASSERT_NE(nullptr, resize_msg); - WidgetMsg_SynchronizeVisualProperties::Param params; - WidgetMsg_SynchronizeVisualProperties::Read(resize_msg, ¶ms); + VisualProperties sent_visual_properties = + visual_properties_manager_.sent_visual_properties_; 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); + sent_visual_properties.compositor_viewport_pixel_rect); + EXPECT_EQ(screen_space_rect.size(), sent_visual_properties.new_size); EXPECT_EQ(local_surface_id_allocation, - std::get<0>(params).local_surface_id_allocation); - EXPECT_EQ(123u, std::get<0>(params).capture_sequence_number); + sent_visual_properties.local_surface_id_allocation); + EXPECT_EQ(123u, sent_visual_properties.capture_sequence_number); } // Test that when we have a gesture scroll sequence that is not consumed by the diff --git a/content/browser/renderer_host/visual_properties_manager.cc b/content/browser/renderer_host/visual_properties_manager.cc new file mode 100644 index 0000000000000..2df6a12aee25a --- /dev/null +++ b/content/browser/renderer_host/visual_properties_manager.cc @@ -0,0 +1,24 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "content/browser/renderer_host/visual_properties_manager.h" +#include "content/browser/renderer_host/render_view_host_impl.h" +#include "content/common/view_messages.h" +#include "content/common/visual_properties.h" + +namespace content { + +VisualPropertiesManager::VisualPropertiesManager( + RenderViewHostImpl* render_view_host) + : render_view_host_(render_view_host) {} +VisualPropertiesManager::~VisualPropertiesManager() = default; + +void VisualPropertiesManager::SendVisualProperties( + const VisualProperties& visual_properties, + int widget_routing_id) { + render_view_host_->Send(new ViewMsg_UpdateVisualProperties( + render_view_host_->GetRoutingID(), visual_properties, widget_routing_id)); +} + +} // namespace content diff --git a/content/browser/renderer_host/visual_properties_manager.h b/content/browser/renderer_host/visual_properties_manager.h new file mode 100644 index 0000000000000..f302359fc8676 --- /dev/null +++ b/content/browser/renderer_host/visual_properties_manager.h @@ -0,0 +1,53 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CONTENT_BROWSER_RENDERER_HOST_VISUAL_PROPERTIES_MANAGER_H_ +#define CONTENT_BROWSER_RENDERER_HOST_VISUAL_PROPERTIES_MANAGER_H_ + +#include "base/macros.h" +#include "base/memory/weak_ptr.h" +#include "content/common/content_export.h" + +namespace content { + +class RenderViewHostImpl; +struct VisualProperties; + +// This class manages page visual properties in the browser. Visual properties +// can be page-specific or widget-specific. Right now, this state is stored in +// RenderWidgetHost. Eventually, we want page-specific state to be stored in +// this class instead. +class CONTENT_EXPORT VisualPropertiesManager { + public: + VisualPropertiesManager(RenderViewHostImpl* render_view_host); + ~VisualPropertiesManager(); + + // Sends both Page and Widget visual properties to the renderer hosting the + // RenderView. The widget_routing_id is used by the renderer to forward Widget + // visual properties to the Widget. + // virtual for testing. + virtual void SendVisualProperties(const VisualProperties& visual_properties, + int widget_routing_id); + + base::WeakPtr<VisualPropertiesManager> GetWeakPtr() { + return weak_factory_.GetWeakPtr(); + } + + private: + RenderViewHostImpl* render_view_host_ = nullptr; + + // TODO(https://crbug.com/1004009): We should not need a weak factory because + // RenderWidgetHostImpl should not outlive RenderViewHostImpl. + // Instances of this class are owned by instances of RenderViewHostImpl. Due + // to unclear ownership semantics, it's not clear if a RenderWidgetHostImpl + // can outlive its RenderViewHostImpl. We assume that it can and thus pass a + // weak ptr. + base::WeakPtrFactory<VisualPropertiesManager> weak_factory_{this}; + + DISALLOW_COPY_AND_ASSIGN(VisualPropertiesManager); +}; + +} // namespace content + +#endif // CONTENT_BROWSER_RENDERER_HOST_VISUAL_PROPERTIES_MANAGER_H_ diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 857316001d672..d9d45f1db54b3 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -2977,27 +2977,28 @@ void WebContentsImpl::CreateNewWindow( } } -void WebContentsImpl::CreateNewWidget( - int32_t render_process_id, - int32_t widget_route_id, - mojo::PendingRemote<mojom::Widget> widget) { +void WebContentsImpl::CreateNewWidget(int32_t render_process_id, + int32_t widget_route_id, + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host) { CreateNewWidget(render_process_id, widget_route_id, /*is_fullscreen=*/false, - std::move(widget)); + std::move(widget), render_view_host); } void WebContentsImpl::CreateNewFullscreenWidget( int32_t render_process_id, int32_t widget_route_id, - mojo::PendingRemote<mojom::Widget> widget) { + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host) { CreateNewWidget(render_process_id, widget_route_id, /*is_fullscreen=*/true, - std::move(widget)); + std::move(widget), render_view_host); } -void WebContentsImpl::CreateNewWidget( - int32_t render_process_id, - int32_t route_id, - bool is_fullscreen, - mojo::PendingRemote<mojom::Widget> widget) { +void WebContentsImpl::CreateNewWidget(int32_t render_process_id, + int32_t route_id, + bool is_fullscreen, + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host) { RenderProcessHost* process = RenderProcessHost::FromID(render_process_id); // A message to create a new widget can only come from an active process for // this WebContentsImpl instance. If any other process sends the request, @@ -3009,6 +3010,8 @@ void WebContentsImpl::CreateNewWidget( RenderWidgetHostImpl* widget_host = new RenderWidgetHostImpl( this, process, route_id, std::move(widget), IsHidden()); + widget_host->BindVisualPropertiesManager( + render_view_host->GetVisualPropertiesManager()); RenderWidgetHostViewBase* widget_view = static_cast<RenderWidgetHostViewBase*>( diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h index 31ec28bb4e059..8b8bc07e8b800 100644 --- a/content/browser/web_contents/web_contents_impl.h +++ b/content/browser/web_contents/web_contents_impl.h @@ -676,11 +676,12 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents, void UpdatePreferredSize(const gfx::Size& pref_size) override; void CreateNewWidget(int32_t render_process_id, int32_t route_id, - mojo::PendingRemote<mojom::Widget> widget) override; - void CreateNewFullscreenWidget( - int32_t render_process_id, - int32_t widget_route_id, - mojo::PendingRemote<mojom::Widget> widget) override; + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host) override; + void CreateNewFullscreenWidget(int32_t render_process_id, + int32_t widget_route_id, + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host) override; void ShowCreatedWidget(int process_id, int widget_route_id, const gfx::Rect& initial_rect) override; @@ -1403,7 +1404,8 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents, void CreateNewWidget(int32_t render_process_id, int32_t route_id, bool is_fullscreen, - mojo::PendingRemote<mojom::Widget> widget); + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host); // Helper for ShowCreatedWidget/ShowCreatedFullscreenWidget. void ShowCreatedWidget(int process_id, diff --git a/content/common/view_messages.h b/content/common/view_messages.h index 848ac033f07f7..e244932d0ba9d 100644 --- a/content/common/view_messages.h +++ b/content/common/view_messages.h @@ -200,12 +200,13 @@ IPC_MESSAGE_ROUTED1(ViewMsg_PpapiBrokerPermissionResult, bool /* result */) #endif -// Sent to the renderer hosting the local main frame when view visual properties -// are updated. Currently also carries state for main widget visual properties. -// Eventually we want this to be used for all page visual updates, not just -// local main frame view visual updates. -IPC_MESSAGE_ROUTED1(ViewMsg_UpdateLocalMainFrameVisualProperties, - content::VisualProperties /* visual_properties */) +// Updates a Renderer's visual properties, including both page properties and +// widget properties. Eventually, we'd like to be able to send separate IPCs, +// but it's not clear if that's possible. The |widget_routing_id| is used to +// forward widget visual properties to the relevant widget. +IPC_MESSAGE_ROUTED2(ViewMsg_UpdateVisualProperties, + content::VisualProperties /* visual_properties */, + int /* widget_routing_id */) // Sent to the main-frame's view to request performing a page scale animation // based on the point/rect provided. diff --git a/content/common/widget_messages.h b/content/common/widget_messages.h index 405771f0032fc..a780b4322062b 100644 --- a/content/common/widget_messages.h +++ b/content/common/widget_messages.h @@ -111,12 +111,6 @@ IPC_MESSAGE_ROUTED2(WidgetMsg_ShowContextMenu, // Expects a Close_ACK message when finished. IPC_MESSAGE_ROUTED0(WidgetMsg_Close) -// Tells the renderer to update visual properties. The resulting -// CompositorFrame will produce a RenderFrameMetadata containing a new -// LocalSurfaceId. This acts as a form of ACK for this message. -IPC_MESSAGE_ROUTED1(WidgetMsg_SynchronizeVisualProperties, - content::VisualProperties /* params */) - // Enables device emulation. See WebDeviceEmulationParams for description. IPC_MESSAGE_ROUTED1(WidgetMsg_EnableDeviceEmulation, blink::WebDeviceEmulationParams /* params */) diff --git a/content/public/test/render_view_test.cc b/content/public/test/render_view_test.cc index fe8fb0c39e92e..dfaf3fea7caf8 100644 --- a/content/public/test/render_view_test.cc +++ b/content/public/test/render_view_test.cc @@ -17,6 +17,7 @@ #include "content/common/frame_messages.h" #include "content/common/input_messages.h" #include "content/common/renderer.mojom.h" +#include "content/common/view_messages.h" #include "content/common/visual_properties.h" #include "content/common/widget_messages.h" #include "content/public/browser/content_browser_client.h" @@ -696,11 +697,13 @@ void RenderViewTest::Resize(gfx::Size new_size, visual_properties.browser_controls_shrink_blink_size = false; visual_properties.is_fullscreen_granted = is_fullscreen_granted; visual_properties.display_mode = blink::kWebDisplayModeBrowser; + RenderViewImpl* view = static_cast<RenderViewImpl*>(view_); + RenderWidget* render_widget = view->GetWidget(); std::unique_ptr<IPC::Message> resize_message( - new WidgetMsg_SynchronizeVisualProperties(0, visual_properties)); - RenderWidget* render_widget = - static_cast<RenderViewImpl*>(view_)->GetWidget(); - render_widget->OnMessageReceived(*resize_message); + new ViewMsg_UpdateVisualProperties(view->GetRoutingID(), + visual_properties, + render_widget->routing_id())); + view->OnMessageReceived(*resize_message); } void RenderViewTest::SimulateUserTypingASCIICharacter(char ascii_character, diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index 2e664afd607fa..f2b9bb2934ac4 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc @@ -1485,7 +1485,8 @@ RenderFrameImpl* RenderFrameImpl::CreateMainFrame( // in OnSynchronizeVisualProperties(), which would pass it along if it did // change. render_widget->UpdateWebViewWithDeviceScaleFactor(); - render_widget->OnSynchronizeVisualProperties(params->visual_properties); + render_widget->SynchronizeVisualPropertiesFromRenderView( + params->visual_properties); // The WebFrame created here was already attached to the Page as its // main frame, and the WebFrameWidget has been initialized, so we can call @@ -1715,7 +1716,7 @@ void RenderFrameImpl::CreateFrame( // thus would not get VisualProperty updates while the frame is provisional, // we need at least one update to them in order to meet expectations in the // renderer, and that update comes as part of the CreateFrame message. - render_frame->render_widget_->OnSynchronizeVisualProperties( + render_frame->render_widget_->SynchronizeVisualPropertiesFromRenderView( widget_params->visual_properties); } diff --git a/content/renderer/render_frame_impl_browsertest.cc b/content/renderer/render_frame_impl_browsertest.cc index 23961e9bac722..2452f2ffc980b 100644 --- a/content/renderer/render_frame_impl_browsertest.cc +++ b/content/renderer/render_frame_impl_browsertest.cc @@ -22,6 +22,7 @@ #include "content/common/navigation_params_mojom_traits.h" #include "content/common/renderer.mojom.h" #include "content/common/unfreezable_frame_messages.h" +#include "content/common/view_messages.h" #include "content/common/widget_messages.h" #include "content/public/common/content_switches.h" #include "content/public/common/previews_state.h" @@ -229,19 +230,24 @@ TEST_F(RenderFrameImplTest, FrameResize) { visual_properties.browser_controls_shrink_blink_size = false; visual_properties.is_fullscreen_granted = false; - WidgetMsg_SynchronizeVisualProperties resize_message(0, visual_properties); - // The main frame's widget will receive the resize message before the // subframe's widget, and it will set the size for the WebView. RenderWidget* main_frame_widget = GetMainRenderFrame()->GetLocalRootRenderWidget(); - main_frame_widget->OnMessageReceived(resize_message); + ViewMsg_UpdateVisualProperties resize_message( + view_->GetRoutingID(), visual_properties, + main_frame_widget->routing_id()); + + static_cast<RenderViewImpl*>(view_)->OnMessageReceived(resize_message); EXPECT_EQ(view_->GetWebView()->MainFrameWidget()->Size(), blink::WebSize(size)); EXPECT_NE(frame_widget()->GetWebWidget()->Size(), blink::WebSize(size)); // The subframe sets the size only for itself. - frame_widget()->OnMessageReceived(resize_message); + ViewMsg_UpdateVisualProperties resize_message_subframe( + view_->GetRoutingID(), visual_properties, frame_widget()->routing_id()); + static_cast<RenderViewImpl*>(view_)->OnMessageReceived( + resize_message_subframe); EXPECT_EQ(frame_widget()->GetWebWidget()->Size(), blink::WebSize(size)); } diff --git a/content/renderer/render_view_browsertest.cc b/content/renderer/render_view_browsertest.cc index d587d02d60b69..3b5eb28f790e1 100644 --- a/content/renderer/render_view_browsertest.cc +++ b/content/renderer/render_view_browsertest.cc @@ -468,7 +468,7 @@ class RenderViewImplScaleFactorTest : public RenderViewImplTest { } void SetDeviceScaleFactor(float dsf) { - view()->GetWidget()->OnSynchronizeVisualProperties( + view()->GetWidget()->SynchronizeVisualPropertiesFromRenderView( MakeVisualPropertiesWithDeviceScaleFactor(dsf)); ASSERT_EQ(dsf, view()->page_properties()->GetDeviceScaleFactor()); ASSERT_EQ(dsf, diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc index 7a82aca3e4594..4da7798e01aca 100644 --- a/content/renderer/render_view_impl.cc +++ b/content/renderer/render_view_impl.cc @@ -1241,8 +1241,8 @@ bool RenderViewImpl::OnMessageReceived(const IPC::Message& message) { IPC_MESSAGE_HANDLER(ViewMsg_MoveOrResizeStarted, OnMoveOrResizeStarted) IPC_MESSAGE_HANDLER(ViewMsg_EnablePreferredSizeChangedMode, OnEnablePreferredSizeChangedMode) - IPC_MESSAGE_HANDLER(ViewMsg_UpdateLocalMainFrameVisualProperties, - OnUpdateLocalMainFramePageVisualProperties) + IPC_MESSAGE_HANDLER(ViewMsg_UpdateVisualProperties, + OnUpdateVisualProperties) IPC_MESSAGE_HANDLER(ViewMsg_SetRendererPrefs, OnSetRendererPrefs) IPC_MESSAGE_HANDLER(ViewMsg_PluginActionAt, OnPluginActionAt) IPC_MESSAGE_HANDLER(ViewMsg_AnimateDoubleTapZoom, @@ -1996,15 +1996,15 @@ void RenderViewImpl::OnPageWasShown() { ApplyPageHidden(/*hidden=*/false, /*initial_setting=*/false); } -void RenderViewImpl::OnUpdateLocalMainFramePageVisualProperties( - const VisualProperties& visual_properties) { +void RenderViewImpl::OnUpdateVisualProperties( + const VisualProperties& visual_properties, + int widget_routing_id) { // TODO(https://crbug.com/998273): We should not forward visual properties to // frozen render widgets. - // If the main frame is detached while the IPC is in flight, then - // render_widget_ may be nullptr when we get here [once we get rid of frozen - // RenderWidgets]. In that case, don't forward anything. - if (render_widget_) { - render_widget_->OnSynchronizeVisualProperties(visual_properties); + // The widget may have been destroyed while the IPC was in flight. + RenderWidget* widget = RenderWidget::FromRoutingID(widget_routing_id); + if (widget) { + widget->SynchronizeVisualPropertiesFromRenderView(visual_properties); } } diff --git a/content/renderer/render_view_impl.h b/content/renderer/render_view_impl.h index 92952cd3a544b..32f2abd1b3f3f 100644 --- a/content/renderer/render_view_impl.h +++ b/content/renderer/render_view_impl.h @@ -463,12 +463,16 @@ class CONTENT_EXPORT RenderViewImpl : public blink::WebViewClient, void OnAudioStateChanged(bool is_audio_playing); void OnSetBackgroundOpaque(bool opaque); + // |visual_properties| contains both page and widget scoped properties. This + // method consumes the page visual properties and forwards widget properties + // using |widget_routing_id|. + void OnUpdateVisualProperties(const VisualProperties& visual_properties, + int widget_routing_id); + // Page message handlers ----------------------------------------------------- void OnPageWasHidden(); void OnPageWasShown(); void OnUpdateScreenInfo(const ScreenInfo& screen_info); - void OnUpdateLocalMainFramePageVisualProperties( - const VisualProperties& visual_properties); void OnUpdatePageVisualProperties(const gfx::Size& visible_viewport_size); void SetPageFrozen(bool frozen); void PutPageIntoBackForwardCache(); diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc index a0740f41a6198..f100cb0c75143 100644 --- a/content/renderer/render_widget.cc +++ b/content/renderer/render_widget.cc @@ -670,8 +670,6 @@ bool RenderWidget::OnMessageReceived(const IPC::Message& message) { IPC_MESSAGE_HANDLER(WidgetMsg_SetIsInert, OnSetIsInert) IPC_MESSAGE_HANDLER(WidgetMsg_SetInheritedEffectiveTouchAction, OnSetInheritedEffectiveTouchAction) - IPC_MESSAGE_HANDLER(WidgetMsg_SynchronizeVisualProperties, - OnSynchronizeVisualProperties) IPC_MESSAGE_HANDLER(WidgetMsg_UpdateRenderThrottlingStatus, OnUpdateRenderThrottlingStatus) IPC_MESSAGE_HANDLER(WidgetMsg_WaitForNextFrameForTests, @@ -763,9 +761,10 @@ void RenderWidget::PrepareForClose() { CloseWebWidget(); } -void RenderWidget::OnSynchronizeVisualProperties( +void RenderWidget::SynchronizeVisualPropertiesFromRenderView( const VisualProperties& visual_properties_from_browser) { - TRACE_EVENT0("renderer", "RenderWidget::OnSynchronizeVisualProperties"); + TRACE_EVENT0("renderer", + "RenderWidget::SynchronizeVisualPropertiesFromRenderView"); VisualProperties visual_properties = visual_properties_from_browser; // Web tests can override the device scale factor in the renderer. diff --git a/content/renderer/render_widget.h b/content/renderer/render_widget.h index f840c77165ea6..e166b5bf22136 100644 --- a/content/renderer/render_widget.h +++ b/content/renderer/render_widget.h @@ -690,8 +690,9 @@ class CONTENT_EXPORT RenderWidget base::OnceCallback<void(const gfx::PresentationFeedback&)>; virtual void RequestPresentation(PresentationTimeCallback callback); - // RenderWidget IPC message handler that can be overridden by subclasses. - virtual void OnSynchronizeVisualProperties( + // Handles widget VisualProperties updates that are coming from the RenderView + // [routing IPC from browser]. + virtual void SynchronizeVisualPropertiesFromRenderView( const VisualProperties& visual_properties); bool in_synchronous_composite_for_testing() const { diff --git a/content/renderer/render_widget_browsertest.cc b/content/renderer/render_widget_browsertest.cc index 2ba0f7a35156a..45d4972fb2131 100644 --- a/content/renderer/render_widget_browsertest.cc +++ b/content/renderer/render_widget_browsertest.cc @@ -28,7 +28,7 @@ class RenderWidgetTest : public RenderViewTest { } void OnSynchronizeVisualProperties(const VisualProperties& params) { - widget()->OnSynchronizeVisualProperties(params); + widget()->SynchronizeVisualPropertiesFromRenderView(params); } void GetCompositionRange(gfx::Range* range) { @@ -234,7 +234,7 @@ TEST_F(RenderWidgetTest, ActivePinchGestureUpdatesLayerTreeHost) { // Sync visual properties on a mainframe RenderWidget. visual_properties.is_pinch_gesture_active = true; - widget()->OnSynchronizeVisualProperties(visual_properties); + widget()->SynchronizeVisualPropertiesFromRenderView(visual_properties); // We do not expect the |is_pinch_gesture_active| value to propagate to the // LayerTreeHost for the main-frame. Since GesturePinch events are handled // directly by the layer tree for the main frame, it already knows whether or diff --git a/content/renderer/render_widget_fullscreen_pepper.cc b/content/renderer/render_widget_fullscreen_pepper.cc index 9bf822a75da94..781c4d75fc7f5 100644 --- a/content/renderer/render_widget_fullscreen_pepper.cc +++ b/content/renderer/render_widget_fullscreen_pepper.cc @@ -382,9 +382,9 @@ void RenderWidgetFullscreenPepper::Close(std::unique_ptr<RenderWidget> widget) { RenderWidget::Close(std::move(widget)); } -void RenderWidgetFullscreenPepper::OnSynchronizeVisualProperties( +void RenderWidgetFullscreenPepper::SynchronizeVisualPropertiesFromRenderView( const VisualProperties& visual_properties) { - RenderWidget::OnSynchronizeVisualProperties(visual_properties); + RenderWidget::SynchronizeVisualPropertiesFromRenderView(visual_properties); UpdateLayerBounds(); } diff --git a/content/renderer/render_widget_fullscreen_pepper.h b/content/renderer/render_widget_fullscreen_pepper.h index cc08b381a2050..e78e161c3a8d7 100644 --- a/content/renderer/render_widget_fullscreen_pepper.h +++ b/content/renderer/render_widget_fullscreen_pepper.h @@ -70,7 +70,7 @@ class RenderWidgetFullscreenPepper : public RenderWidget, // RenderWidget API. void DidInitiatePaint() override; void Close(std::unique_ptr<RenderWidget> widget) override; - void OnSynchronizeVisualProperties( + void SynchronizeVisualPropertiesFromRenderView( const VisualProperties& visual_properties) override; private: diff --git a/content/renderer/render_widget_unittest.cc b/content/renderer/render_widget_unittest.cc index d36bbe97826ac..83f2320180bda 100644 --- a/content/renderer/render_widget_unittest.cc +++ b/content/renderer/render_widget_unittest.cc @@ -563,7 +563,7 @@ TEST_F(RenderWidgetUnittest, ActivePinchGestureUpdatesLayerTreeHostSubFrame) { // Sync visual properties on a child RenderWidget. visual_properties.is_pinch_gesture_active = true; - widget()->OnSynchronizeVisualProperties(visual_properties); + widget()->SynchronizeVisualPropertiesFromRenderView(visual_properties); // We expect the |is_pinch_gesture_active| value to propagate to the // LayerTreeHost for sub-frames. Since GesturePinch events are handled // directly in the main-frame's layer tree (and only there), information about @@ -573,7 +573,7 @@ TEST_F(RenderWidgetUnittest, ActivePinchGestureUpdatesLayerTreeHostSubFrame) { // pinch gestures are active. EXPECT_TRUE(layer_tree_host->is_external_pinch_gesture_active_for_testing()); visual_properties.is_pinch_gesture_active = false; - widget()->OnSynchronizeVisualProperties(visual_properties); + widget()->SynchronizeVisualPropertiesFromRenderView(visual_properties); EXPECT_FALSE(layer_tree_host->is_external_pinch_gesture_active_for_testing()); } @@ -605,7 +605,7 @@ TEST_F(RenderWidgetPopupUnittest, EmulatingPopupRect) { new PopupRenderWidget(&compositor_deps_, &page_properties_)); // Setup emulation on the |parent_widget|. - parent_widget->OnSynchronizeVisualProperties(visual_properties); + parent_widget->SynchronizeVisualPropertiesFromRenderView(visual_properties); parent_widget->OnEnableDeviceEmulation(emulation_params); // Then use it for the popup widget under test. widget()->ApplyEmulatedScreenMetricsForPopupWidget(parent_widget.get()); diff --git a/content/test/stub_render_widget_host_owner_delegate.h b/content/test/stub_render_widget_host_owner_delegate.h index f00c61cad2f5f..3185b79b98f38 100644 --- a/content/test/stub_render_widget_host_owner_delegate.h +++ b/content/test/stub_render_widget_host_owner_delegate.h @@ -27,8 +27,6 @@ class StubRenderWidgetHostOwnerDelegate : public RenderWidgetHostOwnerDelegate { bool IsNeverVisible() override; WebPreferences GetWebkitPreferencesForWidget() override; FrameTreeNode* GetFocusedFrame() override; - void UpdatePageVisualProperties( - const VisualProperties& visual_properties) override {} void ShowContextMenu(RenderFrameHost* render_frame_host, const ContextMenuParams& params) override {} }; diff --git a/content/test/test_web_contents.cc b/content/test/test_web_contents.cc index 1dc9589e0e3d2..d1f42372584b0 100644 --- a/content/test/test_web_contents.cc +++ b/content/test/test_web_contents.cc @@ -388,15 +388,16 @@ void TestWebContents::CreateNewWindow( bool has_user_gesture, SessionStorageNamespace* session_storage_namespace) {} -void TestWebContents::CreateNewWidget( - int32_t render_process_id, - int32_t route_id, - mojo::PendingRemote<mojom::Widget> widget) {} +void TestWebContents::CreateNewWidget(int32_t render_process_id, + int32_t route_id, + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host) {} void TestWebContents::CreateNewFullscreenWidget( int32_t render_process_id, int32_t route_id, - mojo::PendingRemote<mojom::Widget> widget) {} + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host) {} void TestWebContents::ShowCreatedWindow(int process_id, int route_id, diff --git a/content/test/test_web_contents.h b/content/test/test_web_contents.h index 77f4f16898f00..baebeb8b1c159 100644 --- a/content/test/test_web_contents.h +++ b/content/test/test_web_contents.h @@ -188,11 +188,12 @@ class TestWebContents : public WebContentsImpl, public WebContentsTester { SessionStorageNamespace* session_storage_namespace) override; void CreateNewWidget(int32_t render_process_id, int32_t route_id, - mojo::PendingRemote<mojom::Widget> widget) override; - void CreateNewFullscreenWidget( - int32_t render_process_id, - int32_t route_id, - mojo::PendingRemote<mojom::Widget> widget) override; + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host) override; + void CreateNewFullscreenWidget(int32_t render_process_id, + int32_t route_id, + mojo::PendingRemote<mojom::Widget> widget, + RenderViewHostImpl* render_view_host) override; void ShowCreatedWindow(int process_id, int route_id, WindowOpenDisposition disposition,