Hook up BeginFrameSource to SurfaceFactoryClient via SurfaceManager

cc/surfaces/surface_manager.cc

 // Copyright 2014 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 "cc/surfaces/surface_manager.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #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();
 }
 
 SurfaceManager::~SurfaceManager() {
   DCHECK(thread_checker_.CalledOnValidThread());
   for (SurfaceDestroyList::iterator it = surfaces_to_destroy_.begin();
        it != surfaces_to_destroy_.end();
        ++it) {
     DeregisterSurface((*it)->surface_id());
     delete *it;
   }
 }
 
 void SurfaceManager::RegisterSurface(Surface* surface) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(surface);
   DCHECK(!surface_map_.count(surface->surface_id()));
   surface_map_[surface->surface_id()] = surface;
+  // TODO(enne): verify surface namespace is registered
 }
 
 void SurfaceManager::DeregisterSurface(SurfaceId surface_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
   SurfaceMap::iterator it = surface_map_.find(surface_id);
   DCHECK(it != surface_map_.end());
   surface_map_.erase(it);
+  // TODO(enne): verify surface namespace is registered
 }
 
 void SurfaceManager::Destroy(scoped_ptr<Surface> surface) {
   DCHECK(thread_checker_.CalledOnValidThread());
   surface->set_destroyed(true);
   surfaces_to_destroy_.push_back(surface.release());
   GarbageCollectSurfaces();
 }
 
 void SurfaceManager::DidSatisfySequences(uint32_t id_namespace,
@@ skipping 61 matching lines @@
     if (!live_surfaces_set.count((*dest_it)->surface_id())) {
       scoped_ptr<Surface> surf(*dest_it);
       DeregisterSurface(surf->surface_id());
       dest_it = surfaces_to_destroy_.erase(dest_it);
     } else {
       ++dest_it;
     }
   }
 }
 
+void SurfaceManager::RegisterSurfaceFactoryClient(
+    uint32_t id_namespace,
+    SurfaceFactoryClient* client) {

[brianderson, 2016/02/18 23:01:07]

DCHECK namespace is valid?

+  if (client) {
+    DCHECK(!namespace_client_map_[id_namespace].client);
+    namespace_client_map_[id_namespace].client = client;
+    if (BeginFrameSource* source = namespace_client_map_[id_namespace].source) {

[brianderson, 2016/02/18 23:01:07]

Cache map lookup?

+      client->SetBeginFrameSource(source);
+    }
+  } else {
+    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 begin frame source should be removed prior to removing this
+    // client.
+    for (auto iter : registered_sources_)

[brianderson, 2016/02/18 23:01:07]

Maybe use a name other than "iter".

+      DCHECK_NE(iter.second, id_namespace);
+
+    if (iter->second.is_empty())
+      namespace_client_map_.erase(iter);

[brianderson, 2016/02/18 23:01:07]

Does it make a difference whether this is erased here vs. in
UnregisterSurfaceIdNamespace? If it were there, could you DCHECK is_empty()
instead?

+  }
+}
+
+void SurfaceManager::RegisterBeginFrameSource(BeginFrameSource* source,
+                                              uint32_t id_namespace) {
+  DCHECK(source);
+  DCHECK_EQ(namespace_client_map_.count(id_namespace), 1u);
+  DCHECK_EQ(registered_sources_.count(source), 0u);
+
+  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];
+  DCHECK_EQ(namespace_client_map_.count(id_namespace), 1u);
+  registered_sources_.erase(source);
+
+  // 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 set anything null.
+  for (auto iter : registered_sources_)

[brianderson, 2016/02/18 23:01:07]

Nice.

+    RecursivelyAttachBeginFrameSource(iter.second, iter.first);
+}
+
+void SurfaceManager::RecursivelyAttachBeginFrameSource(
+    uint32_t id_namespace,
+    BeginFrameSource* source) {
+  auto& data = namespace_client_map_[id_namespace];

[jbauman, 2016/02/24 19:52:34]

One thing we'll have to be careful of is to make sure we don't have infinite
recursion here, either by keeping a set of namespaces that we've already looked
at in the current recursion, or by just being certain
RegisterSurfaceNamespaceHierarchy is only ever called by trusted code.

[enne (OOO), 2016/02/24 21:19:03]

Mmm, that's a really good point.  Rather than keeping a list, I *think* it would
be sufficient to just pass along the parent of the newly added hierarchy into
the recursion, and if that parent is not seen in the recursion, then no new
loops have been added.

+  if (!data.source) {

[brianderson, 2016/02/18 23:01:07]

Can you add a comment regarding the purpose of this if? It's not immediately
obvious without understanding how UnregisterBeginFrameSource works.

+    data.source = source;
+    if (data.client)
+      data.client->SetBeginFrameSource(source);
+  }
+  for (size_t i = 0; i < data.children.size(); ++i) {
+    RecursivelyAttachBeginFrameSource(data.children[i], source);
+  }
+}
+
+void SurfaceManager::RecursivelyDetachBeginFrameSource(
+    uint32_t id_namespace,
+    BeginFrameSource* source) {

[brianderson, 2016/02/18 23:01:07]

Maybe add some DCHECKS like you have in the other methods.

+  auto iter = namespace_client_map_.find(id_namespace);
+  if (iter->second.source == source) {
+    iter->second.source = nullptr;
+    if (iter->second.client)
+      iter->second.client->SetBeginFrameSource(nullptr);
+  }
+  std::vector<uint32_t>& children = iter->second.children;
+  for (size_t i = 0; i < children.size(); ++i) {
+    RecursivelyDetachBeginFrameSource(children[i], source);
+  }
+}
+
+void SurfaceManager::RegisterSurfaceNamespaceHierarchy(
+    uint32_t parent_namespace,
+    uint32_t child_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);
+
+  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(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);
+
+  BeginFrameSource* parent_source = iter->second.source;
+  if (!parent_source)
+    return;
+
+  if (iter->second.is_empty())
+    namespace_client_map_.erase(iter);
+
+  // TODO(enne): these walks could be done in one step.
+  RecursivelyDetachBeginFrameSource(child_namespace, parent_source);
+  for (auto iter : registered_sources_)
+    RecursivelyAttachBeginFrameSource(iter.second, iter.first);
+}
+
 Surface* SurfaceManager::GetSurfaceForId(SurfaceId surface_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
   SurfaceMap::iterator it = surface_map_.find(surface_id);
   if (it == surface_map_.end())
     return NULL;
   return it->second;
 }
 
 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));
   return changed;
 }
 
 }  // namespace cc