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

Unified Diff: cc/surfaces/surface_manager.cc

Issue 2736053004: SurfaceIds must be reusable as soon as their surfaces are marked destroyed (Closed)
Patch Set: Make deregister private 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 bf72fce1f3a30bc5497190135fcabe9dcb252791..753191d7460be547e751b117afe584ad3734c89b 100644
--- a/cc/surfaces/surface_manager.cc
+++ b/cc/surfaces/surface_manager.cc
@@ -56,7 +56,7 @@ SurfaceManager::~SurfaceManager() {
for (SurfaceDestroyList::iterator it = surfaces_to_destroy_.begin();
it != surfaces_to_destroy_.end();
++it) {
- DeregisterSurface((*it)->surface_id());
+ UnregisterSurface((*it)->surface_id());
}
surfaces_to_destroy_.clear();
@@ -88,22 +88,45 @@ void SurfaceManager::RequestSurfaceResolution(Surface* pending_surface) {
dependency_tracker_->RequestSurfaceResolution(pending_surface);
}
-void SurfaceManager::RegisterSurface(Surface* surface) {
+std::unique_ptr<Surface> SurfaceManager::CreateSurface(
+ base::WeakPtr<SurfaceFactory> surface_factory,
+ const LocalSurfaceId& local_surface_id) {
DCHECK(thread_checker_.CalledOnValidThread());
- DCHECK(surface);
- DCHECK(!surface_map_.count(surface->surface_id()));
- surface_map_[surface->surface_id()] = surface;
-}
+ DCHECK(local_surface_id.is_valid() && surface_factory);
-void SurfaceManager::DeregisterSurface(const SurfaceId& surface_id) {
- DCHECK(thread_checker_.CalledOnValidThread());
- SurfaceMap::iterator it = surface_map_.find(surface_id);
- DCHECK(it != surface_map_.end());
- surface_map_.erase(it);
- RemoveAllSurfaceReferences(surface_id);
-}
+ SurfaceId surface_id(surface_factory->frame_sink_id(), local_surface_id);
+
+ // If no surface with this SurfaceId exists, simply create the surface and
+ // return.
+ auto surface_iter = surface_map_.find(surface_id);
+ if (surface_iter == surface_map_.end()) {
+ auto surface = base::MakeUnique<Surface>(surface_id, surface_factory);
+ surface_map_[surface->surface_id()] = surface.get();
+ return surface;
+ }
-void SurfaceManager::Destroy(std::unique_ptr<Surface> surface) {
+ // If a surface with this SurfaceId exists and it's not marked as destroyed,
+ // we should not receive a request to create a new surface with the same
+ // SurfaceId.
+ DCHECK(surface_iter->second->destroyed());
+
+ // If a surface with this SurfaceId exists and it's marked as destroyed,
+ // it means it's in the garbage collector's queue. We simply take it out of
+ // the queue and reuse it.
+ auto it =
+ std::find_if(surfaces_to_destroy_.begin(), surfaces_to_destroy_.end(),
+ [&surface_id](const std::unique_ptr<Surface>& surface) {
+ return surface->surface_id() == surface_id;
+ });
+ DCHECK(it != surfaces_to_destroy_.end());
+ std::unique_ptr<Surface> surface = std::move(*it);
+ surfaces_to_destroy_.erase(it);
+ surface->set_destroyed(false);
+ DCHECK_EQ(surface_factory.get(), surface->factory().get());
+ return surface;
+}
+
+void SurfaceManager::DestroySurface(std::unique_ptr<Surface> surface) {
DCHECK(thread_checker_.CalledOnValidThread());
surface->set_destroyed(true);
surfaces_to_destroy_.push_back(std::move(surface));
@@ -211,7 +234,7 @@ void SurfaceManager::GarbageCollectSurfaces() {
iter != surfaces_to_destroy_.end();) {
SurfaceId surface_id = (*iter)->surface_id();
if (reachable_surfaces.count(surface_id) == 0) {
- DeregisterSurface(surface_id);
+ UnregisterSurface(surface_id);
surfaces_to_delete.push_back(std::move(*iter));
iter = surfaces_to_destroy_.erase(iter);
} else {
@@ -616,6 +639,14 @@ void SurfaceManager::SurfaceCreated(const SurfaceInfo& surface_info) {
observer.OnSurfaceCreated(surface_info);
}
+void SurfaceManager::UnregisterSurface(const SurfaceId& surface_id) {
+ DCHECK(thread_checker_.CalledOnValidThread());
+ SurfaceMap::iterator it = surface_map_.find(surface_id);
+ DCHECK(it != surface_map_.end());
+ surface_map_.erase(it);
+ RemoveAllSurfaceReferences(surface_id);
+}
+
#if DCHECK_IS_ON()
void SurfaceManager::SurfaceReferencesToStringImpl(const SurfaceId& surface_id,
std::string indent,
« 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