diff --git a/content/browser/renderer_host/browser_compositor_view_mac.mm b/content/browser/renderer_host/browser_compositor_view_mac.mm index e91c70a19d6c3a..07d42e680951ac 100644 --- a/content/browser/renderer_host/browser_compositor_view_mac.mm +++ b/content/browser/renderer_host/browser_compositor_view_mac.mm @@ -188,7 +188,8 @@ void OnCompositingShuttingDown(ui::Compositor* compositor) override {} root_layer_.reset(new ui::Layer(ui::LAYER_SOLID_COLOR)); delegated_frame_host_.reset(new DelegatedFrameHost( frame_sink_id, this, features::IsSurfaceSynchronizationEnabled(), - base::FeatureList::IsEnabled(features::kVizDisplayCompositor))); + base::FeatureList::IsEnabled(features::kVizDisplayCompositor), + true /* should_register_frame_sink_id */)); dfh_display_ = display::Screen::GetScreen()->GetDisplayNearestView(nil); diff --git a/content/browser/renderer_host/delegated_frame_host.cc b/content/browser/renderer_host/delegated_frame_host.cc index 58121d905097d2..db7b0777f9de1a 100644 --- a/content/browser/renderer_host/delegated_frame_host.cc +++ b/content/browser/renderer_host/delegated_frame_host.cc @@ -37,11 +37,13 @@ namespace content { DelegatedFrameHost::DelegatedFrameHost(const viz::FrameSinkId& frame_sink_id, DelegatedFrameHostClient* client, bool enable_surface_synchronization, - bool enable_viz) + bool enable_viz, + bool should_register_frame_sink_id) : frame_sink_id_(frame_sink_id), client_(client), enable_surface_synchronization_(enable_surface_synchronization), enable_viz_(enable_viz), + should_register_frame_sink_id_(should_register_frame_sink_id), tick_clock_(base::DefaultTickClock::GetInstance()), frame_evictor_(std::make_unique(this)) { ImageTransportFactory* factory = ImageTransportFactory::GetInstance(); @@ -639,7 +641,8 @@ void DelegatedFrameHost::SetCompositor(ui::Compositor* compositor) { return; compositor_ = compositor; compositor_->AddObserver(this); - compositor_->AddFrameSink(frame_sink_id_); + if (should_register_frame_sink_id_) + compositor_->AddFrameSink(frame_sink_id_); } void DelegatedFrameHost::ResetCompositor() { @@ -648,7 +651,8 @@ void DelegatedFrameHost::ResetCompositor() { resize_lock_.reset(); if (compositor_->HasObserver(this)) compositor_->RemoveObserver(this); - compositor_->RemoveFrameSink(frame_sink_id_); + if (should_register_frame_sink_id_) + compositor_->RemoveFrameSink(frame_sink_id_); compositor_ = nullptr; } @@ -674,7 +678,7 @@ void DelegatedFrameHost::CreateCompositorFrameSinkSupport() { ->GetHostFrameSinkManager() ->CreateCompositorFrameSinkSupport(this, frame_sink_id_, is_root, needs_sync_points); - if (compositor_) + if (compositor_ && should_register_frame_sink_id_) compositor_->AddFrameSink(frame_sink_id_); if (needs_begin_frame_) support_->SetNeedsBeginFrame(true); @@ -683,7 +687,7 @@ void DelegatedFrameHost::CreateCompositorFrameSinkSupport() { void DelegatedFrameHost::ResetCompositorFrameSinkSupport() { if (!support_) return; - if (compositor_) + if (compositor_ && should_register_frame_sink_id_) compositor_->RemoveFrameSink(frame_sink_id_); support_.reset(); } diff --git a/content/browser/renderer_host/delegated_frame_host.h b/content/browser/renderer_host/delegated_frame_host.h index 6fef880b6ad9f7..7aa0d9ded6bfb1 100644 --- a/content/browser/renderer_host/delegated_frame_host.h +++ b/content/browser/renderer_host/delegated_frame_host.h @@ -76,10 +76,15 @@ class CONTENT_EXPORT DelegatedFrameHost public viz::mojom::CompositorFrameSinkClient, public viz::HostFrameSinkClient { public: + // |should_register_frame_sink_id| flag indicates whether DelegatedFrameHost + // is responsible for registering the associated FrameSinkId with the + // compositor or not. This is set only on non-aura platforms, since aura is + // responsible for doing the appropriate [un]registration. DelegatedFrameHost(const viz::FrameSinkId& frame_sink_id, DelegatedFrameHostClient* client, bool enable_surface_synchronization, - bool enable_viz); + bool enable_viz, + bool should_register_frame_sink_id); ~DelegatedFrameHost() override; // ui::CompositorObserver implementation. @@ -226,6 +231,7 @@ class CONTENT_EXPORT DelegatedFrameHost DelegatedFrameHostClient* const client_; const bool enable_surface_synchronization_; const bool enable_viz_; + const bool should_register_frame_sink_id_; ui::Compositor* compositor_ = nullptr; // The surface id that was most recently activated by diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc index 127ccbb73e4aee..daea689f1270fd 100644 --- a/content/browser/renderer_host/render_widget_host_view_aura.cc +++ b/content/browser/renderer_host/render_widget_host_view_aura.cc @@ -1904,8 +1904,6 @@ void RenderWidgetHostViewAura::CreateAuraWindow(aura::client::WindowType type) { DCHECK(!window_); DCHECK(!is_mus_browser_plugin_guest_); window_ = new aura::Window(this); - if (frame_sink_id_.is_valid()) - window_->set_embed_frame_sink_id(frame_sink_id_); window_->SetName("RenderWidgetHostViewAura"); window_->SetProperty(aura::client::kEmbedType, aura::client::WindowEmbedType::EMBED_IN_OWNER); @@ -1920,6 +1918,10 @@ void RenderWidgetHostViewAura::CreateAuraWindow(aura::client::WindowType type) { window_->SetType(type); window_->Init(ui::LAYER_SOLID_COLOR); window_->layer()->SetColor(background_color_); + // This needs to happen only after |window_| has been initialized using + // Init(), because it needs to have the layer. + if (frame_sink_id_.is_valid()) + window_->SetEmbedFrameSinkId(frame_sink_id_); if (!features::IsMusEnabled()) return; @@ -1948,7 +1950,8 @@ void RenderWidgetHostViewAura::CreateDelegatedFrameHostClient() { base::FeatureList::IsEnabled(features::kVizDisplayCompositor); delegated_frame_host_ = std::make_unique( frame_sink_id_, delegated_frame_host_client_.get(), - features::IsSurfaceSynchronizationEnabled(), enable_viz); + features::IsSurfaceSynchronizationEnabled(), enable_viz, + false /* should_register_frame_sink_id */); if (!create_frame_sink_callback_.is_null()) std::move(create_frame_sink_callback_).Run(frame_sink_id_); diff --git a/ui/aura/hit_test_data_provider_aura_unittest.cc b/ui/aura/hit_test_data_provider_aura_unittest.cc index caec766659e1e3..96a6d82decd14b 100644 --- a/ui/aura/hit_test_data_provider_aura_unittest.cc +++ b/ui/aura/hit_test_data_provider_aura_unittest.cc @@ -190,7 +190,7 @@ TEST_F(HitTestDataProviderAuraTest, Stacking) { // Tests that the hit-test regions get expanded with a custom event targeter. TEST_F(HitTestDataProviderAuraTest, CustomTargeter) { window3()->SetEventTargeter(std::make_unique()); - window2()->set_embed_frame_sink_id(viz::FrameSinkId(1, 2)); + window2()->SetEmbedFrameSinkId(viz::FrameSinkId(1, 2)); const auto hit_test_data = hit_test_data_provider()->GetHitTestData(compositor_frame_); ASSERT_TRUE(hit_test_data); @@ -321,7 +321,7 @@ TEST_F(HitTestDataProviderAuraTest, DoNotSubmit) { ASSERT_TRUE(hit_test_data); EXPECT_EQ(hit_test_data->regions.size(), 2u); - window3()->set_embed_frame_sink_id(viz::FrameSinkId(1, 3)); + window3()->SetEmbedFrameSinkId(viz::FrameSinkId(1, 3)); hit_test_data = hit_test_data_provider()->GetHitTestData(compositor_frame_); ASSERT_TRUE(hit_test_data); EXPECT_EQ(hit_test_data->regions.size(), 1u); @@ -334,7 +334,7 @@ TEST_F(HitTestDataProviderAuraTest, DoNotSubmit) { hit_test_data = hit_test_data_provider()->GetHitTestData(compositor_frame_); ASSERT_TRUE(hit_test_data); EXPECT_EQ(hit_test_data->regions.size(), 1u); - root()->set_embed_frame_sink_id(viz::FrameSinkId(1, 1)); + root()->SetEmbedFrameSinkId(viz::FrameSinkId(1, 1)); hit_test_data = hit_test_data_provider()->GetHitTestData(compositor_frame_); ASSERT_TRUE(hit_test_data); EXPECT_EQ(hit_test_data->regions.size(), 0u); diff --git a/ui/aura/local/window_port_local.cc b/ui/aura/local/window_port_local.cc index e027d23cd7d452..91bd62996c290b 100644 --- a/ui/aura/local/window_port_local.cc +++ b/ui/aura/local/window_port_local.cc @@ -125,12 +125,11 @@ WindowPortLocal::CreateLayerTreeFrameSink() { auto frame_sink = std::make_unique( frame_sink_id_, context_factory_private->GetHostFrameSinkManager(), window_->GetName()); + window_->SetEmbedFrameSinkId(frame_sink_id_); frame_sink->SetSurfaceChangedCallback(base::Bind( &WindowPortLocal::OnSurfaceChanged, weak_factory_.GetWeakPtr())); frame_sink_ = frame_sink->GetWeakPtr(); AllocateLocalSurfaceId(); - if (window_->GetRootWindow()) - window_->layer()->GetCompositor()->AddFrameSink(frame_sink_id_); return std::move(frame_sink); } @@ -152,16 +151,6 @@ viz::FrameSinkId WindowPortLocal::GetFrameSinkId() const { return frame_sink_id_; } -void WindowPortLocal::OnWindowAddedToRootWindow() { - if (frame_sink_id_.is_valid()) - window_->layer()->GetCompositor()->AddFrameSink(frame_sink_id_); -} - -void WindowPortLocal::OnWillRemoveWindowFromRootWindow() { - if (frame_sink_id_.is_valid()) - window_->layer()->GetCompositor()->RemoveFrameSink(frame_sink_id_); -} - void WindowPortLocal::OnEventTargetingPolicyChanged() {} void WindowPortLocal::OnSurfaceChanged(const viz::SurfaceInfo& surface_info) { diff --git a/ui/aura/local/window_port_local.h b/ui/aura/local/window_port_local.h index bd4fa7d1a7d18a..61bf95984fb941 100644 --- a/ui/aura/local/window_port_local.h +++ b/ui/aura/local/window_port_local.h @@ -49,8 +49,6 @@ class AURA_EXPORT WindowPortLocal : public WindowPort { void AllocateLocalSurfaceId() override; const viz::LocalSurfaceId& GetLocalSurfaceId() override; viz::FrameSinkId GetFrameSinkId() const override; - void OnWindowAddedToRootWindow() override; - void OnWillRemoveWindowFromRootWindow() override; void OnEventTargetingPolicyChanged() override; bool ShouldRestackTransientChildren() override; diff --git a/ui/aura/mus/window_port_mus.cc b/ui/aura/mus/window_port_mus.cc index c6e9f38fa5c6b5..a331d49e2cb114 100644 --- a/ui/aura/mus/window_port_mus.cc +++ b/ui/aura/mus/window_port_mus.cc @@ -303,7 +303,7 @@ void WindowPortMus::SetFrameSinkIdFromServer( const viz::FrameSinkId& frame_sink_id) { DCHECK(window_mus_type() == WindowMusType::TOP_LEVEL_IN_WM || window_mus_type() == WindowMusType::EMBED_IN_OWNER); - window_->set_embed_frame_sink_id(frame_sink_id); + window_->SetEmbedFrameSinkId(frame_sink_id); UpdatePrimarySurfaceId(); } @@ -334,7 +334,7 @@ void WindowPortMus::SetFallbackSurfaceInfo( // |primary_surface_id_| shold not be valid, since we didn't know the // |window_->embed_frame_sink_id()|. DCHECK(!primary_surface_id_.is_valid()); - window_->set_embed_frame_sink_id(surface_info.id().frame_sink_id()); + window_->SetEmbedFrameSinkId(surface_info.id().frame_sink_id()); UpdatePrimarySurfaceId(); } @@ -579,10 +579,7 @@ WindowPortMus::CreateLayerTreeFrameSink() { window_->GetName()); layer_tree_frame_sink_local->SetSurfaceChangedCallback(base::BindRepeating( &WindowPortMus::OnSurfaceChanged, weak_ptr_factory_.GetWeakPtr())); - if (window_->layer()->GetCompositor()) { - window_->layer()->GetCompositor()->AddFrameSink(GetFrameSinkId()); - is_frame_sink_id_added_to_compositor_ = true; - } + window_->SetEmbedFrameSinkId(frame_sink_id); local_layer_tree_frame_sink_ = layer_tree_frame_sink_local->GetWeakPtr(); frame_sink = std::move(layer_tree_frame_sink_local); } @@ -595,25 +592,6 @@ WindowPortMus::CreateLayerTreeFrameSink() { return frame_sink; } -void WindowPortMus::OnWindowAddedToRootWindow() { - if (base::FeatureList::IsEnabled(features::kMash)) - return; - if (local_layer_tree_frame_sink_) { - DCHECK(!is_frame_sink_id_added_to_compositor_); - window_->layer()->GetCompositor()->AddFrameSink(GetFrameSinkId()); - is_frame_sink_id_added_to_compositor_ = true; - } -} - -void WindowPortMus::OnWillRemoveWindowFromRootWindow() { - if (base::FeatureList::IsEnabled(features::kMash)) - return; - if (is_frame_sink_id_added_to_compositor_) { - window_->layer()->GetCompositor()->RemoveFrameSink(GetFrameSinkId()); - is_frame_sink_id_added_to_compositor_ = false; - } -} - void WindowPortMus::OnEventTargetingPolicyChanged() { SetEventTargetingPolicy(window_->event_targeting_policy()); } diff --git a/ui/aura/mus/window_port_mus.h b/ui/aura/mus/window_port_mus.h index 2b86740b3bf95b..c476ba5ac4304c 100644 --- a/ui/aura/mus/window_port_mus.h +++ b/ui/aura/mus/window_port_mus.h @@ -277,8 +277,6 @@ class AURA_EXPORT WindowPortMus : public WindowPort, public WindowMus { std::unique_ptr CreateLayerTreeFrameSink() override; void AllocateLocalSurfaceId() override; const viz::LocalSurfaceId& GetLocalSurfaceId() override; - void OnWindowAddedToRootWindow() override; - void OnWillRemoveWindowFromRootWindow() override; void OnEventTargetingPolicyChanged() override; bool ShouldRestackTransientChildren() override; diff --git a/ui/aura/mus/window_port_mus_unittest.cc b/ui/aura/mus/window_port_mus_unittest.cc index 217a3f6d8449b7..8425624d63c89c 100644 --- a/ui/aura/mus/window_port_mus_unittest.cc +++ b/ui/aura/mus/window_port_mus_unittest.cc @@ -35,7 +35,7 @@ TEST_F(WindowPortMusTest, LayerTreeFrameSinkGetsCorrectLocalSurfaceId) { window.SetBounds(gfx::Rect(300, 300)); // Notify the window that it will embed an external client, so that it // correctly generates LocalSurfaceId. - window.set_embed_frame_sink_id(viz::FrameSinkId(0, 1)); + window.SetEmbedFrameSinkId(viz::FrameSinkId(0, 1)); viz::LocalSurfaceId local_surface_id = window.GetLocalSurfaceId(); ASSERT_TRUE(local_surface_id.is_valid()); @@ -64,7 +64,7 @@ TEST_F(WindowPortMusTest, ClientSurfaceEmbedderUpdatesLayer) { Window window(nullptr); window.Init(ui::LAYER_NOT_DRAWN); window.SetBounds(gfx::Rect(300, 300)); - window.set_embed_frame_sink_id(viz::FrameSinkId(0, 1)); + window.SetEmbedFrameSinkId(viz::FrameSinkId(0, 1)); // Allocate a new LocalSurfaceId. The ui::Layer should be updated. window.AllocateLocalSurfaceId(); diff --git a/ui/aura/mus/window_tree_client.cc b/ui/aura/mus/window_tree_client.cc index dd61f0da594c31..4a123788244063 100644 --- a/ui/aura/mus/window_tree_client.cc +++ b/ui/aura/mus/window_tree_client.cc @@ -1203,6 +1203,8 @@ void WindowTreeClient::OnCaptureChanged(ui::Id new_capture_window_id, void WindowTreeClient::OnFrameSinkIdAllocated( ui::Id window_id, const viz::FrameSinkId& frame_sink_id) { + if (!base::FeatureList::IsEnabled(features::kMash)) + return; WindowMus* window = GetWindowByServerId(window_id); if (!window) return; diff --git a/ui/aura/test/window_test_api.cc b/ui/aura/test/window_test_api.cc index 86b3370b1126dd..52f8f7ee406f07 100644 --- a/ui/aura/test/window_test_api.cc +++ b/ui/aura/test/window_test_api.cc @@ -27,5 +27,9 @@ bool WindowTestApi::ContainsMouse() const { host->dispatcher()->GetLastMouseLocationInRoot()); } +void WindowTestApi::DisableFrameSinkRegistration() { + window_->disable_frame_sink_id_registration_ = true; +} + } // namespace test } // namespace aura diff --git a/ui/aura/test/window_test_api.h b/ui/aura/test/window_test_api.h index da131dace94f2c..4a0a2f3150a49e 100644 --- a/ui/aura/test/window_test_api.h +++ b/ui/aura/test/window_test_api.h @@ -21,6 +21,8 @@ class WindowTestApi { bool ContainsMouse() const; + void DisableFrameSinkRegistration(); + private: Window* window_; diff --git a/ui/aura/window.cc b/ui/aura/window.cc index eaf88a4f85c87d..33c2ce7d0806df 100644 --- a/ui/aura/window.cc +++ b/ui/aura/window.cc @@ -957,7 +957,8 @@ void Window::OnStackingChanged() { } void Window::NotifyRemovingFromRootWindow(Window* new_root) { - port_->OnWillRemoveWindowFromRootWindow(); + if (IsEmbeddingClient()) + UnregisterFrameSinkId(); for (WindowObserver& observer : observers_) observer.OnWindowRemovingFromRootWindow(this, new_root); for (Window::Windows::const_iterator it = children_.begin(); @@ -967,7 +968,8 @@ void Window::NotifyRemovingFromRootWindow(Window* new_root) { } void Window::NotifyAddedToRootWindow() { - port_->OnWindowAddedToRootWindow(); + if (IsEmbeddingClient()) + RegisterFrameSinkId(); for (WindowObserver& observer : observers_) observer.OnWindowAddedToRootWindow(this); for (Window::Windows::const_iterator it = children_.begin(); @@ -1109,6 +1111,12 @@ viz::FrameSinkId Window::GetFrameSinkId() const { return port_->GetFrameSinkId(); } +void Window::SetEmbedFrameSinkId(const viz::FrameSinkId& frame_sink_id) { + DCHECK(frame_sink_id.is_valid()); + embed_frame_sink_id_ = frame_sink_id; + RegisterFrameSinkId(); +} + bool Window::IsEmbeddingClient() const { return embed_frame_sink_id_.is_valid(); } @@ -1250,4 +1258,23 @@ void Window::UpdateLayerName() { #endif } +void Window::RegisterFrameSinkId() { + DCHECK(embed_frame_sink_id_.is_valid()); + DCHECK(IsEmbeddingClient()); + if (registered_frame_sink_id_ || disable_frame_sink_id_registration_) + return; + if (auto* compositor = layer()->GetCompositor()) { + compositor->AddFrameSink(embed_frame_sink_id_); + registered_frame_sink_id_ = true; + } +} + +void Window::UnregisterFrameSinkId() { + if (!registered_frame_sink_id_) + return; + registered_frame_sink_id_ = false; + if (auto* compositor = layer()->GetCompositor()) + compositor->RemoveFrameSink(embed_frame_sink_id_); +} + } // namespace aura diff --git a/ui/aura/window.h b/ui/aura/window.h index 515b82af04e59f..3cf40c5df5c751 100644 --- a/ui/aura/window.h +++ b/ui/aura/window.h @@ -397,9 +397,8 @@ class AURA_EXPORT Window : public ui::LayerDelegate, const viz::FrameSinkId& embed_frame_sink_id() const { return embed_frame_sink_id_; } - void set_embed_frame_sink_id(const viz::FrameSinkId& embed_frame_sink_id) { - embed_frame_sink_id_ = embed_frame_sink_id; - } + void SetEmbedFrameSinkId(const viz::FrameSinkId& embed_frame_sink_id); + // Returns whether this window is an embed window. bool IsEmbeddingClient() const; @@ -529,6 +528,11 @@ class AURA_EXPORT Window : public ui::LayerDelegate, // Updates the layer name based on the window's name and id. void UpdateLayerName(); + void RegisterFrameSinkId(); + void UnregisterFrameSinkId(); + bool registered_frame_sink_id_ = false; + bool disable_frame_sink_id_registration_ = false; + // Window owns its corresponding WindowPort, but the ref is held as a raw // pointer in |port_| so that it can still be accessed during destruction. // This is important as deleting the WindowPort may result in trying to lookup diff --git a/ui/aura/window_port.h b/ui/aura/window_port.h index 88984bdc301847..d47a92ab0143f5 100644 --- a/ui/aura/window_port.h +++ b/ui/aura/window_port.h @@ -98,9 +98,6 @@ class AURA_EXPORT WindowPort { // This can return invalid FrameSinkId. virtual viz::FrameSinkId GetFrameSinkId() const = 0; - virtual void OnWindowAddedToRootWindow() = 0; - virtual void OnWillRemoveWindowFromRootWindow() = 0; - virtual void OnEventTargetingPolicyChanged() = 0; // See description of function with same name in transient_window_client. diff --git a/ui/aura/window_port_for_shutdown.cc b/ui/aura/window_port_for_shutdown.cc index fd0c12af649848..168894729cc371 100644 --- a/ui/aura/window_port_for_shutdown.cc +++ b/ui/aura/window_port_for_shutdown.cc @@ -67,10 +67,6 @@ viz::FrameSinkId WindowPortForShutdown::GetFrameSinkId() const { return frame_sink_id_; } -void WindowPortForShutdown::OnWindowAddedToRootWindow() {} - -void WindowPortForShutdown::OnWillRemoveWindowFromRootWindow() {} - void WindowPortForShutdown::OnEventTargetingPolicyChanged() {} bool WindowPortForShutdown::ShouldRestackTransientChildren() { diff --git a/ui/aura/window_port_for_shutdown.h b/ui/aura/window_port_for_shutdown.h index b38a27a78880e9..f8032626120d34 100644 --- a/ui/aura/window_port_for_shutdown.h +++ b/ui/aura/window_port_for_shutdown.h @@ -42,8 +42,6 @@ class WindowPortForShutdown : public WindowPort { void AllocateLocalSurfaceId() override; const viz::LocalSurfaceId& GetLocalSurfaceId() override; viz::FrameSinkId GetFrameSinkId() const override; - void OnWindowAddedToRootWindow() override; - void OnWillRemoveWindowFromRootWindow() override; void OnEventTargetingPolicyChanged() override; bool ShouldRestackTransientChildren() override; diff --git a/ui/aura/window_unittest.cc b/ui/aura/window_unittest.cc index e1eb51183cd15a..375e2ddc552713 100644 --- a/ui/aura/window_unittest.cc +++ b/ui/aura/window_unittest.cc @@ -433,7 +433,8 @@ TEST_P(WindowTest, WindowEmbeddingClientHasValidLocalSurfaceId) { return; std::unique_ptr window(CreateTestWindow( SK_ColorWHITE, 1, gfx::Rect(10, 10, 300, 200), root_window())); - window->set_embed_frame_sink_id(viz::FrameSinkId(0, 1)); + test::WindowTestApi(window.get()).DisableFrameSinkRegistration(); + window->SetEmbedFrameSinkId(viz::FrameSinkId(0, 1)); EXPECT_TRUE(window->GetLocalSurfaceId().is_valid()); }