Surface Synchronization: Distinguish between dependencies and references

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 <queue>
@@ skipping 301 matching lines @@
 }
 
 void SurfaceManager::AddSurfaceReferenceImpl(const SurfaceId& parent_id,
                                              const SurfaceId& child_id) {
   if (parent_id.frame_sink_id() == child_id.frame_sink_id()) {
     DLOG(ERROR) << "Cannot add self reference from " << parent_id << " to "
                 << child_id;
     return;
   }
 
+  // We trust that |parent_id| either exists or is about to exist, since is not
+  // sent over IPC. We don't trust |child_id|, since it is sent over IPC.
+  if (surface_map_.count(child_id) == 0) {
+    DLOG(ERROR) << "No surface in map for " << child_id.ToString();
+    return;
+  }
+
   parent_to_child_refs_[parent_id].insert(child_id);
   child_to_parent_refs_[child_id].insert(parent_id);
 
   if (HasTemporaryReference(child_id))
     RemoveTemporaryReference(child_id, true);
 }
 
 void SurfaceManager::RemoveSurfaceReferenceImpl(const SurfaceId& parent_id,
                                                 const SurfaceId& child_id) {
   if (parent_to_child_refs_.count(parent_id) == 0 ||
@@ skipping 122 matching lines @@
 void SurfaceManager::SurfaceCreated(const SurfaceInfo& surface_info) {
   CHECK(thread_checker_.CalledOnValidThread());
 
   if (lifetime_type_ == LifetimeType::REFERENCES) {
     // We can get into a situation where multiple CompositorFrames arrive for
     // a CompositorFrameSink before the client can add any references for the
     // frame. When the second frame with a new size arrives, the first will be
     // destroyed in SurfaceFactory and then if there are no references it will
     // be deleted during surface GC. A temporary reference, removed when a
     // real reference is received, is added to prevent this from happening.
-    auto it = child_to_parent_refs_.find(surface_info.id());
-    // TODO(fsamuel): Tests create empty sets and so we also need to check that
-    // they're not empty here. Ideally tests shouldn't do that and we shouldn't
-    // use array notation into maps in tests (see https://crbug.com/691115).
-    bool has_real_reference =
-        it != child_to_parent_refs_.end() && !it->second.empty();
-    if (!has_real_reference)
-      AddTemporaryReference(surface_info.id());
+    AddTemporaryReference(surface_info.id());
   }
 
   for (auto& observer : observer_list_)
     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());
@@ skipping 42 matching lines @@
     std::vector<SurfaceId> children(iter->second.begin(), iter->second.end());
     std::sort(children.begin(), children.end());
 
     for (const SurfaceId& child_id : children)
       SurfaceReferencesToStringImpl(child_id, indent + "  ", str);
   }
 }
 #endif  // DCHECK_IS_ON()
 
 }  // namespace cc