Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3694)

Unified Diff: cc/surfaces/surface_manager.cc

Issue 2684933003: Move frame_sink_id management to framesink_manager.cc/h from (Closed)
Patch Set: Addressed CR comments Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « cc/surfaces/surface_manager.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: cc/surfaces/surface_manager.cc
diff --git a/cc/surfaces/surface_manager.cc b/cc/surfaces/surface_manager.cc
index 64a180f6b1071b9a4c8702623235bef18738b584..f6ecbb8997ec06bb22d673b61e3d5687c7fe5400 100644
--- a/cc/surfaces/surface_manager.cc
+++ b/cc/surfaces/surface_manager.cc
@@ -23,15 +23,6 @@
namespace cc {
-SurfaceManager::FrameSinkSourceMapping::FrameSinkSourceMapping()
- : source(nullptr) {}
-
-SurfaceManager::FrameSinkSourceMapping::FrameSinkSourceMapping(
- const FrameSinkSourceMapping& other) = default;
-
-SurfaceManager::FrameSinkSourceMapping::~FrameSinkSourceMapping() {
-}
-
SurfaceManager::SurfaceManager(LifetimeType lifetime_type)
: lifetime_type_(lifetime_type),
root_surface_id_(FrameSinkId(0u, 0u),
@@ -59,11 +50,6 @@ SurfaceManager::~SurfaceManager() {
UnregisterSurface((*it)->surface_id());
}
surfaces_to_destroy_.clear();
-
- // All surface factory clients should be unregistered prior to SurfaceManager
- // destruction.
- DCHECK_EQ(clients_.size(), 0u);
- DCHECK_EQ(registered_sources_.size(), 0u);
}
#if DCHECK_IS_ON()
@@ -163,12 +149,11 @@ void SurfaceManager::SatisfySequence(const SurfaceSequence& sequence) {
}
void SurfaceManager::RegisterFrameSinkId(const FrameSinkId& frame_sink_id) {
- bool inserted = valid_frame_sink_ids_.insert(frame_sink_id).second;
- DCHECK(inserted);
+ framesink_manager_.RegisterFrameSinkId(frame_sink_id);
}
void SurfaceManager::InvalidateFrameSinkId(const FrameSinkId& frame_sink_id) {
- valid_frame_sink_ids_.erase(frame_sink_id);
+ framesink_manager_.InvalidateFrameSinkId(frame_sink_id);
if (using_surface_references()) {
// Remove any temporary references owned by |frame_sink_id|.
@@ -304,7 +289,7 @@ SurfaceManager::SurfaceIdSet SurfaceManager::GetLiveSurfacesForSequences() {
const SurfaceId& surface_id = map_entry.first;
Surface* surface = map_entry.second;
surface->SatisfyDestructionDependencies(&satisfied_sequences_,
- &valid_frame_sink_ids_);
+ framesink_manager_.GetValidFrameSinkIds());
if (!surface->destroyed() || surface->GetDestructionDependencyCount() > 0) {
live_surfaces_set.insert(surface_id);
@@ -430,186 +415,36 @@ void SurfaceManager::RemoveTemporaryReference(const SurfaceId& surface_id,
void SurfaceManager::RegisterSurfaceFactoryClient(
const FrameSinkId& frame_sink_id,
SurfaceFactoryClient* client) {
- DCHECK(client);
- DCHECK_EQ(valid_frame_sink_ids_.count(frame_sink_id), 1u);
-
- clients_[frame_sink_id] = client;
-
- auto it = frame_sink_source_map_.find(frame_sink_id);
- if (it != frame_sink_source_map_.end()) {
- if (it->second.source)
- client->SetBeginFrameSource(it->second.source);
- }
+ framesink_manager_.RegisterSurfaceFactoryClient(frame_sink_id, client);
}
void SurfaceManager::UnregisterSurfaceFactoryClient(
const FrameSinkId& frame_sink_id) {
- DCHECK_EQ(valid_frame_sink_ids_.count(frame_sink_id), 1u);
- auto client_iter = clients_.find(frame_sink_id);
- DCHECK(client_iter != clients_.end());
-
- auto source_iter = frame_sink_source_map_.find(frame_sink_id);
- if (source_iter != frame_sink_source_map_.end()) {
- if (source_iter->second.source)
- client_iter->second->SetBeginFrameSource(nullptr);
- if (!source_iter->second.has_children())
- frame_sink_source_map_.erase(source_iter);
- }
- clients_.erase(client_iter);
+ framesink_manager_.UnregisterSurfaceFactoryClient(frame_sink_id);
}
void SurfaceManager::RegisterBeginFrameSource(
BeginFrameSource* source,
const FrameSinkId& frame_sink_id) {
- DCHECK(source);
- DCHECK_EQ(registered_sources_.count(source), 0u);
- DCHECK_EQ(valid_frame_sink_ids_.count(frame_sink_id), 1u);
-
- registered_sources_[source] = frame_sink_id;
- RecursivelyAttachBeginFrameSource(frame_sink_id, source);
+ framesink_manager_.RegisterBeginFrameSource(source, frame_sink_id);
}
void SurfaceManager::UnregisterBeginFrameSource(BeginFrameSource* source) {
- DCHECK(source);
- DCHECK_EQ(registered_sources_.count(source), 1u);
-
- FrameSinkId frame_sink_id = registered_sources_[source];
- registered_sources_.erase(source);
-
- if (frame_sink_source_map_.count(frame_sink_id) == 0u)
- return;
-
- // TODO(enne): these walks could be done in one step.
- // Remove this begin frame source from its subtree.
- RecursivelyDetachBeginFrameSource(frame_sink_id, 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(
- const FrameSinkId& frame_sink_id,
- BeginFrameSource* source) {
- FrameSinkSourceMapping& mapping = frame_sink_source_map_[frame_sink_id];
- if (!mapping.source) {
- mapping.source = source;
- auto client_iter = clients_.find(frame_sink_id);
- if (client_iter != clients_.end())
- client_iter->second->SetBeginFrameSource(source);
- }
- for (size_t i = 0; i < mapping.children.size(); ++i)
- RecursivelyAttachBeginFrameSource(mapping.children[i], source);
-}
-
-void SurfaceManager::RecursivelyDetachBeginFrameSource(
- const FrameSinkId& frame_sink_id,
- BeginFrameSource* source) {
- auto iter = frame_sink_source_map_.find(frame_sink_id);
- if (iter == frame_sink_source_map_.end())
- return;
- if (iter->second.source == source) {
- iter->second.source = nullptr;
- auto client_iter = clients_.find(frame_sink_id);
- if (client_iter != clients_.end())
- client_iter->second->SetBeginFrameSource(nullptr);
- }
-
- if (!iter->second.has_children() && !clients_.count(frame_sink_id)) {
- frame_sink_source_map_.erase(iter);
- return;
- }
-
- std::vector<FrameSinkId>& children = iter->second.children;
- for (size_t i = 0; i < children.size(); ++i) {
- RecursivelyDetachBeginFrameSource(children[i], source);
- }
-}
-
-bool SurfaceManager::ChildContains(
- const FrameSinkId& child_frame_sink_id,
- const FrameSinkId& search_frame_sink_id) const {
- auto iter = frame_sink_source_map_.find(child_frame_sink_id);
- if (iter == frame_sink_source_map_.end())
- return false;
-
- const std::vector<FrameSinkId>& children = iter->second.children;
- for (size_t i = 0; i < children.size(); ++i) {
- if (children[i] == search_frame_sink_id)
- return true;
- if (ChildContains(children[i], search_frame_sink_id))
- return true;
- }
- return false;
+ framesink_manager_.UnregisterBeginFrameSource(source);
}
void SurfaceManager::RegisterFrameSinkHierarchy(
const FrameSinkId& parent_frame_sink_id,
const FrameSinkId& child_frame_sink_id) {
- // 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_frame_sink_id, parent_frame_sink_id));
-
- std::vector<FrameSinkId>& children =
- frame_sink_source_map_[parent_frame_sink_id].children;
- for (size_t i = 0; i < children.size(); ++i)
- DCHECK(children[i] != child_frame_sink_id);
- children.push_back(child_frame_sink_id);
-
- // If the parent has no source, then attaching it to this child will
- // not change any downstream sources.
- BeginFrameSource* parent_source =
- frame_sink_source_map_[parent_frame_sink_id].source;
- if (!parent_source)
- return;
-
- DCHECK_EQ(registered_sources_.count(parent_source), 1u);
- RecursivelyAttachBeginFrameSource(child_frame_sink_id, parent_source);
+ framesink_manager_.RegisterFrameSinkHierarchy(parent_frame_sink_id,
+ child_frame_sink_id);
}
void SurfaceManager::UnregisterFrameSinkHierarchy(
const FrameSinkId& parent_frame_sink_id,
const FrameSinkId& child_frame_sink_id) {
- // Deliberately do not check validity of either parent or child FrameSinkId
- // here. They were valid during the registration, so were valid at some
- // point in time. This makes it possible to invalidate parent and child
- // FrameSinkIds independently of each other and not have an ordering
- // dependency of unregistering the hierarchy first before either of them.
- DCHECK_EQ(frame_sink_source_map_.count(parent_frame_sink_id), 1u);
-
- auto iter = frame_sink_source_map_.find(parent_frame_sink_id);
-
- std::vector<FrameSinkId>& children = iter->second.children;
- bool found_child = false;
- for (size_t i = 0; i < children.size(); ++i) {
- if (children[i] == child_frame_sink_id) {
- found_child = true;
- children[i] = children.back();
- children.resize(children.size() - 1);
- break;
- }
- }
- DCHECK(found_child);
-
- // The SurfaceFactoryClient and hierarchy can be registered/unregistered
- // in either order, so empty frame_sink_source_map entries need to be
- // checked when removing either clients or relationships.
- if (!iter->second.has_children() && !clients_.count(parent_frame_sink_id) &&
- !iter->second.source) {
- frame_sink_source_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_frame_sink_id, parent_source);
- for (auto source_iter : registered_sources_)
- RecursivelyAttachBeginFrameSource(source_iter.second, source_iter.first);
+ framesink_manager_.UnregisterFrameSinkHierarchy(parent_frame_sink_id,
+ child_frame_sink_id);
}
Surface* SurfaceManager::GetSurfaceForId(const SurfaceId& surface_id) {
« no previous file with comments | « cc/surfaces/surface_manager.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698