Commit 9526fcc7 authored by Fady Samuel's avatar Fady Samuel Committed by Commit Bot

viz: Simplify FrameSinkManager API

FrameSinkManager is responsible for creating and updating the FrameSink
hierarchy to allow propagation of BeginFrames across CompositorFrameSink.
This CL removes some publicly exposed methods on FrameSinkManager that
simply forward operations along to SurfaceManager. This takes us one step
closer toward merging mojom::FrameSinkManager and viz::FrameSinkManager
together.

Bug: 722935
Change-Id: Ibb4b5b174a56e138add1ba44b32919ff9ad4ec52
Reviewed-on: https://chromium-review.googlesource.com/574878
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: default avatarAntoine Labour <piman@chromium.org>
Reviewed-by: default avatarBo Liu <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487295}
parent 79ef8f84
......@@ -33,7 +33,8 @@ HardwareRenderer::HardwareRenderer(RenderThreadManager* state)
last_committed_layer_tree_frame_sink_id_(0u),
last_submitted_layer_tree_frame_sink_id_(0u) {
DCHECK(last_egl_context_);
surfaces_->GetFrameSinkManager()->RegisterFrameSinkId(frame_sink_id_);
surfaces_->GetFrameSinkManager()->surface_manager()->RegisterFrameSinkId(
frame_sink_id_);
CreateNewCompositorFrameSinkSupport();
}
......@@ -43,7 +44,8 @@ HardwareRenderer::~HardwareRenderer() {
if (child_id_.is_valid())
DestroySurface();
support_.reset();
surfaces_->GetFrameSinkManager()->InvalidateFrameSinkId(frame_sink_id_);
surfaces_->GetFrameSinkManager()->surface_manager()->InvalidateFrameSinkId(
frame_sink_id_);
// Reset draw constraints.
render_thread_manager_->PostExternalDrawConstraintsToChildCompositorOnRT(
......
......@@ -53,7 +53,7 @@ CompositorFrameSinkSupport::~CompositorFrameSinkSupport() {
EvictCurrentSurface();
frame_sink_manager_->UnregisterFrameSinkManagerClient(frame_sink_id_);
if (handles_frame_sink_id_invalidation_)
frame_sink_manager_->InvalidateFrameSinkId(frame_sink_id_);
surface_manager_->InvalidateFrameSinkId(frame_sink_id_);
}
void CompositorFrameSinkSupport::SetDestructionCallback(
......@@ -318,7 +318,7 @@ void CompositorFrameSinkSupport::Init(FrameSinkManager* frame_sink_manager) {
frame_sink_manager_ = frame_sink_manager;
surface_manager_ = frame_sink_manager->surface_manager();
if (handles_frame_sink_id_invalidation_)
frame_sink_manager_->RegisterFrameSinkId(frame_sink_id_);
surface_manager_->RegisterFrameSinkId(frame_sink_id_);
frame_sink_manager_->RegisterFrameSinkManagerClient(frame_sink_id_, this);
}
......
......@@ -586,7 +586,8 @@ TEST_F(CompositorFrameSinkSupportTest,
local_surface_id);
local_surface_id_ = LocalSurfaceId();
manager_.RegisterFrameSinkId(kYetAnotherArbitraryFrameSinkId);
manager_.surface_manager()->RegisterFrameSinkId(
kYetAnotherArbitraryFrameSinkId);
SurfaceId surface_id(kAnotherArbitraryFrameSinkId, local_surface_id);
cc::Surface* surface = GetSurfaceForId(surface_id);
......@@ -648,7 +649,7 @@ TEST_F(CompositorFrameSinkSupportTest, InvalidFrameSinkId) {
support_->SubmitCompositorFrame(local_surface_id,
cc::test::MakeCompositorFrame());
manager_.RegisterFrameSinkId(frame_sink_id);
manager_.surface_manager()->RegisterFrameSinkId(frame_sink_id);
GetSurfaceForId(id)->AddDestructionDependency(
SurfaceSequence(frame_sink_id, 4));
......@@ -657,7 +658,7 @@ TEST_F(CompositorFrameSinkSupportTest, InvalidFrameSinkId) {
// Verify the dependency has prevented the surface from getting destroyed.
EXPECT_TRUE(GetSurfaceForId(id));
manager_.InvalidateFrameSinkId(frame_sink_id);
manager_.surface_manager()->InvalidateFrameSinkId(frame_sink_id);
// Verify that the invalidated namespace caused the unsatisfied sequence
// to be ignored.
......@@ -670,7 +671,7 @@ TEST_F(CompositorFrameSinkSupportTest, DestroyCycle) {
auto support2 = CompositorFrameSinkSupport::Create(
&fake_support_client_, &manager_, kYetAnotherArbitraryFrameSinkId,
kIsChildRoot, kHandlesFrameSinkIdInvalidation, kNeedsSyncPoints);
manager_.RegisterFrameSinkId(kAnotherArbitraryFrameSinkId);
manager_.surface_manager()->RegisterFrameSinkId(kAnotherArbitraryFrameSinkId);
// Give local_surface_id_ an initial frame so another client can refer to
// that surface.
{
......
......@@ -73,8 +73,6 @@ class DirectLayerTreeFrameSinkTest : public testing::Test {
display_rect_(display_size_),
support_manager_(&frame_sink_manager_),
context_provider_(cc::TestContextProvider::Create()) {
frame_sink_manager_.RegisterFrameSinkId(kArbitraryFrameSinkId);
auto display_output_surface = cc::FakeOutputSurface::Create3d();
display_output_surface_ = display_output_surface.get();
......
......@@ -34,14 +34,6 @@ FrameSinkManager::~FrameSinkManager() {
DCHECK_EQ(registered_sources_.size(), 0u);
}
void FrameSinkManager::RegisterFrameSinkId(const FrameSinkId& frame_sink_id) {
surface_manager_.RegisterFrameSinkId(frame_sink_id);
}
void FrameSinkManager::InvalidateFrameSinkId(const FrameSinkId& frame_sink_id) {
surface_manager_.InvalidateFrameSinkId(frame_sink_id);
}
void FrameSinkManager::RegisterFrameSinkManagerClient(
const FrameSinkId& frame_sink_id,
FrameSinkManagerClient* client) {
......@@ -237,8 +229,4 @@ void FrameSinkManager::UnregisterFrameSinkHierarchy(
RecursivelyAttachBeginFrameSource(source_iter.second, source_iter.first);
}
void FrameSinkManager::DropTemporaryReference(const SurfaceId& surface_id) {
surface_manager_.DropTemporaryReference(surface_id);
}
} // namespace viz
......@@ -41,12 +41,6 @@ class VIZ_SERVICE_EXPORT FrameSinkManager {
cc::SurfaceManager::LifetimeType::SEQUENCES);
~FrameSinkManager();
void RegisterFrameSinkId(const FrameSinkId& frame_sink_id);
// Invalidate a frame_sink_id that might still have associated sequences,
// possibly because a renderer process has crashed.
void InvalidateFrameSinkId(const FrameSinkId& frame_sink_id);
// CompositorFrameSinkSupport, hierarchy, and BeginFrameSource can be
// registered and unregistered in any order with respect to each other.
//
......@@ -82,11 +76,6 @@ class VIZ_SERVICE_EXPORT FrameSinkManager {
void UnregisterFrameSinkHierarchy(const FrameSinkId& parent_frame_sink_id,
const FrameSinkId& child_frame_sink_id);
// Drops the temporary reference for |surface_id|. If a surface reference has
// already been added from the parent to |surface_id| then this will do
// nothing.
void DropTemporaryReference(const SurfaceId& surface_id);
cc::SurfaceManager* surface_manager() { return &surface_manager_; }
private:
......
......@@ -105,7 +105,7 @@ void FrameSinkManagerImpl::UnregisterFrameSinkHierarchy(
}
void FrameSinkManagerImpl::DropTemporaryReference(const SurfaceId& surface_id) {
manager_.DropTemporaryReference(surface_id);
manager_.surface_manager()->DropTemporaryReference(surface_id);
}
void FrameSinkManagerImpl::DestroyCompositorFrameSink(FrameSinkId sink_id) {
......
......@@ -66,20 +66,9 @@ class FakeFrameSinkManagerClient : public FrameSinkManagerClient {
class FrameSinkManagerTest : public testing::Test {
public:
// These tests don't care about namespace registration, so just preregister
// a set of namespaces that tests can use freely without worrying if they're
// valid or not.
enum { MAX_FRAME_SINK = 10 };
FrameSinkManagerTest() {
for (size_t i = 0; i < MAX_FRAME_SINK; ++i)
manager_.RegisterFrameSinkId(FrameSinkId(i, i));
}
FrameSinkManagerTest() = default;
~FrameSinkManagerTest() override {
for (size_t i = 0; i < MAX_FRAME_SINK; ++i)
manager_.InvalidateFrameSinkId(FrameSinkId(i, i));
}
~FrameSinkManagerTest() override = default;
protected:
FrameSinkManager manager_;
......
......@@ -66,7 +66,8 @@ RenderWidgetHostViewChildFrame::RenderWidgetHostViewChildFrame(
background_color_(SK_ColorWHITE),
weak_factory_(this) {
if (!service_manager::ServiceManagerIsRemote()) {
GetFrameSinkManager()->RegisterFrameSinkId(frame_sink_id_);
GetFrameSinkManager()->surface_manager()->RegisterFrameSinkId(
frame_sink_id_);
CreateCompositorFrameSinkSupport();
}
}
......@@ -74,8 +75,10 @@ RenderWidgetHostViewChildFrame::RenderWidgetHostViewChildFrame(
RenderWidgetHostViewChildFrame::~RenderWidgetHostViewChildFrame() {
if (!service_manager::ServiceManagerIsRemote()) {
ResetCompositorFrameSinkSupport();
if (GetFrameSinkManager())
GetFrameSinkManager()->InvalidateFrameSinkId(frame_sink_id_);
if (GetFrameSinkManager()) {
GetFrameSinkManager()->surface_manager()->InvalidateFrameSinkId(
frame_sink_id_);
}
}
}
......
......@@ -465,7 +465,7 @@ CompositorImpl::CompositorImpl(CompositorClient* client,
num_successive_context_creation_failures_(0),
layer_tree_frame_sink_request_pending_(false),
weak_factory_(this) {
GetFrameSinkManager()->RegisterFrameSinkId(frame_sink_id_);
GetFrameSinkManager()->surface_manager()->RegisterFrameSinkId(frame_sink_id_);
DCHECK(client);
DCHECK(root_window);
DCHECK(root_window->GetLayer() == nullptr);
......@@ -483,7 +483,8 @@ CompositorImpl::~CompositorImpl() {
root_window_->SetLayer(nullptr);
// Clean-up any surface references.
SetSurface(NULL);
GetFrameSinkManager()->InvalidateFrameSinkId(frame_sink_id_);
GetFrameSinkManager()->surface_manager()->InvalidateFrameSinkId(
frame_sink_id_);
}
bool CompositorImpl::IsForSubframe() {
......
......@@ -56,6 +56,7 @@ DelegatedFrameHost::DelegatedFrameHost(const viz::FrameSinkId& frame_sink_id,
factory->GetContextFactory()->AddObserver(this);
factory->GetContextFactoryPrivate()
->GetFrameSinkManager()
->surface_manager()
->RegisterFrameSinkId(frame_sink_id_);
CreateCompositorFrameSinkSupport();
}
......@@ -768,6 +769,7 @@ DelegatedFrameHost::~DelegatedFrameHost() {
factory->GetContextFactoryPrivate()
->GetFrameSinkManager()
->surface_manager()
->InvalidateFrameSinkId(frame_sink_id_);
DCHECK(!vsync_manager_.get());
......
......@@ -69,12 +69,13 @@ TestRenderWidgetHostView::TestRenderWidgetHostView(RenderWidgetHost* rwh)
background_color_(SK_ColorWHITE) {
#if defined(OS_ANDROID)
frame_sink_id_ = AllocateFrameSinkId();
GetFrameSinkManager()->RegisterFrameSinkId(frame_sink_id_);
GetFrameSinkManager()->surface_manager()->RegisterFrameSinkId(frame_sink_id_);
#else
// Not all tests initialize or need an image transport factory.
if (ImageTransportFactory::GetInstance()) {
frame_sink_id_ = AllocateFrameSinkId();
GetFrameSinkManager()->RegisterFrameSinkId(frame_sink_id_);
GetFrameSinkManager()->surface_manager()->RegisterFrameSinkId(
frame_sink_id_);
}
#endif
......@@ -91,7 +92,7 @@ TestRenderWidgetHostView::TestRenderWidgetHostView(RenderWidgetHost* rwh)
TestRenderWidgetHostView::~TestRenderWidgetHostView() {
viz::FrameSinkManager* manager = GetFrameSinkManager();
if (manager) {
manager->InvalidateFrameSinkId(frame_sink_id_);
manager->surface_manager()->InvalidateFrameSinkId(frame_sink_id_);
}
}
......
......@@ -65,7 +65,7 @@ DelegatedFrameHostAndroid::DelegatedFrameHostAndroid(
DCHECK(view_);
DCHECK(client_);
frame_sink_manager_->RegisterFrameSinkId(frame_sink_id_);
frame_sink_manager_->surface_manager()->RegisterFrameSinkId(frame_sink_id_);
CreateNewCompositorFrameSinkSupport();
}
......@@ -73,7 +73,7 @@ DelegatedFrameHostAndroid::~DelegatedFrameHostAndroid() {
DestroyDelegatedContent();
DetachFromCompositor();
support_.reset();
frame_sink_manager_->InvalidateFrameSinkId(frame_sink_id_);
frame_sink_manager_->surface_manager()->InvalidateFrameSinkId(frame_sink_id_);
}
void DelegatedFrameHostAndroid::SubmitCompositorFrame(
......
......@@ -69,8 +69,9 @@ Compositor::Compositor(const viz::FrameSinkId& frame_sink_id,
weak_ptr_factory_(this),
lock_timeout_weak_ptr_factory_(this) {
if (context_factory_private) {
context_factory_private->GetFrameSinkManager()->RegisterFrameSinkId(
frame_sink_id_);
context_factory_private->GetFrameSinkManager()
->surface_manager()
->RegisterFrameSinkId(frame_sink_id_);
}
root_web_layer_ = cc::Layer::Create();
......@@ -217,7 +218,7 @@ Compositor::~Compositor() {
DCHECK(client.is_valid());
manager->UnregisterFrameSinkHierarchy(frame_sink_id_, client);
}
manager->InvalidateFrameSinkId(frame_sink_id_);
manager->surface_manager()->InvalidateFrameSinkId(frame_sink_id_);
}
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment