diff options
42 files changed, 785 insertions, 606 deletions
diff --git a/android_webview/browser/hardware_renderer.cc b/android_webview/browser/hardware_renderer.cc index 5688b1c..6cb87d8 100644 --- a/android_webview/browser/hardware_renderer.cc +++ b/android_webview/browser/hardware_renderer.cc @@ -53,6 +53,9 @@ HardwareRenderer::HardwareRenderer(SharedRendererState* state) surface_manager_.reset(new cc::SurfaceManager); surface_id_allocator_.reset(new cc::SurfaceIdAllocator(1)); + surface_id_allocator_->RegisterSurfaceIdNamespace(surface_manager_.get()); + surface_manager_->RegisterSurfaceFactoryClient( + surface_id_allocator_->id_namespace(), this); display_.reset(new cc::Display(this, surface_manager_.get(), nullptr, nullptr, settings)); } @@ -66,6 +69,8 @@ HardwareRenderer::~HardwareRenderer() { surface_factory_->Destroy(child_id_); display_.reset(); surface_factory_.reset(); + surface_manager_->UnregisterSurfaceFactoryClient( + surface_id_allocator_->id_namespace()); // Reset draw constraints. shared_renderer_state_->PostExternalDrawConstraintsToChildCompositorOnRT( @@ -222,7 +227,6 @@ void HardwareRenderer::ReturnResources( } void HardwareRenderer::SetBeginFrameSource( - cc::SurfaceId surface_id, cc::BeginFrameSource* begin_frame_source) { // TODO(tansell): Hook this up. } diff --git a/android_webview/browser/hardware_renderer.h b/android_webview/browser/hardware_renderer.h index eb1ccc9..a0d3d44 100644 --- a/android_webview/browser/hardware_renderer.h +++ b/android_webview/browser/hardware_renderer.h @@ -48,8 +48,7 @@ class HardwareRenderer : public cc::DisplayClient, // cc::SurfaceFactoryClient implementation. void ReturnResources(const cc::ReturnedResourceArray& resources) override; - void SetBeginFrameSource(cc::SurfaceId surface_id, - cc::BeginFrameSource* begin_frame_source) override; + void SetBeginFrameSource(cc::BeginFrameSource* begin_frame_source) override; void ReturnResourcesInChildFrame(); void ReturnResourcesToCompositor(const cc::ReturnedResourceArray& resources, diff --git a/cc/BUILD.gn b/cc/BUILD.gn index 2ed2554..68b4e06 100644 --- a/cc/BUILD.gn +++ b/cc/BUILD.gn @@ -915,6 +915,7 @@ test("cc_unittests") { "surfaces/surface_display_output_surface_unittest.cc", "surfaces/surface_factory_unittest.cc", "surfaces/surface_hittest_unittest.cc", + "surfaces/surface_manager_unittest.cc", "surfaces/surface_unittest.cc", "surfaces/surfaces_pixeltest.cc", diff --git a/cc/cc_tests.gyp b/cc/cc_tests.gyp index dd84cd7..ba0279e 100644 --- a/cc/cc_tests.gyp +++ b/cc/cc_tests.gyp @@ -165,6 +165,7 @@ 'surfaces/surface_display_output_surface_unittest.cc', 'surfaces/surface_factory_unittest.cc', 'surfaces/surface_hittest_unittest.cc', + 'surfaces/surface_manager_unittest.cc', 'surfaces/surface_unittest.cc', 'surfaces/surfaces_pixeltest.cc', ], diff --git a/cc/surfaces/display.cc b/cc/surfaces/display.cc index b8ba195..6866ced 100644 --- a/cc/surfaces/display.cc +++ b/cc/surfaces/display.cc @@ -56,6 +56,7 @@ Display::~Display() { bool Display::Initialize(scoped_ptr<OutputSurface> output_surface, DisplayScheduler* scheduler) { + // TODO(enne): register/unregister BeginFrameSource with SurfaceManager here. output_surface_ = std::move(output_surface); scheduler_ = scheduler; return output_surface_->BindToClient(this); @@ -134,8 +135,8 @@ void Display::InitializeRenderer() { // overlays. bool output_partial_list = renderer_->Capabilities().using_partial_swap && !output_surface_->GetOverlayCandidateValidator(); - aggregator_.reset(new SurfaceAggregator( - this, manager_, resource_provider_.get(), output_partial_list)); + aggregator_.reset(new SurfaceAggregator(manager_, resource_provider_.get(), + output_partial_list)); } void Display::DidLoseOutputSurface() { @@ -153,31 +154,6 @@ void Display::UpdateRootSurfaceResourcesLocked() { scheduler_->SetRootSurfaceResourcesLocked(root_surface_resources_locked); } -void Display::AddSurface(Surface* surface) { - // Checking for the output_surface ensures Display::Initialize has been - // called and that scheduler_ won't change its value. - DCHECK(output_surface_); - - // WebView's HardwareRenderer will never have a scheduler. - if (!scheduler_) - return; - - surface->AddBeginFrameSource(scheduler_->begin_frame_source_for_children()); -} - -void Display::RemoveSurface(Surface* surface) { - // Checking for the output_surface ensures Display::Initialize has been - // called and that scheduler_ won't change its value. - DCHECK(output_surface_); - - // WebView's HardwareRenderer will never have a scheduler. - if (!scheduler_) - return; - - surface->RemoveBeginFrameSource( - scheduler_->begin_frame_source_for_children()); -} - bool Display::DrawAndSwap() { TRACE_EVENT0("cc", "Display::DrawAndSwap"); diff --git a/cc/surfaces/display.h b/cc/surfaces/display.h index 6c29dc8..4646179 100644 --- a/cc/surfaces/display.h +++ b/cc/surfaces/display.h @@ -49,7 +49,6 @@ class TextureMailboxDeleter; class CC_SURFACES_EXPORT Display : public DisplaySchedulerClient, public OutputSurfaceClient, public RendererClient, - public SurfaceAggregatorClient, public SurfaceDamageObserver { public: Display(DisplayClient* client, @@ -70,10 +69,6 @@ class CC_SURFACES_EXPORT Display : public DisplaySchedulerClient, SurfaceId CurrentSurfaceId(); - // SurfaceAggregatorClient implementation - void AddSurface(Surface* surface) override; - void RemoveSurface(Surface* surface) override; - // DisplaySchedulerClient implementation. bool DrawAndSwap() override; diff --git a/cc/surfaces/display_unittest.cc b/cc/surfaces/display_unittest.cc index 560e5db..cbc7e34 100644 --- a/cc/surfaces/display_unittest.cc +++ b/cc/surfaces/display_unittest.cc @@ -35,8 +35,7 @@ class FakeSurfaceFactoryClient : public SurfaceFactoryClient { void ReturnResources(const ReturnedResourceArray& resources) override {} - void SetBeginFrameSource(SurfaceId surface_id, - BeginFrameSource* begin_frame_source) override { + void SetBeginFrameSource(BeginFrameSource* begin_frame_source) override { begin_frame_source_ = begin_frame_source; } @@ -154,29 +153,6 @@ void CopyCallback(bool* called, scoped_ptr<CopyOutputResult> result) { *called = true; } -// Verify Display responds to SurfaceAggregatorClient methods properly. -TEST_F(DisplayTest, DisplayAsSurfaceAggregatorClient) { - SetUpContext(nullptr); - TestDisplayClient client; - RendererSettings settings; - Display display(&client, &manager_, shared_bitmap_manager_.get(), nullptr, - settings); - - TestDisplayScheduler scheduler(&display, &fake_begin_frame_source_, - task_runner_.get()); - display.Initialize(std::move(output_surface_), &scheduler); - - SurfaceId surface_id(6); - factory_.Create(surface_id); - Surface* surface = manager_.GetSurfaceForId(surface_id); - - EXPECT_EQ(nullptr, surface_factory_client_.begin_frame_source()); - display.AddSurface(surface); - EXPECT_NE(nullptr, surface_factory_client_.begin_frame_source()); - display.RemoveSurface(surface); - EXPECT_EQ(nullptr, surface_factory_client_.begin_frame_source()); -} - // Check that frame is damaged and swapped only under correct conditions. TEST_F(DisplayTest, DisplayDamaged) { SetUpContext(nullptr); diff --git a/cc/surfaces/surface.cc b/cc/surfaces/surface.cc index 6a96230..281a690 100644 --- a/cc/surfaces/surface.cc +++ b/cc/surfaces/surface.cc @@ -39,9 +39,6 @@ Surface::~Surface() { } if (!draw_callback_.is_null()) draw_callback_.Run(SurfaceDrawStatus::DRAW_SKIPPED); - - if (factory_) - factory_->SetBeginFrameSource(surface_id_, NULL); } void Surface::QueueFrame(scoped_ptr<CompositorFrame> frame, @@ -178,34 +175,6 @@ void Surface::SatisfyDestructionDependencies( destruction_dependencies_.end()); } -void Surface::AddBeginFrameSource(BeginFrameSource* begin_frame_source) { - DCHECK(base::STLIsSorted(begin_frame_sources_)); - DCHECK(!ContainsValue(begin_frame_sources_, begin_frame_source)) - << begin_frame_source; - begin_frame_sources_.insert(begin_frame_source); - UpdatePrimaryBeginFrameSource(); -} - -void Surface::RemoveBeginFrameSource(BeginFrameSource* begin_frame_source) { - size_t erase_count = begin_frame_sources_.erase(begin_frame_source); - DCHECK_EQ(1u, erase_count); - UpdatePrimaryBeginFrameSource(); -} - -void Surface::UpdatePrimaryBeginFrameSource() { - // Ensure the BeginFrameSources are sorted so our we make a stable decision - // regarding which source is primary. - // TODO(brianderson): Do something smarter based on coverage instead. - DCHECK(base::STLIsSorted(begin_frame_sources_)); - - BeginFrameSource* primary_source = nullptr; - if (!begin_frame_sources_.empty()) - primary_source = *begin_frame_sources_.begin(); - - if (factory_) - factory_->SetBeginFrameSource(surface_id_, primary_source); -} - void Surface::ClearCopyRequests() { if (current_frame_) { for (const auto& render_pass : diff --git a/cc/surfaces/surface.h b/cc/surfaces/surface.h index 97706eb..340db19 100644 --- a/cc/surfaces/surface.h +++ b/cc/surfaces/surface.h @@ -84,12 +84,8 @@ class CC_SURFACES_EXPORT Surface { bool destroyed() const { return destroyed_; } void set_destroyed(bool destroyed) { destroyed_ = destroyed; } - void AddBeginFrameSource(BeginFrameSource* begin_frame_source); - void RemoveBeginFrameSource(BeginFrameSource* begin_frame_source); - private: void ClearCopyRequests(); - void UpdatePrimaryBeginFrameSource(); SurfaceId surface_id_; base::WeakPtr<SurfaceFactory> factory_; diff --git a/cc/surfaces/surface_aggregator.cc b/cc/surfaces/surface_aggregator.cc index 9115b74..4c7e04e 100644 --- a/cc/surfaces/surface_aggregator.cc +++ b/cc/surfaces/surface_aggregator.cc @@ -42,12 +42,10 @@ void MoveMatchingRequests( } // namespace -SurfaceAggregator::SurfaceAggregator(SurfaceAggregatorClient* client, - SurfaceManager* manager, +SurfaceAggregator::SurfaceAggregator(SurfaceManager* manager, ResourceProvider* provider, bool aggregate_only_damaged) - : client_(client), - manager_(manager), + : manager_(manager), provider_(provider), next_render_pass_id_(1), aggregate_only_damaged_(aggregate_only_damaged), @@ -479,19 +477,9 @@ void SurfaceAggregator::ProcessAddedAndRemovedSurfaces() { Surface* surface_ptr = manager_->GetSurfaceForId(surface.first); if (surface_ptr) { surface_ptr->RunDrawCallbacks(SurfaceDrawStatus::DRAW_SKIPPED); - client_->RemoveSurface(surface_ptr); } } } - - for (const auto& surface : contained_surfaces_) { - if (!previous_contained_surfaces_.count(surface.first)) { - // Notify client of added surface. - Surface* surface_ptr = manager_->GetSurfaceForId(surface.first); - if (surface_ptr) - client_->AddSurface(surface_ptr); - } - } } // Walk the Surface tree from surface_id. Validate the resources of the current diff --git a/cc/surfaces/surface_aggregator.h b/cc/surfaces/surface_aggregator.h index c18da1b..9360ed9 100644 --- a/cc/surfaces/surface_aggregator.h +++ b/cc/surfaces/surface_aggregator.h @@ -27,20 +27,11 @@ class Surface; class SurfaceDrawQuad; class SurfaceManager; -class CC_SURFACES_EXPORT SurfaceAggregatorClient { - public: - virtual ~SurfaceAggregatorClient() {} - - virtual void AddSurface(Surface* surface) = 0; - virtual void RemoveSurface(Surface* surface) = 0; -}; - class CC_SURFACES_EXPORT SurfaceAggregator { public: using SurfaceIndexMap = std::unordered_map<SurfaceId, int, SurfaceIdHash>; - SurfaceAggregator(SurfaceAggregatorClient* client, - SurfaceManager* manager, + SurfaceAggregator(SurfaceManager* manager, ResourceProvider* provider, bool aggregate_only_damaged); ~SurfaceAggregator(); @@ -109,7 +100,6 @@ class CC_SURFACES_EXPORT SurfaceAggregator { const RenderPass& source, const gfx::Rect& full_rect) const; - SurfaceAggregatorClient* client_; // Outlives this class. SurfaceManager* manager_; ResourceProvider* provider_; diff --git a/cc/surfaces/surface_aggregator_perftest.cc b/cc/surfaces/surface_aggregator_perftest.cc index c6e4234..b79bd88 100644 --- a/cc/surfaces/surface_aggregator_perftest.cc +++ b/cc/surfaces/surface_aggregator_perftest.cc @@ -24,14 +24,7 @@ namespace { class EmptySurfaceFactoryClient : public SurfaceFactoryClient { public: void ReturnResources(const ReturnedResourceArray& resources) override {} - void SetBeginFrameSource(SurfaceId surface_id, - BeginFrameSource* begin_frame_source) override {} -}; - -class EmptySurfaceAggregatorClient : public SurfaceAggregatorClient { - public: - void AddSurface(Surface* surface) override {} - void RemoveSurface(Surface* surface) override {} + void SetBeginFrameSource(BeginFrameSource* begin_frame_source) override {} }; class SurfaceAggregatorPerfTest : public testing::Test { @@ -52,8 +45,7 @@ class SurfaceAggregatorPerfTest : public testing::Test { bool optimize_damage, bool full_damage, const std::string& name) { - aggregator_.reset(new SurfaceAggregator(&surface_aggregator_client_, - &manager_, resource_provider_.get(), + aggregator_.reset(new SurfaceAggregator(&manager_, resource_provider_.get(), optimize_damage)); for (int i = 1; i <= num_surfaces; i++) { factory_.Create(SurfaceId(i)); @@ -149,7 +141,6 @@ class SurfaceAggregatorPerfTest : public testing::Test { scoped_ptr<SharedBitmapManager> shared_bitmap_manager_; scoped_ptr<ResourceProvider> resource_provider_; scoped_ptr<SurfaceAggregator> aggregator_; - EmptySurfaceAggregatorClient surface_aggregator_client_; LapTimer timer_; }; diff --git a/cc/surfaces/surface_aggregator_unittest.cc b/cc/surfaces/surface_aggregator_unittest.cc index 95f0261..dca531d 100644 --- a/cc/surfaces/surface_aggregator_unittest.cc +++ b/cc/surfaces/surface_aggregator_unittest.cc @@ -56,41 +56,17 @@ class EmptySurfaceFactoryClient : public SurfaceFactoryClient { last_damage_rect_ = damage_rect; } - void SetBeginFrameSource(SurfaceId surface_id, - BeginFrameSource* begin_frame_source) override {} + void SetBeginFrameSource(BeginFrameSource* begin_frame_source) override {} gfx::Rect last_damage_rect_; SurfaceId last_surface_id_; }; -class FakeSurfaceAggregatorClient : public SurfaceAggregatorClient { - public: - void AddSurface(Surface* surface) override { - EXPECT_FALSE(HasSurface(surface)); - surfaces_.insert(surface); - } - - void RemoveSurface(Surface* surface) override { - EXPECT_TRUE(HasSurface(surface)); - surfaces_.erase(surface); - } - - bool HasSurface(Surface* surface) const { - return surfaces_.count(surface) != 0; - } - - private: - std::set<Surface*> surfaces_; -}; - class SurfaceAggregatorTest : public testing::Test { public: explicit SurfaceAggregatorTest(bool use_damage_rect) : factory_(&manager_, &empty_client_), - aggregator_(&surface_aggregator_client_, - &manager_, - NULL, - use_damage_rect) {} + aggregator_(&manager_, NULL, use_damage_rect) {} SurfaceAggregatorTest() : SurfaceAggregatorTest(false) {} @@ -98,19 +74,15 @@ class SurfaceAggregatorTest : public testing::Test { SurfaceManager manager_; EmptySurfaceFactoryClient empty_client_; SurfaceFactory factory_; - FakeSurfaceAggregatorClient surface_aggregator_client_; SurfaceAggregator aggregator_; }; TEST_F(SurfaceAggregatorTest, ValidSurfaceNoFrame) { SurfaceId one_id(7); factory_.Create(one_id); - Surface* surface = manager_.GetSurfaceForId(one_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(surface)); scoped_ptr<CompositorFrame> frame = aggregator_.Aggregate(one_id); EXPECT_FALSE(frame); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(surface)); factory_.Destroy(one_id); } @@ -215,9 +187,7 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, SimpleFrame) { SurfaceId ids[] = {root_surface_id_}; - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); AggregateAndVerify(passes, arraysize(passes), ids, arraysize(ids)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); // Check that WillDrawSurface was called. EXPECT_EQ(gfx::Rect(SurfaceSize()), empty_client_.last_damage_rect_); @@ -227,8 +197,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, SimpleFrame) { TEST_F(SurfaceAggregatorValidSurfaceTest, OpacityCopied) { SurfaceId embedded_surface_id = allocator_.GenerateId(); factory_.Create(embedded_surface_id); - Surface* embedded_surface = manager_.GetSurfaceForId(embedded_surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(embedded_surface)); test::Quad embedded_quads[] = {test::Quad::SolidColorQuad(SK_ColorGREEN), test::Quad::SolidColorQuad(SK_ColorBLUE)}; @@ -243,15 +211,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, OpacityCopied) { SubmitCompositorFrame(passes, arraysize(passes), root_surface_id_); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(embedded_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(embedded_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -286,9 +248,7 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, MultiPassSimpleFrame) { SurfaceId ids[] = {root_surface_id_}; - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); AggregateAndVerify(passes, arraysize(passes), ids, arraysize(ids)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); } // This tests very simple embedding. root_surface has a frame containing a few @@ -298,8 +258,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, MultiPassSimpleFrame) { TEST_F(SurfaceAggregatorValidSurfaceTest, SimpleSurfaceReference) { SurfaceId embedded_surface_id = allocator_.GenerateId(); factory_.Create(embedded_surface_id); - Surface* embedded_surface = manager_.GetSurfaceForId(embedded_surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(embedded_surface)); test::Quad embedded_quads[] = {test::Quad::SolidColorQuad(SK_ColorGREEN)}; test::Pass embedded_passes[] = { @@ -315,9 +273,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, SimpleSurfaceReference) { SubmitCompositorFrame(root_passes, arraysize(root_passes), root_surface_id_); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(embedded_surface)); - test::Quad expected_quads[] = {test::Quad::SolidColorQuad(SK_ColorWHITE), test::Quad::SolidColorQuad(SK_ColorGREEN), test::Quad::SolidColorQuad(SK_ColorBLACK)}; @@ -327,17 +282,12 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, SimpleSurfaceReference) { AggregateAndVerify( expected_passes, arraysize(expected_passes), ids, arraysize(ids)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(embedded_surface)); - factory_.Destroy(embedded_surface_id); } TEST_F(SurfaceAggregatorValidSurfaceTest, CopyRequest) { SurfaceId embedded_surface_id = allocator_.GenerateId(); factory_.Create(embedded_surface_id); - Surface* embedded_surface = manager_.GetSurfaceForId(embedded_surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(embedded_surface)); test::Quad embedded_quads[] = {test::Quad::SolidColorQuad(SK_ColorGREEN)}; test::Pass embedded_passes[] = { @@ -357,15 +307,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, CopyRequest) { SubmitCompositorFrame(root_passes, arraysize(root_passes), root_surface_id_); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(embedded_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(embedded_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -402,8 +346,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, CopyRequest) { TEST_F(SurfaceAggregatorValidSurfaceTest, RootCopyRequest) { SurfaceId embedded_surface_id = allocator_.GenerateId(); factory_.Create(embedded_surface_id); - Surface* embedded_surface = manager_.GetSurfaceForId(embedded_surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(embedded_surface)); test::Quad embedded_quads[] = {test::Quad::SolidColorQuad(SK_ColorGREEN)}; test::Pass embedded_passes[] = { @@ -444,15 +386,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, RootCopyRequest) { SurfaceFactory::DrawCallback()); } - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(embedded_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(embedded_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -500,8 +436,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, UnreferencedSurface) { SurfaceId embedded_surface_id = allocator_.GenerateId(); SurfaceId nonexistent_surface_id = allocator_.GenerateId(); factory_.Create(embedded_surface_id); - Surface* embedded_surface = manager_.GetSurfaceForId(embedded_surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(embedded_surface)); test::Quad embedded_quads[] = {test::Quad::SolidColorQuad(SK_ColorGREEN)}; test::Pass embedded_passes[] = { @@ -516,7 +450,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, UnreferencedSurface) { SurfaceId parent_surface_id = allocator_.GenerateId(); factory_.Create(parent_surface_id); - Surface* parent_surface = manager_.GetSurfaceForId(parent_surface_id); test::Quad parent_quads[] = { test::Quad::SolidColorQuad(SK_ColorWHITE), @@ -558,17 +491,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, UnreferencedSurface) { SurfaceFactory::DrawCallback()); } - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(parent_surface)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(embedded_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(parent_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(embedded_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -606,8 +531,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, UnreferencedSurface) { TEST_F(SurfaceAggregatorValidSurfaceTest, MultiPassSurfaceReference) { SurfaceId embedded_surface_id = child_allocator_.GenerateId(); factory_.Create(embedded_surface_id); - Surface* embedded_surface = manager_.GetSurfaceForId(embedded_surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(embedded_surface)); RenderPassId pass_ids[] = {RenderPassId(1, 1), RenderPassId(1, 2), RenderPassId(1, 3)}; @@ -636,15 +559,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, MultiPassSurfaceReference) { SubmitCompositorFrame(root_passes, arraysize(root_passes), root_surface_id_); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(embedded_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(embedded_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -766,10 +683,8 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, InvalidSurfaceReference) { test::Pass(expected_quads, arraysize(expected_quads))}; SurfaceId ids[] = {root_surface_id_, InvalidSurfaceId()}; - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); AggregateAndVerify( expected_passes, arraysize(expected_passes), ids, arraysize(ids)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); } // Tests a reference to a valid surface with no submitted frame. This quad @@ -777,8 +692,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, InvalidSurfaceReference) { TEST_F(SurfaceAggregatorValidSurfaceTest, ValidSurfaceReferenceWithNoFrame) { SurfaceId surface_with_no_frame_id = allocator_.GenerateId(); factory_.Create(surface_with_no_frame_id); - Surface* surface_with_no_frame = - manager_.GetSurfaceForId(surface_with_no_frame_id); test::Quad quads[] = {test::Quad::SolidColorQuad(SK_ColorGREEN), test::Quad::SurfaceQuad(surface_with_no_frame_id, 1.f), @@ -792,12 +705,8 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, ValidSurfaceReferenceWithNoFrame) { test::Pass expected_passes[] = { test::Pass(expected_quads, arraysize(expected_quads))}; SurfaceId ids[] = {root_surface_id_, surface_with_no_frame_id}; - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(surface_with_no_frame)); AggregateAndVerify( expected_passes, arraysize(expected_passes), ids, arraysize(ids)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(surface_with_no_frame)); factory_.Destroy(surface_with_no_frame_id); } @@ -814,18 +723,14 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, SimpleCyclicalReference) { test::Pass expected_passes[] = { test::Pass(expected_quads, arraysize(expected_quads))}; SurfaceId ids[] = {root_surface_id_}; - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); AggregateAndVerify( expected_passes, arraysize(expected_passes), ids, arraysize(ids)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); } // Tests a more complex cycle with one intermediate surface. TEST_F(SurfaceAggregatorValidSurfaceTest, TwoSurfaceCyclicalReference) { SurfaceId child_surface_id = allocator_.GenerateId(); factory_.Create(child_surface_id); - Surface* child_surface = manager_.GetSurfaceForId(child_surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(child_surface)); test::Quad parent_quads[] = {test::Quad::SolidColorQuad(SK_ColorBLUE), test::Quad::SurfaceQuad(child_surface_id, 1.f), @@ -857,12 +762,8 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, TwoSurfaceCyclicalReference) { test::Pass expected_passes[] = { test::Pass(expected_quads, arraysize(expected_quads))}; SurfaceId ids[] = {root_surface_id_, child_surface_id}; - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(child_surface)); AggregateAndVerify( expected_passes, arraysize(expected_passes), ids, arraysize(ids)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); factory_.Destroy(child_surface_id); } @@ -871,8 +772,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, TwoSurfaceCyclicalReference) { TEST_F(SurfaceAggregatorValidSurfaceTest, RenderPassIdMapping) { SurfaceId child_surface_id = allocator_.GenerateId(); factory_.Create(child_surface_id); - Surface* child_surface = manager_.GetSurfaceForId(child_surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(child_surface)); RenderPassId child_pass_id[] = {RenderPassId(1, 1), RenderPassId(1, 2)}; test::Quad child_quad[][1] = {{test::Quad::SolidColorQuad(SK_ColorGREEN)}, @@ -896,15 +795,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, RenderPassIdMapping) { SubmitCompositorFrame(parent_passes, arraysize(parent_passes), root_surface_id_); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(child_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -1002,8 +895,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateSharedQuadStateProperties) { RenderPassId pass_id(1, 1); SurfaceId grandchild_surface_id = allocator_.GenerateId(); factory_.Create(grandchild_surface_id); - Surface* grandchild_surface = manager_.GetSurfaceForId(grandchild_surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(grandchild_surface)); scoped_ptr<RenderPass> grandchild_pass = RenderPass::Create(); gfx::Rect output_rect(SurfaceSize()); gfx::Rect damage_rect(SurfaceSize()); @@ -1016,8 +907,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateSharedQuadStateProperties) { SurfaceId child_one_surface_id = allocator_.GenerateId(); factory_.Create(child_one_surface_id); - Surface* child_one_surface = manager_.GetSurfaceForId(child_one_surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(child_one_surface)); scoped_ptr<RenderPass> child_one_pass = RenderPass::Create(); child_one_pass->SetNew( @@ -1036,8 +925,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateSharedQuadStateProperties) { SurfaceId child_two_surface_id = allocator_.GenerateId(); factory_.Create(child_two_surface_id); - Surface* child_two_surface = manager_.GetSurfaceForId(child_two_surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(child_two_surface)); scoped_ptr<RenderPass> child_two_pass = RenderPass::Create(); child_two_pass->SetNew( @@ -1071,19 +958,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateSharedQuadStateProperties) { QueuePassAsFrame(std::move(root_pass), root_surface_id_); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(grandchild_surface)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(child_one_surface)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(child_two_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(grandchild_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_one_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_two_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -1130,8 +1007,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateMultiplePassWithTransform) { // Innermost child surface. SurfaceId child_surface_id = allocator_.GenerateId(); factory_.Create(child_surface_id); - Surface* child_surface = manager_.GetSurfaceForId(child_surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(child_surface)); { RenderPassId child_pass_id[] = {RenderPassId(1, 1), RenderPassId(1, 2)}; test::Quad child_quads[][1] = { @@ -1173,8 +1048,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateMultiplePassWithTransform) { // Middle child surface. SurfaceId middle_surface_id = allocator_.GenerateId(); factory_.Create(middle_surface_id); - Surface* middle_surface = manager_.GetSurfaceForId(middle_surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(middle_surface)); { test::Quad middle_quads[] = { test::Quad::SurfaceQuad(child_surface_id, 1.f)}; @@ -1238,17 +1111,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateMultiplePassWithTransform) { factory_.SubmitCompositorFrame(root_surface_id_, std::move(root_frame), SurfaceFactory::DrawCallback()); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(child_surface)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(middle_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(middle_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -1352,7 +1217,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateDamageRect) { SurfaceId child_surface_id = allocator_.GenerateId(); factory_.Create(child_surface_id); - Surface* child_surface = manager_.GetSurfaceForId(child_surface_id); factory_.SubmitCompositorFrame(child_surface_id, std::move(child_frame), SurfaceFactory::DrawCallback()); @@ -1380,7 +1244,6 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateDamageRect) { SurfaceId parent_surface_id = allocator_.GenerateId(); factory_.Create(parent_surface_id); - Surface* parent_surface = manager_.GetSurfaceForId(parent_surface_id); factory_.SubmitCompositorFrame(parent_surface_id, std::move(parent_surface_frame), SurfaceFactory::DrawCallback()); @@ -1417,17 +1280,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateDamageRect) { factory_.SubmitCompositorFrame(root_surface_id_, std::move(root_frame), SurfaceFactory::DrawCallback()); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(child_surface)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(parent_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(parent_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -1462,17 +1317,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateDamageRect) { factory_.SubmitCompositorFrame(child_surface_id, std::move(child_frame), SurfaceFactory::DrawCallback()); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(parent_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(parent_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -1532,17 +1379,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateDamageRect) { factory_.SubmitCompositorFrame(root_surface_id_, std::move(root_frame), SurfaceFactory::DrawCallback()); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(parent_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(parent_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -1561,17 +1400,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateDamageRect) { // No Surface changed, so no damage should be given. { - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(parent_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(parent_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -1588,18 +1419,10 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateDamageRect) { // SetFullDamageRectForSurface should cause the entire output to be // marked as damaged. { - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(parent_surface)); - aggregator_.SetFullDamageForSurface(root_surface_id_); scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(parent_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -1628,7 +1451,6 @@ class SurfaceAggregatorPartialSwapTest TEST_F(SurfaceAggregatorPartialSwapTest, IgnoreOutside) { SurfaceId child_surface_id = allocator_.GenerateId(); factory_.Create(child_surface_id); - Surface* child_surface = manager_.GetSurfaceForId(child_surface_id); // The child surface has two quads, one with a visible rect of 13,13 4x4 and // the other other with a visible rect of 10,10 2x2 (relative to root target // space). @@ -1674,15 +1496,9 @@ TEST_F(SurfaceAggregatorPartialSwapTest, IgnoreOutside) { SubmitPassListAsFrame(root_surface_id_, &root_pass_list); } - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(child_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -1715,15 +1531,9 @@ TEST_F(SurfaceAggregatorPartialSwapTest, IgnoreOutside) { } { - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -1776,15 +1586,9 @@ TEST_F(SurfaceAggregatorPartialSwapTest, IgnoreOutside) { } { - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -1806,15 +1610,9 @@ TEST_F(SurfaceAggregatorPartialSwapTest, IgnoreOutside) { } { - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - scoped_ptr<CompositorFrame> aggregated_frame = aggregator_.Aggregate(root_surface_id_); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - ASSERT_TRUE(aggregated_frame); ASSERT_TRUE(aggregated_frame->delegated_frame_data); @@ -1843,9 +1641,8 @@ class SurfaceAggregatorWithResourcesTest : public testing::Test { resource_provider_ = FakeResourceProvider::Create( output_surface_.get(), shared_bitmap_manager_.get()); - aggregator_.reset(new SurfaceAggregator(&surface_aggregator_client_, - &manager_, resource_provider_.get(), - false)); + aggregator_.reset( + new SurfaceAggregator(&manager_, resource_provider_.get(), false)); } protected: @@ -1855,7 +1652,6 @@ class SurfaceAggregatorWithResourcesTest : public testing::Test { scoped_ptr<SharedBitmapManager> shared_bitmap_manager_; scoped_ptr<ResourceProvider> resource_provider_; scoped_ptr<SurfaceAggregator> aggregator_; - FakeSurfaceAggregatorClient surface_aggregator_client_; }; class ResourceTrackingSurfaceFactoryClient : public SurfaceFactoryClient { @@ -1871,8 +1667,7 @@ class ResourceTrackingSurfaceFactoryClient : public SurfaceFactoryClient { return returned_resources_; } - void SetBeginFrameSource(SurfaceId surface_id, - BeginFrameSource* begin_frame_source) override {} + void SetBeginFrameSource(BeginFrameSource* begin_frame_source) override {} private: ReturnedResourceArray returned_resources_; @@ -1933,30 +1728,21 @@ TEST_F(SurfaceAggregatorWithResourcesTest, TakeResourcesOneSurface) { SurfaceFactory factory(&manager_, &client); SurfaceId surface_id(7u); factory.Create(surface_id); - Surface* surface = manager_.GetSurfaceForId(surface_id); ResourceId ids[] = {11, 12, 13}; SubmitCompositorFrameWithResources(ids, arraysize(ids), true, SurfaceId(), &factory, surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(surface)); - scoped_ptr<CompositorFrame> frame = aggregator_->Aggregate(surface_id); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(surface)); - // Nothing should be available to be returned yet. EXPECT_TRUE(client.returned_resources().empty()); SubmitCompositorFrameWithResources(NULL, 0u, true, SurfaceId(), &factory, surface_id); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(surface)); - frame = aggregator_->Aggregate(surface_id); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(surface)); - ASSERT_EQ(3u, client.returned_resources().size()); ResourceId returned_ids[3]; for (size_t i = 0; i < 3; ++i) { @@ -1972,7 +1758,6 @@ TEST_F(SurfaceAggregatorWithResourcesTest, TakeInvalidResources) { SurfaceFactory factory(&manager_, &client); SurfaceId surface_id(7u); factory.Create(surface_id); - Surface* surface = manager_.GetSurfaceForId(surface_id); scoped_ptr<DelegatedFrameData> frame_data(new DelegatedFrameData); scoped_ptr<RenderPass> pass = RenderPass::Create(); @@ -1989,13 +1774,9 @@ TEST_F(SurfaceAggregatorWithResourcesTest, TakeInvalidResources) { factory.SubmitCompositorFrame(surface_id, std::move(frame), SurfaceFactory::DrawCallback()); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(surface)); - scoped_ptr<CompositorFrame> returned_frame = aggregator_->Aggregate(surface_id); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(surface)); - // Nothing should be available to be returned yet. EXPECT_TRUE(client.returned_resources().empty()); @@ -2012,11 +1793,9 @@ TEST_F(SurfaceAggregatorWithResourcesTest, TwoSurfaces) { SurfaceFactory factory(&manager_, &client); SurfaceId surface1_id(7u); factory.Create(surface1_id); - Surface* surface1 = manager_.GetSurfaceForId(surface1_id); SurfaceId surface2_id(8u); factory.Create(surface2_id); - Surface* surface2 = manager_.GetSurfaceForId(surface2_id); ResourceId ids[] = {11, 12, 13}; SubmitCompositorFrameWithResources(ids, arraysize(ids), true, SurfaceId(), @@ -2025,28 +1804,16 @@ TEST_F(SurfaceAggregatorWithResourcesTest, TwoSurfaces) { SubmitCompositorFrameWithResources(ids2, arraysize(ids2), true, SurfaceId(), &factory, surface2_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(surface1)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(surface2)); - scoped_ptr<CompositorFrame> frame = aggregator_->Aggregate(surface1_id); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(surface1)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(surface2)); - SubmitCompositorFrameWithResources(NULL, 0, true, SurfaceId(), &factory, surface1_id); // Nothing should be available to be returned yet. EXPECT_TRUE(client.returned_resources().empty()); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(surface1)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(surface2)); - frame = aggregator_->Aggregate(surface2_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(surface1)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(surface2)); - // surface1_id wasn't referenced, so its resources should be returned. ASSERT_EQ(3u, client.returned_resources().size()); ResourceId returned_ids[3]; @@ -2067,13 +1834,10 @@ TEST_F(SurfaceAggregatorWithResourcesTest, InvalidChildSurface) { SurfaceFactory factory(&manager_, &client); SurfaceId root_surface_id(7u); factory.Create(root_surface_id); - Surface* root_surface = manager_.GetSurfaceForId(root_surface_id); SurfaceId middle_surface_id(8u); factory.Create(middle_surface_id); - Surface* middle_surface = manager_.GetSurfaceForId(middle_surface_id); SurfaceId child_surface_id(9u); factory.Create(child_surface_id); - Surface* child_surface = manager_.GetSurfaceForId(child_surface_id); ResourceId ids[] = {14, 15, 16}; SubmitCompositorFrameWithResources(ids, arraysize(ids), true, SurfaceId(), @@ -2089,17 +1853,9 @@ TEST_F(SurfaceAggregatorWithResourcesTest, InvalidChildSurface) { middle_surface_id, &factory, root_surface_id); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(middle_surface)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(child_surface)); - scoped_ptr<CompositorFrame> frame; frame = aggregator_->Aggregate(root_surface_id); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(middle_surface)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(child_surface)); - RenderPassList* pass_list = &frame->delegated_frame_data->render_pass_list; ASSERT_EQ(1u, pass_list->size()); EXPECT_EQ(1u, pass_list->back()->shared_quad_state_list.size()); @@ -2109,16 +1865,8 @@ TEST_F(SurfaceAggregatorWithResourcesTest, InvalidChildSurface) { child_surface_id, &factory, middle_surface_id); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(middle_surface)); - EXPECT_FALSE(surface_aggregator_client_.HasSurface(child_surface)); - frame = aggregator_->Aggregate(root_surface_id); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(middle_surface)); - EXPECT_TRUE(surface_aggregator_client_.HasSurface(child_surface)); - pass_list = &frame->delegated_frame_data->render_pass_list; ASSERT_EQ(1u, pass_list->size()); EXPECT_EQ(3u, pass_list->back()->shared_quad_state_list.size()); diff --git a/cc/surfaces/surface_display_output_surface.cc b/cc/surfaces/surface_display_output_surface.cc index 53d781f..642ec9f 100644 --- a/cc/surfaces/surface_display_output_surface.cc +++ b/cc/surfaces/surface_display_output_surface.cc @@ -33,7 +33,8 @@ SurfaceDisplayOutputSurface::SurfaceDisplayOutputSurface( } SurfaceDisplayOutputSurface::~SurfaceDisplayOutputSurface() { - client_ = NULL; + if (HasClient()) + DetachFromClient(); if (!surface_id_.is_null()) { factory_.Destroy(surface_id_); } @@ -72,6 +73,9 @@ void SurfaceDisplayOutputSurface::SwapBuffers(CompositorFrame* frame) { bool SurfaceDisplayOutputSurface::BindToClient(OutputSurfaceClient* client) { DCHECK(client); DCHECK(display_client_); + factory_.manager()->RegisterSurfaceFactoryClient(allocator_->id_namespace(), + this); + client_ = client; // Avoid initializing GL context here, as this should be sharing the // Display's context. @@ -84,6 +88,16 @@ void SurfaceDisplayOutputSurface::ForceReclaimResources() { SurfaceFactory::DrawCallback()); } +void SurfaceDisplayOutputSurface::DetachFromClient() { + DCHECK(HasClient()); + // Unregister the SurfaceFactoryClient here instead of the dtor so that only + // one client is alive for this namespace at any given time. + factory_.manager()->UnregisterSurfaceFactoryClient( + allocator_->id_namespace()); + OutputSurface::DetachFromClient(); + DCHECK(!HasClient()); +} + void SurfaceDisplayOutputSurface::ReturnResources( const ReturnedResourceArray& resources) { CompositorFrameAck ack; @@ -93,7 +107,6 @@ void SurfaceDisplayOutputSurface::ReturnResources( } void SurfaceDisplayOutputSurface::SetBeginFrameSource( - SurfaceId surface_id, BeginFrameSource* begin_frame_source) { // TODO(tansell): Hook this up. } diff --git a/cc/surfaces/surface_display_output_surface.h b/cc/surfaces/surface_display_output_surface.h index d173218..3b45807 100644 --- a/cc/surfaces/surface_display_output_surface.h +++ b/cc/surfaces/surface_display_output_surface.h @@ -43,11 +43,11 @@ class CC_SURFACES_EXPORT SurfaceDisplayOutputSurface void SwapBuffers(CompositorFrame* frame) override; bool BindToClient(OutputSurfaceClient* client) override; void ForceReclaimResources() override; + void DetachFromClient() override; // SurfaceFactoryClient implementation. void ReturnResources(const ReturnedResourceArray& resources) override; - void SetBeginFrameSource(SurfaceId surface_id, - BeginFrameSource* begin_frame_source) override; + void SetBeginFrameSource(BeginFrameSource* begin_frame_source) override; private: void SwapBuffersComplete(SurfaceDrawStatus drawn); diff --git a/cc/surfaces/surface_display_output_surface_unittest.cc b/cc/surfaces/surface_display_output_surface_unittest.cc index 327010b..367891cd 100644 --- a/cc/surfaces/surface_display_output_surface_unittest.cc +++ b/cc/surfaces/surface_display_output_surface_unittest.cc @@ -63,6 +63,7 @@ class SurfaceDisplayOutputSurfaceTest : public testing::Test { &allocator_, context_provider_, nullptr) { + surface_manager_.RegisterSurfaceIdNamespace(allocator_.id_namespace()); output_surface_ = display_client_.output_surface(); display_client_.set_surface_output_surface( &surface_display_output_surface_); diff --git a/cc/surfaces/surface_factory.cc b/cc/surfaces/surface_factory.cc index a45c809..d7f4810 100644 --- a/cc/surfaces/surface_factory.cc +++ b/cc/surfaces/surface_factory.cc @@ -29,7 +29,6 @@ SurfaceFactory::~SurfaceFactory() { << " entries in map on destruction."; } DestroyAll(); - client_->SetBeginFrameSource(SurfaceId(), nullptr); } void SurfaceFactory::DestroyAll() { @@ -54,11 +53,6 @@ void SurfaceFactory::Destroy(SurfaceId surface_id) { manager_->Destroy(std::move(surface)); } -void SurfaceFactory::SetBeginFrameSource(SurfaceId surface_id, - BeginFrameSource* begin_frame_source) { - client_->SetBeginFrameSource(surface_id, begin_frame_source); -} - void SurfaceFactory::SubmitCompositorFrame(SurfaceId surface_id, scoped_ptr<CompositorFrame> frame, const DrawCallback& callback) { diff --git a/cc/surfaces/surface_factory.h b/cc/surfaces/surface_factory.h index 1fcb3f1..742cc2a 100644 --- a/cc/surfaces/surface_factory.h +++ b/cc/surfaces/surface_factory.h @@ -49,9 +49,6 @@ class CC_SURFACES_EXPORT SurfaceFactory void Destroy(SurfaceId surface_id); void DestroyAll(); - void SetBeginFrameSource(SurfaceId surface_id, - BeginFrameSource* begin_frame_source); - // A frame can only be submitted to a surface created by this factory, // although the frame may reference surfaces created by other factories. // The callback is called the first time this frame is used to draw, or if diff --git a/cc/surfaces/surface_factory_client.h b/cc/surfaces/surface_factory_client.h index e79b185..f42bf35 100644 --- a/cc/surfaces/surface_factory_client.h +++ b/cc/surfaces/surface_factory_client.h @@ -24,14 +24,8 @@ class CC_SURFACES_EXPORT SurfaceFactoryClient { virtual void WillDrawSurface(SurfaceId surface_id, const gfx::Rect& damage_rect) {} - // This allows the SurfaceFactory to tell it's client what BeginFrameSource - // to use for a given surface_id. - // If there are multiple active surface_ids, it is the client's - // responsibility to pick or distribute the correct BeginFrameSource. - // If surface_id is null, then all BeginFrameSources previously - // set by this function should be invalidated. - virtual void SetBeginFrameSource(SurfaceId surface_id, - BeginFrameSource* begin_frame_source) = 0; + // This allows the SurfaceFactory to pass a BeginFrameSource to use. + virtual void SetBeginFrameSource(BeginFrameSource* begin_frame_source) = 0; }; } // namespace cc diff --git a/cc/surfaces/surface_factory_unittest.cc b/cc/surfaces/surface_factory_unittest.cc index 540d9da..a48d669 100644 --- a/cc/surfaces/surface_factory_unittest.cc +++ b/cc/surfaces/surface_factory_unittest.cc @@ -36,8 +36,7 @@ class TestSurfaceFactoryClient : public SurfaceFactoryClient { returned_resources_.end(), resources.begin(), resources.end()); } - void SetBeginFrameSource(SurfaceId surface_id, - BeginFrameSource* begin_frame_source) override { + void SetBeginFrameSource(BeginFrameSource* begin_frame_source) override { begin_frame_source_ = begin_frame_source; } @@ -594,36 +593,5 @@ TEST_F(SurfaceFactoryTest, DuplicateCopyRequest) { EXPECT_TRUE(called3); } -// Verifies BFS is forwarded to the client. -TEST_F(SurfaceFactoryTest, SetBeginFrameSource) { - FakeBeginFrameSource bfs1; - FakeBeginFrameSource bfs2; - EXPECT_EQ(nullptr, client_.begin_frame_source()); - factory_->SetBeginFrameSource(surface_id_, &bfs1); - EXPECT_EQ(&bfs1, client_.begin_frame_source()); - factory_->SetBeginFrameSource(surface_id_, &bfs2); - EXPECT_EQ(&bfs2, client_.begin_frame_source()); - factory_->SetBeginFrameSource(surface_id_, nullptr); - EXPECT_EQ(nullptr, client_.begin_frame_source()); -} - -TEST_F(SurfaceFactoryTest, BeginFrameSourceRemovedOnFactoryDestruction) { - FakeBeginFrameSource bfs; - factory_->SetBeginFrameSource(surface_id_, &bfs); - EXPECT_EQ(&bfs, client_.begin_frame_source()); - - // Prevent the Surface from being destroyed when we destroy the factory. - manager_.RegisterSurfaceIdNamespace(0); - manager_.GetSurfaceForId(surface_id_) - ->AddDestructionDependency(SurfaceSequence(0, 4)); - - surface_id_ = SurfaceId(); - factory_->DestroyAll(); - - EXPECT_EQ(&bfs, client_.begin_frame_source()); - factory_.reset(); - EXPECT_EQ(nullptr, client_.begin_frame_source()); -} - } // namespace } // namespace cc diff --git a/cc/surfaces/surface_manager.cc b/cc/surfaces/surface_manager.cc index 03951e4..23b49bc 100644 --- a/cc/surfaces/surface_manager.cc +++ b/cc/surfaces/surface_manager.cc @@ -9,10 +9,19 @@ #include "base/logging.h" #include "cc/surfaces/surface.h" +#include "cc/surfaces/surface_factory_client.h" #include "cc/surfaces/surface_id_allocator.h" namespace cc { +SurfaceManager::ClientSourceMapping::ClientSourceMapping() + : client(nullptr), source(nullptr) {} + +SurfaceManager::ClientSourceMapping::~ClientSourceMapping() { + DCHECK(is_empty()) << "client: " << client + << ", children: " << children.size(); +} + SurfaceManager::SurfaceManager() { thread_checker_.DetachFromThread(); } @@ -25,6 +34,10 @@ SurfaceManager::~SurfaceManager() { DeregisterSurface((*it)->surface_id()); delete *it; } + // All hierarchies, sources, and surface factory clients should be + // unregistered prior to SurfaceManager destruction. + DCHECK_EQ(namespace_client_map_.size(), 0u); + DCHECK_EQ(registered_sources_.size(), 0u); } void SurfaceManager::RegisterSurface(Surface* surface) { @@ -120,6 +133,192 @@ void SurfaceManager::GarbageCollectSurfaces() { } } +void SurfaceManager::RegisterSurfaceFactoryClient( + uint32_t id_namespace, + SurfaceFactoryClient* client) { + DCHECK(client); + DCHECK(!namespace_client_map_[id_namespace].client); + DCHECK_EQ(valid_surface_id_namespaces_.count(id_namespace), 1u); + + auto iter = namespace_client_map_.find(id_namespace); + if (iter == namespace_client_map_.end()) { + auto insert_result = namespace_client_map_.insert( + std::make_pair(id_namespace, ClientSourceMapping())); + DCHECK(insert_result.second); + iter = insert_result.first; + } + iter->second.client = client; + + // Propagate any previously set sources to the new client. + if (iter->second.source) + client->SetBeginFrameSource(iter->second.source); +} + +void SurfaceManager::UnregisterSurfaceFactoryClient(uint32_t id_namespace) { + DCHECK_EQ(valid_surface_id_namespaces_.count(id_namespace), 1u); + DCHECK_EQ(namespace_client_map_.count(id_namespace), 1u); + + auto iter = namespace_client_map_.find(id_namespace); + if (iter->second.source) + iter->second.client->SetBeginFrameSource(nullptr); + iter->second.client = nullptr; + + // The SurfaceFactoryClient and hierarchy can be registered/unregistered + // in either order, so empty namespace_client_map entries need to be + // checked when removing either clients or relationships. + if (iter->second.is_empty()) + namespace_client_map_.erase(iter); +} + +void SurfaceManager::RegisterBeginFrameSource(BeginFrameSource* source, + uint32_t id_namespace) { + DCHECK(source); + DCHECK_EQ(registered_sources_.count(source), 0u); + DCHECK_EQ(valid_surface_id_namespaces_.count(id_namespace), 1u); + + registered_sources_[source] = id_namespace; + RecursivelyAttachBeginFrameSource(id_namespace, source); +} + +void SurfaceManager::UnregisterBeginFrameSource(BeginFrameSource* source) { + DCHECK(source); + DCHECK_EQ(registered_sources_.count(source), 1u); + + uint32_t id_namespace = registered_sources_[source]; + registered_sources_.erase(source); + + if (namespace_client_map_.count(id_namespace) == 0u) + return; + + // TODO(enne): these walks could be done in one step. + // Remove this begin frame source from its subtree. + RecursivelyDetachBeginFrameSource(id_namespace, source); + // Then flush every remaining registered source to fix any sources that + // became null because of the previous step but that have an alternative. + for (auto source_iter : registered_sources_) + RecursivelyAttachBeginFrameSource(source_iter.second, source_iter.first); +} + +void SurfaceManager::RecursivelyAttachBeginFrameSource( + uint32_t id_namespace, + BeginFrameSource* source) { + ClientSourceMapping& mapping = namespace_client_map_[id_namespace]; + if (!mapping.source) { + mapping.source = source; + if (mapping.client) + mapping.client->SetBeginFrameSource(source); + } + for (size_t i = 0; i < mapping.children.size(); ++i) + RecursivelyAttachBeginFrameSource(mapping.children[i], source); +} + +void SurfaceManager::RecursivelyDetachBeginFrameSource( + uint32_t id_namespace, + BeginFrameSource* source) { + auto iter = namespace_client_map_.find(id_namespace); + if (iter == namespace_client_map_.end()) + return; + if (iter->second.source == source) { + iter->second.source = nullptr; + if (iter->second.client) + iter->second.client->SetBeginFrameSource(nullptr); + } + + if (iter->second.is_empty()) { + namespace_client_map_.erase(iter); + return; + } + + std::vector<uint32_t>& children = iter->second.children; + for (size_t i = 0; i < children.size(); ++i) { + RecursivelyDetachBeginFrameSource(children[i], source); + } +} + +bool SurfaceManager::ChildContains(uint32_t child_namespace, + uint32_t search_namespace) const { + auto iter = namespace_client_map_.find(child_namespace); + if (iter == namespace_client_map_.end()) + return false; + + const std::vector<uint32_t>& children = iter->second.children; + for (size_t i = 0; i < children.size(); ++i) { + if (children[i] == search_namespace) + return true; + if (ChildContains(children[i], search_namespace)) + return true; + } + return false; +} + +void SurfaceManager::RegisterSurfaceNamespaceHierarchy( + uint32_t parent_namespace, + uint32_t child_namespace) { + DCHECK_EQ(valid_surface_id_namespaces_.count(parent_namespace), 1u); + DCHECK_EQ(valid_surface_id_namespaces_.count(child_namespace), 1u); + + // If it's possible to reach the parent through the child's descendant chain, + // then this will create an infinite loop. Might as well just crash here. + CHECK(!ChildContains(child_namespace, parent_namespace)); + + std::vector<uint32_t>& children = + namespace_client_map_[parent_namespace].children; + for (size_t i = 0; i < children.size(); ++i) + DCHECK_NE(children[i], child_namespace); + children.push_back(child_namespace); + + // If the parent has no source, then attaching it to this child will + // not change any downstream sources. + BeginFrameSource* parent_source = + namespace_client_map_[parent_namespace].source; + if (!parent_source) + return; + + DCHECK_EQ(registered_sources_.count(parent_source), 1u); + RecursivelyAttachBeginFrameSource(child_namespace, parent_source); +} + +void SurfaceManager::UnregisterSurfaceNamespaceHierarchy( + uint32_t parent_namespace, + uint32_t child_namespace) { + DCHECK_EQ(valid_surface_id_namespaces_.count(parent_namespace), 1u); + DCHECK_EQ(valid_surface_id_namespaces_.count(child_namespace), 1u); + DCHECK_EQ(namespace_client_map_.count(parent_namespace), 1u); + + auto iter = namespace_client_map_.find(parent_namespace); + + std::vector<uint32_t>& children = iter->second.children; + bool found_child = false; + for (size_t i = 0; i < children.size(); ++i) { + if (children[i] == child_namespace) { + found_child = true; + children[i] = children[children.size() - 1]; + children.resize(children.size() - 1); + break; + } + } + DCHECK(found_child); + + // The SurfaceFactoryClient and hierarchy can be registered/unregistered + // in either order, so empty namespace_client_map entries need to be + // checked when removing either clients or relationships. + if (iter->second.is_empty()) { + namespace_client_map_.erase(iter); + return; + } + + // If the parent does not have a begin frame source, then disconnecting it + // will not change any of its children. + BeginFrameSource* parent_source = iter->second.source; + if (!parent_source) + return; + + // TODO(enne): these walks could be done in one step. + RecursivelyDetachBeginFrameSource(child_namespace, parent_source); + for (auto source_iter : registered_sources_) + RecursivelyAttachBeginFrameSource(source_iter.second, source_iter.first); +} + Surface* SurfaceManager::GetSurfaceForId(SurfaceId surface_id) { DCHECK(thread_checker_.CalledOnValidThread()); SurfaceMap::iterator it = surface_map_.find(surface_id); @@ -129,7 +328,7 @@ Surface* SurfaceManager::GetSurfaceForId(SurfaceId surface_id) { } bool SurfaceManager::SurfaceModified(SurfaceId surface_id) { - DCHECK(thread_checker_.CalledOnValidThread()); + CHECK(thread_checker_.CalledOnValidThread()); bool changed = false; FOR_EACH_OBSERVER(SurfaceDamageObserver, observer_list_, OnSurfaceDamaged(surface_id, &changed)); diff --git a/cc/surfaces/surface_manager.h b/cc/surfaces/surface_manager.h index a3f7b9b..8ddace4 100644 --- a/cc/surfaces/surface_manager.h +++ b/cc/surfaces/surface_manager.h @@ -21,8 +21,10 @@ #include "cc/surfaces/surfaces_export.h" namespace cc { +class BeginFrameSource; class CompositorFrame; class Surface; +class SurfaceFactoryClient; class CC_SURFACES_EXPORT SurfaceManager { public: @@ -58,7 +60,47 @@ class CC_SURFACES_EXPORT SurfaceManager { // possibly because a renderer process has crashed. void InvalidateSurfaceIdNamespace(uint32_t id_namespace); + // SurfaceFactoryClient, hierarchy, and BeginFrameSource can be registered + // and unregistered in any order with respect to each other. + // + // This happens in practice, e.g. the relationship to between ui::Compositor / + // DelegatedFrameHost is known before ui::Compositor has a surface/client). + // However, DelegatedFrameHost can register itself as a client before its + // relationship with the ui::Compositor is known. + + // Associates a SurfaceFactoryClient with the surface id namespace it uses. + // SurfaceFactoryClient and surface namespaces/allocators have a 1:1 mapping. + // Caller guarantees the client is alive between register/unregister. + // Reregistering the same namespace when a previous client is active is not + // valid. + void RegisterSurfaceFactoryClient(uint32_t id_namespace, + SurfaceFactoryClient* client); + void UnregisterSurfaceFactoryClient(uint32_t id_namespace); + + // Associates a |source| with a particular namespace. That namespace and + // any children of that namespace with valid clients can potentially use + // that |source|. + void RegisterBeginFrameSource(BeginFrameSource* source, + uint32_t id_namespace); + void UnregisterBeginFrameSource(BeginFrameSource* source); + + // Register a relationship between two namespaces. This relationship means + // that surfaces from the child namespace will be displayed in the parent. + // Children are allowed to use any begin frame source that their parent can + // use. + void RegisterSurfaceNamespaceHierarchy(uint32_t parent_namespace, + uint32_t child_namespace); + void UnregisterSurfaceNamespaceHierarchy(uint32_t parent_namespace, + uint32_t child_namespace); + private: + void RecursivelyAttachBeginFrameSource(uint32_t id_namespace, + BeginFrameSource* source); + void RecursivelyDetachBeginFrameSource(uint32_t id_namespace, + BeginFrameSource* source); + // Returns true if |child namespace| is or has |search_namespace| as a child. + bool ChildContains(uint32_t child_namespace, uint32_t search_namespace) const; + void GarbageCollectSurfaces(); using SurfaceMap = std::unordered_map<SurfaceId, Surface*, SurfaceIdHash>; @@ -80,6 +122,25 @@ class CC_SURFACES_EXPORT SurfaceManager { // satisfied. std::unordered_set<uint32_t> valid_surface_id_namespaces_; + // Begin frame source routing. Both BeginFrameSource and SurfaceFactoryClient + // pointers guaranteed alive by callers until unregistered. + struct ClientSourceMapping { + ClientSourceMapping(); + ~ClientSourceMapping(); + bool is_empty() const { return !client && !children.size(); } + // The client that's responsible for creating this namespace. Never null. + SurfaceFactoryClient* client; + // The currently assigned begin frame source for this client. + BeginFrameSource* source; + // This represents a dag of parent -> children mapping. + std::vector<uint32_t> children; + }; + std::unordered_map<uint32_t, ClientSourceMapping> namespace_client_map_; + // Set of which sources are registered to which namespace. Any child + // that is implicitly using this namespace must be reachable by the + // parent in the dag. + std::unordered_map<BeginFrameSource*, uint32_t> registered_sources_; + DISALLOW_COPY_AND_ASSIGN(SurfaceManager); }; diff --git a/cc/surfaces/surface_manager_unittest.cc b/cc/surfaces/surface_manager_unittest.cc new file mode 100644 index 0000000..9e9ff23 --- /dev/null +++ b/cc/surfaces/surface_manager_unittest.cc @@ -0,0 +1,382 @@ +// Copyright 2016 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 <stddef.h> + +#include "cc/scheduler/begin_frame_source.h" +#include "cc/surfaces/surface_factory_client.h" +#include "cc/surfaces/surface_manager.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace cc { + +class FakeSurfaceFactoryClient : public SurfaceFactoryClient { + public: + explicit FakeSurfaceFactoryClient(int id_namespace) + : source_(nullptr), manager_(nullptr), id_namespace_(id_namespace) {} + FakeSurfaceFactoryClient(int id_namespace, SurfaceManager* manager) + : source_(nullptr), manager_(nullptr), id_namespace_(id_namespace) { + DCHECK(manager); + Register(manager); + } + + ~FakeSurfaceFactoryClient() override { + if (manager_) { + Unregister(); + } + EXPECT_EQ(source_, nullptr); + } + + BeginFrameSource* source() { return source_; } + uint32_t id_namespace() { return id_namespace_; } + + void Register(SurfaceManager* manager) { + EXPECT_EQ(manager_, nullptr); + manager_ = manager; + manager_->RegisterSurfaceFactoryClient(id_namespace_, this); + } + + void Unregister() { + EXPECT_NE(manager_, nullptr); + manager_->UnregisterSurfaceFactoryClient(id_namespace_); + manager_ = nullptr; + } + + // SurfaceFactoryClient implementation. + void ReturnResources(const ReturnedResourceArray& resources) override{}; + void SetBeginFrameSource(BeginFrameSource* begin_frame_source) override { + DCHECK(!source_ || !begin_frame_source); + source_ = begin_frame_source; + }; + + private: + BeginFrameSource* source_; + SurfaceManager* manager_; + uint32_t id_namespace_; +}; + +class EmptyBeginFrameSource : public BeginFrameSource { + public: + void DidFinishFrame(size_t remaining_frames) override{}; + void AddObserver(BeginFrameObserver* obs) override{}; + void RemoveObserver(BeginFrameObserver* obs) override{}; + void SetClientReady() override {} + void AsValueInto(base::trace_event::TracedValue* dict) const override{}; +}; + +class SurfaceManagerTest : 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_NAMESPACE = 10 }; + + SurfaceManagerTest() { + for (size_t i = 0; i < MAX_NAMESPACE; ++i) + manager_.RegisterSurfaceIdNamespace(i); + } + + ~SurfaceManagerTest() override { + for (size_t i = 0; i < MAX_NAMESPACE; ++i) + manager_.InvalidateSurfaceIdNamespace(i); + } + + protected: + SurfaceManager manager_; +}; + +TEST_F(SurfaceManagerTest, SingleClients) { + FakeSurfaceFactoryClient client(1); + FakeSurfaceFactoryClient other_client(2); + EmptyBeginFrameSource source; + + EXPECT_EQ(client.source(), nullptr); + EXPECT_EQ(other_client.source(), nullptr); + client.Register(&manager_); + other_client.Register(&manager_); + EXPECT_EQ(client.source(), nullptr); + EXPECT_EQ(other_client.source(), nullptr); + + // Test setting unsetting BFS + manager_.RegisterBeginFrameSource(&source, client.id_namespace()); + EXPECT_EQ(client.source(), &source); + EXPECT_EQ(other_client.source(), nullptr); + manager_.UnregisterBeginFrameSource(&source); + EXPECT_EQ(client.source(), nullptr); + EXPECT_EQ(other_client.source(), nullptr); + + // Set BFS for other namespace + manager_.RegisterBeginFrameSource(&source, other_client.id_namespace()); + EXPECT_EQ(other_client.source(), &source); + EXPECT_EQ(client.source(), nullptr); + manager_.UnregisterBeginFrameSource(&source); + EXPECT_EQ(client.source(), nullptr); + EXPECT_EQ(other_client.source(), nullptr); + + // Re-set BFS for original + manager_.RegisterBeginFrameSource(&source, client.id_namespace()); + EXPECT_EQ(client.source(), &source); + manager_.UnregisterBeginFrameSource(&source); + EXPECT_EQ(client.source(), nullptr); +} + +TEST_F(SurfaceManagerTest, MultipleDisplays) { + EmptyBeginFrameSource root1_source; + EmptyBeginFrameSource root2_source; + + // root1 -> A -> B + // root2 -> C + FakeSurfaceFactoryClient root1(1, &manager_); + FakeSurfaceFactoryClient root2(2, &manager_); + FakeSurfaceFactoryClient client_a(3, &manager_); + FakeSurfaceFactoryClient client_b(4, &manager_); + FakeSurfaceFactoryClient client_c(5, &manager_); + + manager_.RegisterBeginFrameSource(&root1_source, root1.id_namespace()); + manager_.RegisterBeginFrameSource(&root2_source, root2.id_namespace()); + EXPECT_EQ(root1.source(), &root1_source); + EXPECT_EQ(root2.source(), &root2_source); + + // Set up initial hierarchy. + manager_.RegisterSurfaceNamespaceHierarchy(root1.id_namespace(), + client_a.id_namespace()); + EXPECT_EQ(client_a.source(), root1.source()); + manager_.RegisterSurfaceNamespaceHierarchy(client_a.id_namespace(), + client_b.id_namespace()); + EXPECT_EQ(client_b.source(), root1.source()); + manager_.RegisterSurfaceNamespaceHierarchy(root2.id_namespace(), + client_c.id_namespace()); + EXPECT_EQ(client_c.source(), root2.source()); + + // Attach A into root2's subtree, like a window moving across displays. + // root1 -> A -> B + // root2 -> C -> A -> B + manager_.RegisterSurfaceNamespaceHierarchy(client_c.id_namespace(), + client_a.id_namespace()); + // With the heuristic of just keeping existing BFS in the face of multiple, + // no client sources should change. + EXPECT_EQ(client_a.source(), root1.source()); + EXPECT_EQ(client_b.source(), root1.source()); + EXPECT_EQ(client_c.source(), root2.source()); + + // Detach A from root1. A and B should now be updated to root2. + manager_.UnregisterSurfaceNamespaceHierarchy(root1.id_namespace(), + client_a.id_namespace()); + EXPECT_EQ(client_a.source(), root2.source()); + EXPECT_EQ(client_b.source(), root2.source()); + EXPECT_EQ(client_c.source(), root2.source()); + + // Detach root1 from BFS. root1 should now have no source. + manager_.UnregisterBeginFrameSource(&root1_source); + EXPECT_EQ(root1.source(), nullptr); + EXPECT_NE(root2.source(), nullptr); + + // Detatch root2 from BFS. + manager_.UnregisterBeginFrameSource(&root2_source); + EXPECT_EQ(client_a.source(), nullptr); + EXPECT_EQ(client_b.source(), nullptr); + EXPECT_EQ(client_c.source(), nullptr); + EXPECT_EQ(root2.source(), nullptr); + + // Cleanup hierarchy. + manager_.UnregisterSurfaceNamespaceHierarchy(root2.id_namespace(), + client_c.id_namespace()); + manager_.UnregisterSurfaceNamespaceHierarchy(client_c.id_namespace(), + client_a.id_namespace()); + manager_.UnregisterSurfaceNamespaceHierarchy(client_a.id_namespace(), + client_b.id_namespace()); +} + +// In practice, registering and unregistering both parent/child relationships +// and SurfaceFactoryClients can happen in any ordering with respect to +// each other. These following tests verify that all the data structures +// are properly set up and cleaned up under the four permutations of orderings +// of this nesting. + +class SurfaceManagerOrderingTest : public SurfaceManagerTest { + public: + SurfaceManagerOrderingTest() + : client_a_(1), + client_b_(2), + client_c_(3), + hierarchy_registered_(false), + clients_registered_(false), + bfs_registered_(false) { + AssertCorrectBFSState(); + } + + ~SurfaceManagerOrderingTest() override { + EXPECT_EQ(hierarchy_registered_, false); + EXPECT_EQ(clients_registered_, false); + EXPECT_EQ(bfs_registered_, false); + AssertCorrectBFSState(); + } + + void RegisterHierarchy() { + DCHECK(!hierarchy_registered_); + hierarchy_registered_ = true; + manager_.RegisterSurfaceNamespaceHierarchy(client_a_.id_namespace(), + client_b_.id_namespace()); + manager_.RegisterSurfaceNamespaceHierarchy(client_b_.id_namespace(), + client_c_.id_namespace()); + AssertCorrectBFSState(); + } + void UnregisterHierarchy() { + DCHECK(hierarchy_registered_); + hierarchy_registered_ = false; + manager_.UnregisterSurfaceNamespaceHierarchy(client_a_.id_namespace(), + client_b_.id_namespace()); + manager_.UnregisterSurfaceNamespaceHierarchy(client_b_.id_namespace(), + client_c_.id_namespace()); + AssertCorrectBFSState(); + } + + void RegisterClients() { + DCHECK(!clients_registered_); + clients_registered_ = true; + client_a_.Register(&manager_); + client_b_.Register(&manager_); + client_c_.Register(&manager_); + AssertCorrectBFSState(); + } + + void UnregisterClients() { + DCHECK(clients_registered_); + clients_registered_ = false; + client_a_.Unregister(); + client_b_.Unregister(); + client_c_.Unregister(); + AssertCorrectBFSState(); + } + + void RegisterBFS() { + DCHECK(!bfs_registered_); + bfs_registered_ = true; + manager_.RegisterBeginFrameSource(&source_, client_a_.id_namespace()); + AssertCorrectBFSState(); + } + void UnregisterBFS() { + DCHECK(bfs_registered_); + bfs_registered_ = false; + manager_.UnregisterBeginFrameSource(&source_); + AssertCorrectBFSState(); + } + + void AssertEmptyBFS() { + EXPECT_EQ(client_a_.source(), nullptr); + EXPECT_EQ(client_b_.source(), nullptr); + EXPECT_EQ(client_c_.source(), nullptr); + } + + void AssertAllValidBFS() { + EXPECT_EQ(client_a_.source(), &source_); + EXPECT_EQ(client_b_.source(), &source_); + EXPECT_EQ(client_c_.source(), &source_); + } + + protected: + void AssertCorrectBFSState() { + if (!clients_registered_ || !bfs_registered_) { + AssertEmptyBFS(); + return; + } + if (!hierarchy_registered_) { + // A valid but not attached to anything. + EXPECT_EQ(client_a_.source(), &source_); + EXPECT_EQ(client_b_.source(), nullptr); + EXPECT_EQ(client_c_.source(), nullptr); + return; + } + + AssertAllValidBFS(); + } + + EmptyBeginFrameSource source_; + // A -> B -> C hierarchy, with A always having the BFS. + FakeSurfaceFactoryClient client_a_; + FakeSurfaceFactoryClient client_b_; + FakeSurfaceFactoryClient client_c_; + + bool hierarchy_registered_; + bool clients_registered_; + bool bfs_registered_; +}; + +enum RegisterOrder { REGISTER_HIERARCHY_FIRST, REGISTER_CLIENTS_FIRST }; +enum UnregisterOrder { UNREGISTER_HIERARCHY_FIRST, UNREGISTER_CLIENTS_FIRST }; +enum BFSOrder { BFS_FIRST, BFS_SECOND, BFS_THIRD }; + +static const RegisterOrder kRegisterOrderList[] = {REGISTER_HIERARCHY_FIRST, + REGISTER_CLIENTS_FIRST}; +static const UnregisterOrder kUnregisterOrderList[] = { + UNREGISTER_HIERARCHY_FIRST, UNREGISTER_CLIENTS_FIRST}; +static const BFSOrder kBFSOrderList[] = {BFS_FIRST, BFS_SECOND, BFS_THIRD}; + +class SurfaceManagerOrderingParamTest + : public SurfaceManagerOrderingTest, + public ::testing::WithParamInterface< + std::tr1::tuple<RegisterOrder, UnregisterOrder, BFSOrder>> {}; + +TEST_P(SurfaceManagerOrderingParamTest, Ordering) { + // Test the four permutations of client/hierarchy setting/unsetting and test + // each place the BFS can be added and removed. The BFS and the + // client/hierarchy are less related, so BFS is tested independently instead + // of every permutation of BFS setting and unsetting. + // The register/unregister functions themselves test most of the state. + RegisterOrder register_order = std::tr1::get<0>(GetParam()); + UnregisterOrder unregister_order = std::tr1::get<1>(GetParam()); + BFSOrder bfs_order = std::tr1::get<2>(GetParam()); + + // Attach everything up in the specified order. + if (bfs_order == BFS_FIRST) + RegisterBFS(); + + if (register_order == REGISTER_HIERARCHY_FIRST) + RegisterHierarchy(); + else + RegisterClients(); + + if (bfs_order == BFS_SECOND) + RegisterBFS(); + + if (register_order == REGISTER_HIERARCHY_FIRST) + RegisterClients(); + else + RegisterHierarchy(); + + if (bfs_order == BFS_THIRD) + RegisterBFS(); + + // Everything hooked up, so should be valid. + AssertAllValidBFS(); + + // Detach everything in the specified order. + if (bfs_order == BFS_THIRD) + UnregisterBFS(); + + if (unregister_order == UNREGISTER_HIERARCHY_FIRST) + UnregisterHierarchy(); + else + UnregisterClients(); + + if (bfs_order == BFS_SECOND) + UnregisterBFS(); + + if (unregister_order == UNREGISTER_HIERARCHY_FIRST) + UnregisterClients(); + else + UnregisterHierarchy(); + + if (bfs_order == BFS_FIRST) + UnregisterBFS(); +} + +INSTANTIATE_TEST_CASE_P( + SurfaceManagerOrderingParamTestInstantiation, + SurfaceManagerOrderingParamTest, + ::testing::Combine(::testing::ValuesIn(kRegisterOrderList), + ::testing::ValuesIn(kUnregisterOrderList), + ::testing::ValuesIn(kBFSOrderList))); + +} // namespace cc diff --git a/cc/surfaces/surface_unittest.cc b/cc/surfaces/surface_unittest.cc index 5a9b744..72070c6 100644 --- a/cc/surfaces/surface_unittest.cc +++ b/cc/surfaces/surface_unittest.cc @@ -19,8 +19,7 @@ class FakeSurfaceFactoryClient : public SurfaceFactoryClient { void ReturnResources(const ReturnedResourceArray& resources) override {} - void SetBeginFrameSource(SurfaceId surface_id, - BeginFrameSource* begin_frame_source) override { + void SetBeginFrameSource(BeginFrameSource* begin_frame_source) override { begin_frame_source_ = begin_frame_source; } @@ -45,111 +44,5 @@ TEST(SurfaceTest, SurfaceLifetime) { EXPECT_EQ(NULL, manager.GetSurfaceForId(surface_id)); } -TEST(SurfaceTest, StableBeginFrameSourceIndependentOfOrderAdded) { - SurfaceManager manager; - FakeSurfaceFactoryClient surface_factory_client; - SurfaceFactory factory(&manager, &surface_factory_client); - - SurfaceId surface_id(6); - factory.Create(surface_id); - Surface* surface = manager.GetSurfaceForId(surface_id); - - FakeBeginFrameSource bfs1; - FakeBeginFrameSource bfs2; - FakeBeginFrameSource bfs3; - - // Order 1. - surface->AddBeginFrameSource(&bfs1); - surface->AddBeginFrameSource(&bfs2); - surface->AddBeginFrameSource(&bfs3); - BeginFrameSource* bfs_order1 = surface_factory_client.begin_frame_source(); - // Make sure one of the provided sources was chosen. - EXPECT_TRUE(&bfs1 == bfs_order1 || &bfs2 == bfs_order1 || - &bfs3 == bfs_order1); - surface->RemoveBeginFrameSource(&bfs1); - surface->RemoveBeginFrameSource(&bfs2); - surface->RemoveBeginFrameSource(&bfs3); - EXPECT_EQ(nullptr, surface_factory_client.begin_frame_source()); - - // Order 2. - surface->AddBeginFrameSource(&bfs1); - surface->AddBeginFrameSource(&bfs3); - surface->AddBeginFrameSource(&bfs2); - BeginFrameSource* bfs_order2 = surface_factory_client.begin_frame_source(); - // Verify choice is same as before. - EXPECT_EQ(bfs_order1, bfs_order2); - surface->RemoveBeginFrameSource(&bfs1); - surface->RemoveBeginFrameSource(&bfs2); - surface->RemoveBeginFrameSource(&bfs3); - EXPECT_EQ(nullptr, surface_factory_client.begin_frame_source()); - - // Order 3. - surface->AddBeginFrameSource(&bfs2); - surface->AddBeginFrameSource(&bfs1); - surface->AddBeginFrameSource(&bfs3); - BeginFrameSource* bfs_order3 = surface_factory_client.begin_frame_source(); - // Verify choice is same as before. - EXPECT_EQ(bfs_order2, bfs_order3); - surface->RemoveBeginFrameSource(&bfs1); - surface->RemoveBeginFrameSource(&bfs2); - surface->RemoveBeginFrameSource(&bfs3); - EXPECT_EQ(nullptr, surface_factory_client.begin_frame_source()); - - // Order 4. - surface->AddBeginFrameSource(&bfs2); - surface->AddBeginFrameSource(&bfs3); - surface->AddBeginFrameSource(&bfs1); - BeginFrameSource* bfs_order4 = surface_factory_client.begin_frame_source(); - // Verify choice is same as before. - EXPECT_EQ(bfs_order3, bfs_order4); - surface->RemoveBeginFrameSource(&bfs1); - surface->RemoveBeginFrameSource(&bfs2); - surface->RemoveBeginFrameSource(&bfs3); - EXPECT_EQ(nullptr, surface_factory_client.begin_frame_source()); - - // Order 5. - surface->AddBeginFrameSource(&bfs3); - surface->AddBeginFrameSource(&bfs1); - surface->AddBeginFrameSource(&bfs2); - BeginFrameSource* bfs_order5 = surface_factory_client.begin_frame_source(); - // Verify choice is same as before. - EXPECT_EQ(bfs_order4, bfs_order5); - surface->RemoveBeginFrameSource(&bfs1); - surface->RemoveBeginFrameSource(&bfs2); - surface->RemoveBeginFrameSource(&bfs3); - EXPECT_EQ(nullptr, surface_factory_client.begin_frame_source()); - - // Order 6. - surface->AddBeginFrameSource(&bfs3); - surface->AddBeginFrameSource(&bfs2); - surface->AddBeginFrameSource(&bfs1); - BeginFrameSource* bfs_order6 = surface_factory_client.begin_frame_source(); - // Verify choice is same as before. - EXPECT_EQ(bfs_order5, bfs_order6); - surface->RemoveBeginFrameSource(&bfs1); - surface->RemoveBeginFrameSource(&bfs2); - surface->RemoveBeginFrameSource(&bfs3); - EXPECT_EQ(nullptr, surface_factory_client.begin_frame_source()); -} - -TEST(SurfaceTest, BeginFrameSourceRemovedOnSurfaceDestruction) { - SurfaceManager manager; - FakeSurfaceFactoryClient surface_factory_client; - SurfaceFactory factory(&manager, &surface_factory_client); - FakeBeginFrameSource bfs; - - SurfaceId surface_id(6); - factory.Create(surface_id); - Surface* surface = manager.GetSurfaceForId(surface_id); - surface->AddBeginFrameSource(&bfs); - - BeginFrameSource* bfs_before = surface_factory_client.begin_frame_source(); - factory.Destroy(surface_id); - BeginFrameSource* bfs_after = surface_factory_client.begin_frame_source(); - - EXPECT_EQ(&bfs, bfs_before); - EXPECT_EQ(nullptr, bfs_after); -} - } // namespace } // namespace cc diff --git a/cc/surfaces/surfaces_pixeltest.cc b/cc/surfaces/surfaces_pixeltest.cc index 053a563..405a3d8 100644 --- a/cc/surfaces/surfaces_pixeltest.cc +++ b/cc/surfaces/surfaces_pixeltest.cc @@ -24,14 +24,7 @@ namespace { class EmptySurfaceFactoryClient : public SurfaceFactoryClient { public: void ReturnResources(const ReturnedResourceArray& resources) override {} - void SetBeginFrameSource(SurfaceId surface_id, - BeginFrameSource* begin_frame_source) override {} -}; - -class EmptySurfaceAggregatorClient : public SurfaceAggregatorClient { - public: - void AddSurface(Surface* surface) override {} - void RemoveSurface(Surface* surface) override {} + void SetBeginFrameSource(BeginFrameSource* begin_frame_source) override {} }; class SurfacesPixelTest : public RendererPixelTest<GLRenderer> { @@ -91,9 +84,7 @@ TEST_F(SurfacesPixelTest, DrawSimpleFrame) { factory_.SubmitCompositorFrame(root_surface_id, std::move(root_frame), SurfaceFactory::DrawCallback()); - EmptySurfaceAggregatorClient surface_aggregator_client; - SurfaceAggregator aggregator(&surface_aggregator_client, &manager_, - resource_provider_.get(), true); + SurfaceAggregator aggregator(&manager_, resource_provider_.get(), true); scoped_ptr<CompositorFrame> aggregated_frame = aggregator.Aggregate(root_surface_id); factory_.Destroy(root_surface_id); @@ -177,9 +168,7 @@ TEST_F(SurfacesPixelTest, DrawSimpleAggregatedFrame) { SurfaceFactory::DrawCallback()); } - EmptySurfaceAggregatorClient surface_aggregator_client; - SurfaceAggregator aggregator(&surface_aggregator_client, &manager_, - resource_provider_.get(), true); + SurfaceAggregator aggregator(&manager_, resource_provider_.get(), true); scoped_ptr<CompositorFrame> aggregated_frame = aggregator.Aggregate(root_surface_id); @@ -322,9 +311,7 @@ TEST_F(SurfacesPixelTest, DrawAggregatedFrameWithSurfaceTransforms) { SurfaceFactory::DrawCallback()); } - EmptySurfaceAggregatorClient surface_aggregator_client; - SurfaceAggregator aggregator(&surface_aggregator_client, &manager_, - resource_provider_.get(), true); + SurfaceAggregator aggregator(&manager_, resource_provider_.get(), true); scoped_ptr<CompositorFrame> aggregated_frame = aggregator.Aggregate(root_surface_id); diff --git a/cc/test/surface_hittest_test_helpers.h b/cc/test/surface_hittest_test_helpers.h index e3f8ff9..81b292d 100644 --- a/cc/test/surface_hittest_test_helpers.h +++ b/cc/test/surface_hittest_test_helpers.h @@ -29,8 +29,7 @@ namespace test { class EmptySurfaceFactoryClient : public SurfaceFactoryClient { public: void ReturnResources(const ReturnedResourceArray& resources) override {} - void SetBeginFrameSource(SurfaceId surface_id, - BeginFrameSource* begin_frame_source) override {} + void SetBeginFrameSource(BeginFrameSource* begin_frame_source) override {} }; void CreateSharedQuadState(RenderPass* pass, diff --git a/components/mus/surfaces/top_level_display_client.cc b/components/mus/surfaces/top_level_display_client.cc index fc0568a..c4f1de1 100644 --- a/components/mus/surfaces/top_level_display_client.cc +++ b/components/mus/surfaces/top_level_display_client.cc @@ -119,7 +119,6 @@ void TopLevelDisplayClient::ReturnResources( } void TopLevelDisplayClient::SetBeginFrameSource( - cc::SurfaceId surface_id, cc::BeginFrameSource* begin_frame_source) { // TODO(tansell): Implement this. } diff --git a/components/mus/surfaces/top_level_display_client.h b/components/mus/surfaces/top_level_display_client.h index ce3e39a..de5ce2a 100644 --- a/components/mus/surfaces/top_level_display_client.h +++ b/components/mus/surfaces/top_level_display_client.h @@ -64,8 +64,7 @@ class TopLevelDisplayClient : public cc::DisplayClient, // SurfaceFactoryClient implementation. void ReturnResources(const cc::ReturnedResourceArray& resources) override; - void SetBeginFrameSource(cc::SurfaceId surface_id, - cc::BeginFrameSource* begin_frame_source) override; + void SetBeginFrameSource(cc::BeginFrameSource* begin_frame_source) override; scoped_refptr<base::SingleThreadTaskRunner> task_runner_; scoped_refptr<SurfacesState> surfaces_state_; diff --git a/components/mus/ws/server_window_surface.cc b/components/mus/ws/server_window_surface.cc index 9c836c9..800de90 100644 --- a/components/mus/ws/server_window_surface.cc +++ b/components/mus/ws/server_window_surface.cc @@ -32,13 +32,10 @@ ServerWindowSurface::ServerWindowSurface( : manager_(manager), surface_type_(surface_type), surface_id_(manager->GenerateId()), - surface_factory_(manager_->window() - ->delegate() - ->GetSurfacesState() - ->manager(), - this), + surface_factory_(manager_->GetSurfaceManager(), this), client_(std::move(client)), - binding_(this, std::move(request)) { + binding_(this, std::move(request)), + registered_surface_factory_client_(false) { surface_factory_.Create(surface_id_); } @@ -47,6 +44,11 @@ ServerWindowSurface::~ServerWindowSurface() { // call back into here and access |client_| so we should destroy // |surface_factory_|'s resources early on. surface_factory_.DestroyAll(); + + if (registered_surface_factory_client_) { + cc::SurfaceManager* surface_manager = manager_->GetSurfaceManager(); + surface_manager->UnregisterSurfaceFactoryClient(manager_->id_namespace()); + } } void ServerWindowSurface::SubmitCompositorFrame( @@ -84,6 +86,13 @@ void ServerWindowSurface::DestroySurfacesScheduledForDestruction() { surface_factory_.Destroy(id); } +void ServerWindowSurface::RegisterForBeginFrames() { + DCHECK(!registered_surface_factory_client_); + registered_surface_factory_client_ = true; + cc::SurfaceManager* surface_manager = manager_->GetSurfaceManager(); + surface_manager->RegisterSurfaceFactoryClient(manager_->id_namespace(), this); +} + ServerWindow* ServerWindowSurface::window() { return manager_->window(); } @@ -153,7 +162,6 @@ void ServerWindowSurface::ReturnResources( } void ServerWindowSurface::SetBeginFrameSource( - cc::SurfaceId surface_id, cc::BeginFrameSource* begin_frame_source) { // TODO(tansell): Implement this. } diff --git a/components/mus/ws/server_window_surface.h b/components/mus/ws/server_window_surface.h index 378c09d..f6197a6 100644 --- a/components/mus/ws/server_window_surface.h +++ b/components/mus/ws/server_window_surface.h @@ -59,6 +59,9 @@ class ServerWindowSurface : public mojom::Surface, // Destroys old surfaces that have been outdated by a new surface. void DestroySurfacesScheduledForDestruction(); + // Registers this with the SurfaceManager + void RegisterForBeginFrames(); + private: ServerWindow* window(); @@ -78,8 +81,7 @@ class ServerWindowSurface : public mojom::Surface, // SurfaceFactoryClient implementation. void ReturnResources(const cc::ReturnedResourceArray& resources) override; - void SetBeginFrameSource(cc::SurfaceId surface_id, - cc::BeginFrameSource* begin_frame_source) override; + void SetBeginFrameSource(cc::BeginFrameSource* begin_frame_source) override; ServerWindowSurfaceManager* manager_; // Owns this. @@ -98,6 +100,8 @@ class ServerWindowSurface : public mojom::Surface, // Set of surface ids that need to be destroyed. std::set<cc::SurfaceId> surfaces_scheduled_for_destruction_; + bool registered_surface_factory_client_; + DISALLOW_COPY_AND_ASSIGN(ServerWindowSurface); }; diff --git a/components/mus/ws/server_window_surface_manager.cc b/components/mus/ws/server_window_surface_manager.cc index 1699de4..83323d2 100644 --- a/components/mus/ws/server_window_surface_manager.cc +++ b/components/mus/ws/server_window_surface_manager.cc @@ -4,6 +4,7 @@ #include "components/mus/ws/server_window_surface_manager.h" +#include "components/mus/surfaces/surfaces_state.h" #include "components/mus/ws/server_window.h" #include "components/mus/ws/server_window_delegate.h" #include "components/mus/ws/server_window_surface.h" @@ -16,9 +17,15 @@ ServerWindowSurfaceManager::ServerWindowSurfaceManager(ServerWindow* window) surface_id_allocator_(WindowIdToTransportId(window->id())), waiting_for_initial_frames_( window_->properties().count(mus::mojom::kWaitForUnderlay_Property) > - 0) {} + 0) { + surface_id_allocator_.RegisterSurfaceIdNamespace(GetSurfaceManager()); +} -ServerWindowSurfaceManager::~ServerWindowSurfaceManager() {} +ServerWindowSurfaceManager::~ServerWindowSurfaceManager() { + // Explicitly clear the type to surface manager so that this manager + // is still valid prior during ~ServerWindowSurface. + type_to_surface_map_.clear(); +} bool ServerWindowSurfaceManager::ShouldDraw() { if (!waiting_for_initial_frames_) @@ -34,8 +41,16 @@ void ServerWindowSurfaceManager::CreateSurface( mojom::SurfaceType surface_type, mojo::InterfaceRequest<mojom::Surface> request, mojom::SurfaceClientPtr client) { - type_to_surface_map_[surface_type] = make_scoped_ptr(new ServerWindowSurface( + scoped_ptr<ServerWindowSurface> surface(new ServerWindowSurface( this, surface_type, std::move(request), std::move(client))); + if (!HasAnySurface()) { + // Only one SurfaceFactoryClient can be registered per surface id namespace, + // so register the first one. Since all surfaces created by this manager + // represent the same window, the begin frame source can be shared by + // all surfaces created here. + surface->RegisterForBeginFrames(); + } + type_to_surface_map_[surface_type] = std::move(surface); } ServerWindowSurface* ServerWindowSurfaceManager::GetDefaultSurface() { @@ -52,10 +67,19 @@ ServerWindowSurface* ServerWindowSurfaceManager::GetSurfaceByType( return iter == type_to_surface_map_.end() ? nullptr : iter->second.get(); } -bool ServerWindowSurfaceManager::HasSurfaceOfType(mojom::SurfaceType type) { +bool ServerWindowSurfaceManager::HasSurfaceOfType( + mojom::SurfaceType type) const { return type_to_surface_map_.count(type) > 0; } +bool ServerWindowSurfaceManager::HasAnySurface() { + return GetDefaultSurface() || GetUnderlaySurface(); +} + +cc::SurfaceManager* ServerWindowSurfaceManager::GetSurfaceManager() { + return window()->delegate()->GetSurfacesState()->manager(); +} + bool ServerWindowSurfaceManager::IsSurfaceReadyAndNonEmpty( mojom::SurfaceType type) const { auto iter = type_to_surface_map_.find(type); diff --git a/components/mus/ws/server_window_surface_manager.h b/components/mus/ws/server_window_surface_manager.h index a56827f..872e477 100644 --- a/components/mus/ws/server_window_surface_manager.h +++ b/components/mus/ws/server_window_surface_manager.h @@ -43,13 +43,17 @@ class ServerWindowSurfaceManager { ServerWindowSurface* GetDefaultSurface(); ServerWindowSurface* GetUnderlaySurface(); ServerWindowSurface* GetSurfaceByType(mojom::SurfaceType type); - bool HasSurfaceOfType(mojom::SurfaceType type); + bool HasSurfaceOfType(mojom::SurfaceType type) const; + bool HasAnySurface(); + + uint32_t id_namespace() const { return surface_id_allocator_.id_namespace(); } + cc::SurfaceManager* GetSurfaceManager(); private: friend class ServerWindowSurfaceManagerTestApi; friend class ServerWindowSurface; - // Returns true if a surface of |type| has been set and it's size is greater + // Returns true if a surface of |type| has been set and its size is greater // than the size of the window. bool IsSurfaceReadyAndNonEmpty(mojom::SurfaceType type) const; diff --git a/components/mus/ws/test_server_window_delegate.cc b/components/mus/ws/test_server_window_delegate.cc index 9a033b3..1660e78 100644 --- a/components/mus/ws/test_server_window_delegate.cc +++ b/components/mus/ws/test_server_window_delegate.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "components/mus/surfaces/surfaces_state.h" #include "components/mus/ws/test_server_window_delegate.h" #include "components/mus/ws/server_window.h" @@ -9,12 +10,13 @@ namespace mus { namespace ws { -TestServerWindowDelegate::TestServerWindowDelegate() : root_window_(nullptr) {} +TestServerWindowDelegate::TestServerWindowDelegate() + : root_window_(nullptr), surfaces_state_(new SurfacesState()) {} TestServerWindowDelegate::~TestServerWindowDelegate() {} mus::SurfacesState* TestServerWindowDelegate::GetSurfacesState() { - return nullptr; + return surfaces_state_.get(); } void TestServerWindowDelegate::OnScheduleWindowPaint(ServerWindow* window) {} diff --git a/components/mus/ws/test_server_window_delegate.h b/components/mus/ws/test_server_window_delegate.h index f7661e5..a452a01 100644 --- a/components/mus/ws/test_server_window_delegate.h +++ b/components/mus/ws/test_server_window_delegate.h @@ -33,6 +33,7 @@ class TestServerWindowDelegate : public ServerWindowDelegate { const ClientWindowId& client_window_id) override; const ServerWindow* root_window_; + scoped_refptr<mus::SurfacesState> surfaces_state_; DISALLOW_COPY_AND_ASSIGN(TestServerWindowDelegate); }; diff --git a/components/mus/ws/window_finder_unittest.cc b/components/mus/ws/window_finder_unittest.cc index f38d708..2d4d42f 100644 --- a/components/mus/ws/window_finder_unittest.cc +++ b/components/mus/ws/window_finder_unittest.cc @@ -27,7 +27,7 @@ TEST(WindowFinderTest, FindDeepestVisibleWindow) { child1.SetVisible(true); child1.SetBounds(gfx::Rect(10, 10, 20, 20)); - ServerWindow child2(&window_delegate, WindowId(1, 3)); + ServerWindow child2(&window_delegate, WindowId(1, 4)); root.Add(&child2); EnableHitTest(&child2); child2.SetVisible(true); diff --git a/components/mus/ws/window_tree_unittest.cc b/components/mus/ws/window_tree_unittest.cc index 17f59eb..e0b5eb1 100644 --- a/components/mus/ws/window_tree_unittest.cc +++ b/components/mus/ws/window_tree_unittest.cc @@ -163,7 +163,8 @@ class WindowTreeTest : public testing::Test { WindowTreeTest() : wm_client_(nullptr), cursor_id_(0), - platform_display_factory_(&cursor_id_) {} + platform_display_factory_(&cursor_id_), + surfaces_state_(new SurfacesState()) {} ~WindowTreeTest() override {} // WindowTree for the window manager. @@ -217,10 +218,10 @@ class WindowTreeTest : public testing::Test { void SetUp() override { PlatformDisplay::set_factory_for_testing(&platform_display_factory_); connection_manager_.reset( - new ConnectionManager(&delegate_, scoped_refptr<SurfacesState>())); + new ConnectionManager(&delegate_, surfaces_state_)); display_ = new Display(connection_manager_.get(), nullptr, scoped_refptr<GpuState>(), - scoped_refptr<mus::SurfacesState>()); + surfaces_state_); display_binding_ = new TestDisplayBinding(display_, connection_manager_.get()); display_->Init(make_scoped_ptr(display_binding_)); @@ -236,6 +237,7 @@ class WindowTreeTest : public testing::Test { // Owned by ConnectionManager. TestDisplayBinding* display_binding_; Display* display_ = nullptr; + scoped_refptr<SurfacesState> surfaces_state_; scoped_ptr<ConnectionManager> connection_manager_; base::MessageLoop message_loop_; diff --git a/content/browser/compositor/delegated_frame_host.cc b/content/browser/compositor/delegated_frame_host.cc index 7686744..15288bf 100644 --- a/content/browser/compositor/delegated_frame_host.cc +++ b/content/browser/compositor/delegated_frame_host.cc @@ -71,10 +71,13 @@ DelegatedFrameHost::DelegatedFrameHost(DelegatedFrameHostClient* client) skipped_frames_(false), current_scale_factor_(1.f), can_lock_compositor_(YES_CAN_LOCK), - delegated_frame_evictor_(new DelegatedFrameEvictor(this)) { + delegated_frame_evictor_(new DelegatedFrameEvictor(this)), + begin_frame_source_(nullptr) { ImageTransportFactory* factory = ImageTransportFactory::GetInstance(); factory->AddObserver(this); id_allocator_ = factory->GetContextFactory()->CreateSurfaceIdAllocator(); + factory->GetSurfaceManager()->RegisterSurfaceFactoryClient( + id_allocator_->id_namespace(), this); } void DelegatedFrameHost::WasShown(const ui::LatencyInfo& latency_info) { @@ -512,9 +515,9 @@ void DelegatedFrameHost::WillDrawSurface(cc::SurfaceId id, } void DelegatedFrameHost::SetBeginFrameSource( - cc::SurfaceId surface_id, cc::BeginFrameSource* begin_frame_source) { - // TODO(tansell): Hook this up. + // TODO(enne): forward this to DelegatedFrameHostClient to observe and then to + // the renderer as an external begin frame source. } void DelegatedFrameHost::EvictDelegatedFrame() { @@ -762,10 +765,13 @@ void DelegatedFrameHost::OnLostResources() { DelegatedFrameHost::~DelegatedFrameHost() { DCHECK(!compositor_); - ImageTransportFactory::GetInstance()->RemoveObserver(this); + ImageTransportFactory* factory = ImageTransportFactory::GetInstance(); + factory->RemoveObserver(this); if (!surface_id_.is_null()) surface_factory_->Destroy(surface_id_); + factory->GetSurfaceManager()->UnregisterSurfaceFactoryClient( + id_allocator_->id_namespace()); DCHECK(!vsync_manager_.get()); } @@ -779,6 +785,11 @@ void DelegatedFrameHost::SetCompositor(ui::Compositor* compositor) { DCHECK(!vsync_manager_.get()); vsync_manager_ = compositor_->vsync_manager(); vsync_manager_->AddObserver(this); + + ImageTransportFactory* factory = ImageTransportFactory::GetInstance(); + uint32_t parent = compositor->surface_id_allocator()->id_namespace(); + factory->GetSurfaceManager()->RegisterSurfaceNamespaceHierarchy( + parent, id_allocator_->id_namespace()); } void DelegatedFrameHost::ResetCompositor() { @@ -794,6 +805,12 @@ void DelegatedFrameHost::ResetCompositor() { vsync_manager_->RemoveObserver(this); vsync_manager_ = NULL; } + + ImageTransportFactory* factory = ImageTransportFactory::GetInstance(); + uint32_t parent = compositor_->surface_id_allocator()->id_namespace(); + factory->GetSurfaceManager()->UnregisterSurfaceNamespaceHierarchy( + parent, id_allocator_->id_namespace()); + compositor_ = nullptr; } diff --git a/content/browser/compositor/delegated_frame_host.h b/content/browser/compositor/delegated_frame_host.h index e0ac4c0..5cb9301 100644 --- a/content/browser/compositor/delegated_frame_host.h +++ b/content/browser/compositor/delegated_frame_host.h @@ -116,8 +116,7 @@ class CONTENT_EXPORT DelegatedFrameHost // cc::SurfaceFactoryClient implementation. void ReturnResources(const cc::ReturnedResourceArray& resources) override; void WillDrawSurface(cc::SurfaceId id, const gfx::Rect& damage_rect) override; - void SetBeginFrameSource(cc::SurfaceId surface_id, - cc::BeginFrameSource* begin_frame_source) override; + void SetBeginFrameSource(cc::BeginFrameSource* begin_frame_source) override; bool CanCopyToBitmap() const; @@ -307,6 +306,8 @@ class CONTENT_EXPORT DelegatedFrameHost yuv_readback_pipeline_; scoped_ptr<DelegatedFrameEvictor> delegated_frame_evictor_; + + cc::BeginFrameSource* begin_frame_source_; }; } // namespace content diff --git a/content/browser/frame_host/render_widget_host_view_child_frame.cc b/content/browser/frame_host/render_widget_host_view_child_frame.cc index 7c60d9c..7c26bf8 100644 --- a/content/browser/frame_host/render_widget_host_view_child_frame.cc +++ b/content/browser/frame_host/render_widget_host_view_child_frame.cc @@ -561,7 +561,6 @@ void RenderWidgetHostViewChildFrame::ReturnResources( } void RenderWidgetHostViewChildFrame::SetBeginFrameSource( - cc::SurfaceId surface_id, cc::BeginFrameSource* begin_frame_source) { // TODO(tansell): Hook this up. } diff --git a/content/browser/frame_host/render_widget_host_view_child_frame.h b/content/browser/frame_host/render_widget_host_view_child_frame.h index f6cba54..45bc03b1 100644 --- a/content/browser/frame_host/render_widget_host_view_child_frame.h +++ b/content/browser/frame_host/render_widget_host_view_child_frame.h @@ -175,8 +175,7 @@ class CONTENT_EXPORT RenderWidgetHostViewChildFrame // cc::SurfaceFactoryClient implementation. void ReturnResources(const cc::ReturnedResourceArray& resources) override; - void SetBeginFrameSource(cc::SurfaceId surface_id, - cc::BeginFrameSource* begin_frame_source) override; + void SetBeginFrameSource(cc::BeginFrameSource* begin_frame_source) override; // Declared 'public' instead of 'protected' here to allow derived classes // to Bind() to it. diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc index 72d1ac1..b50408e 100644 --- a/content/browser/renderer_host/render_widget_host_view_android.cc +++ b/content/browser/renderer_host/render_widget_host_view_android.cc @@ -967,7 +967,6 @@ void RenderWidgetHostViewAndroid::ReturnResources( } void RenderWidgetHostViewAndroid::SetBeginFrameSource( - cc::SurfaceId surface_id, cc::BeginFrameSource* begin_frame_source) { // TODO(tansell): Hook this up. } diff --git a/content/browser/renderer_host/render_widget_host_view_android.h b/content/browser/renderer_host/render_widget_host_view_android.h index ae3d606..5407cc7 100644 --- a/content/browser/renderer_host/render_widget_host_view_android.h +++ b/content/browser/renderer_host/render_widget_host_view_android.h @@ -169,8 +169,7 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid // cc::SurfaceFactoryClient implementation. void ReturnResources(const cc::ReturnedResourceArray& resources) override; - void SetBeginFrameSource(cc::SurfaceId surface_id, - cc::BeginFrameSource* begin_frame_source) override; + void SetBeginFrameSource(cc::BeginFrameSource* begin_frame_source) override; // ui::GestureProviderClient implementation. void OnGestureEvent(const ui::GestureEventData& gesture) override; |