diff options
author | enne <enne@chromium.org> | 2016-03-08 16:25:12 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-03-09 00:26:06 +0000 |
commit | 4e3c9d5b507f8a7cfb99ef3cc855184943a1dbd8 (patch) | |
tree | e37866af865af000937b9f5e75815883260115e3 | |
parent | 09361b7a2000298da27d831553547eed54a6f894 (diff) | |
download | chromium_src-4e3c9d5b507f8a7cfb99ef3cc855184943a1dbd8.zip chromium_src-4e3c9d5b507f8a7cfb99ef3cc855184943a1dbd8.tar.gz chromium_src-4e3c9d5b507f8a7cfb99ef3cc855184943a1dbd8.tar.bz2 |
Hook up BeginFrameSource to SurfaceFactoryClient via SurfaceManager
SurfaceManager now maintains a dag of surface id namespaces.
Optionally, a single BeginFrameSource input can be attached to a single
namespace node. Every namespace node also has a SurfaceFactoryClient.
This client is informed of a current BeginFrameSource, which is chosen
from any BeginFrameSource attached to it or a parent of that node.
Any children of that namespace also are able to use that source.
SurfaceManager is responsible for picking which source to use, of which
it currently just picks the first one until that source goes is removed
after which it arbitrarily picks another valid one. In practice, this
means that a window moved to another display in ChromeOS will switch its
BeginFrameSource after the window is dropped onto the new window.
Because the users of this dag all have very different requirements, the
ordering of SurfaceFactoryClient registration, namespace hierarchy
registration, and BeginFrameSource attaching are not particularly
strict. BeginFrameSources, SurfaceFactoryClients, and hierarchies can
be registered and unregistered in any order with respect to each other.
BUG=401331
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
Review URL: https://codereview.chromium.org/1673783004
Cr-Commit-Position: refs/heads/master@{#379988}
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; |