DirectCompositorFrameSink Uses CompositorFrameSinkSupport

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 102 matching lines @@
 }
 
 void SurfaceManager::SatisfySequence(const SurfaceSequence& sequence) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_EQ(lifetime_type_, LifetimeType::SEQUENCES);
   satisfied_sequences_.insert(sequence);
   GarbageCollectSurfaces();
 }
 
 void SurfaceManager::RegisterFrameSinkId(const FrameSinkId& frame_sink_id) {
-  bool inserted = valid_frame_sink_ids_.insert(frame_sink_id).second;
-  DCHECK(inserted);
+  valid_frame_sink_ids_.insert(frame_sink_id);

[danakj, 2017/01/31 20:19:17]

Why this change?

[Alex Z., 2017/01/31 21:09:30]

From fsamuel's earlier comment:

ui::Compositor Registers the FrameSinkId first:
https://cs.chromium.org/chromium/src/ui/compositor/compositor.cc?sq=package:c...

However, we eventually want to get rid of that in favor of the SurfaceReference
approach to reference tracking. We're not there yet. For the time being, we are
registering the FrameSinkId in CompositorFrameSinkSupport. However, while we're
using SurfaceSequences, we cannot get rid of RegisterFrameSinkId in
ui::Compositor, so I thought getting rid of the DCHECK would be easier for now.

Note that once SurfaceReferences are used everywhere, RegisterFrameSinkId and
InvalidateFrameSinkId will likely go away.

[danakj, 2017/01/31 21:49:40]

Can we have a parameter passed around maybe saying to use refs or surfaces. And
ui::Compositor would register if appropriate, and CompositorFrameSinkSupport
would if appropriate? Or would that be insane?
FWIW, likely shouldn't motivate code too much, as it's also likely to change.

[danakj, 2017/02/03 18:48:43]

I'm not seeing anything in the replies to this, did I miss it? And how would we
ensure this check came back in a timely manner if we remove it now?

[Alex Z., 2017/02/03 19:32:20]

I added a flag to CompositorFrameSinkSupport in patchset 15 where 
CompositorFrameSinkSupport wouldn't register or invalidate 
FrameSinkId if it's supporting DCFS. fsamuel wasn't happy about 
using the flag as he'd like to go with the design where CFSSupport 
manages FrameSinkId. 

Would adding a "TODO: DCHECK if the insert is successful." be enough assurance?

 }
 
 void SurfaceManager::InvalidateFrameSinkId(const FrameSinkId& frame_sink_id) {
   valid_frame_sink_ids_.erase(frame_sink_id);
   GarbageCollectSurfaces();
 }
 
 const SurfaceId& SurfaceManager::GetRootSurfaceId() const {
   return root_surface_id_;
 }
@@ skipping 479 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