DirectCompositorFrameSink Uses CompositorFrameSinkSupport

cc/surfaces/compositor_frame_sink_support.cc

 // Copyright 2016 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/compositor_frame_sink_support.h"
 
 #include "cc/output/compositor_frame.h"
 #include "cc/scheduler/begin_frame_source.h"
 #include "cc/surfaces/compositor_frame_sink_support_client.h"
 #include "cc/surfaces/display.h"
 #include "cc/surfaces/surface.h"
 #include "cc/surfaces/surface_manager.h"
 
 namespace cc {
 
 CompositorFrameSinkSupport::CompositorFrameSinkSupport(
     CompositorFrameSinkSupportClient* client,
     SurfaceManager* surface_manager,
     const FrameSinkId& frame_sink_id,
-    std::unique_ptr<Display> display,
-    std::unique_ptr<BeginFrameSource> display_begin_frame_source)
+    Display* display,
+    bool needs_sync_points)
     : client_(client),
       surface_manager_(surface_manager),
       frame_sink_id_(frame_sink_id),
-      display_begin_frame_source_(std::move(display_begin_frame_source)),
-      display_(std::move(display)),
+      display_(display),
       surface_factory_(frame_sink_id_, surface_manager_, this),
       weak_factory_(this) {
   surface_manager_->RegisterFrameSinkId(frame_sink_id_);
   surface_manager_->RegisterSurfaceFactoryClient(frame_sink_id_, this);
+  surface_factory_.set_needs_sync_points(needs_sync_points);
 
   if (display_) {
     display_->Initialize(this, surface_manager_);
     display_->SetVisible(true);
   }
 }
 
 CompositorFrameSinkSupport::~CompositorFrameSinkSupport() {
   for (auto& child_frame_sink_id : child_frame_sinks_) {
     DCHECK(child_frame_sink_id.is_valid());
@@ skipping 14 matching lines @@
 
 void CompositorFrameSinkSupport::SetNeedsBeginFrame(bool needs_begin_frame) {
   needs_begin_frame_ = needs_begin_frame;
   UpdateNeedsBeginFramesInternal();
 }
 
 void CompositorFrameSinkSupport::SubmitCompositorFrame(
     const LocalSurfaceId& local_surface_id,
     CompositorFrame frame) {
   ++ack_pending_count_;
+  if (display_) {

[danakj, 2017/01/31 19:25:01]

why this reordering?

[Alex Z., 2017/01/31 20:34:18]

The reordering was to fix this crash in the state_machine:
https://paste.googleplex.com/5211561517907968

[danakj, 2017/01/31 20:41:38]

That crash is from CompositorFrameSinkSupport::SubmitCompositorFrame and
Scheduler::DidReceiveCompositorFrameAck happening on the same stack frame right?
Normally we prevent that by posting acks back. How does this fix it and why is
it the right way to fix?

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

I don't know. I did the reordering because SetLocalSurfaceId() is 
called before SubmitCompositorFrame() in 
DirectCompositorFrameSink::SubmitCompositorFrame() currently, and that 
fixed the crash.

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

Er, well, I don't think that's a very good motivation for reordering them :/ Can
you look at the call stacks and explain why this fixes it for me? Or look at the
history there maybe it would explain. If you're stuck I can help out lmk.

[Alex Z., 2017/02/02 00:17:32]

Like we've discussed on slack,  CompositorFrameSinkSupport got away 
with a weird ordering of SetLocalSurfaceId / SubmitCompositorFrame 
because we only used that code in Mus Window Server, and it did 
nothing in DidReceiveCompositorFrameAck.

+    display_->SetLocalSurfaceId(local_surface_id,
+                                frame.metadata.device_scale_factor);
+  }
   surface_factory_.SubmitCompositorFrame(
       local_surface_id, std::move(frame),
       base::Bind(&CompositorFrameSinkSupport::DidReceiveCompositorFrameAck,
                  weak_factory_.GetWeakPtr()));
-  if (display_) {
-    display_->SetLocalSurfaceId(local_surface_id,
-                                frame.metadata.device_scale_factor);
-  }
 }
 
 void CompositorFrameSinkSupport::Require(const LocalSurfaceId& local_surface_id,
                                          const SurfaceSequence& sequence) {
   surface_manager_->RequireSequence(SurfaceId(frame_sink_id_, local_surface_id),
                                     sequence);
 }
 
 void CompositorFrameSinkSupport::Satisfy(const SurfaceSequence& sequence) {
   surface_manager_->SatisfySequence(sequence);
 }
 
 void CompositorFrameSinkSupport::DidReceiveCompositorFrameAck() {
   DCHECK_GT(ack_pending_count_, 0);
   ack_pending_count_--;
 
   if (!client_)
     return;
+

[danakj, 2017/01/31 19:25:01]

random whitespace?

[Alex Z., 2017/01/31 20:34:18]

Done.

   client_->DidReceiveCompositorFrameAck();
   if (!surface_returned_resources_.empty()) {
     client_->ReclaimResources(surface_returned_resources_);
     surface_returned_resources_.clear();
   }
 }
 
 void CompositorFrameSinkSupport::AddChildFrameSink(
     const FrameSinkId& child_frame_sink_id) {
   child_frame_sinks_.insert(child_frame_sink_id);
   surface_manager_->RegisterFrameSinkHierarchy(frame_sink_id_,
                                                child_frame_sink_id);
 }
 
 void CompositorFrameSinkSupport::RemoveChildFrameSink(
     const FrameSinkId& child_frame_sink_id) {
   auto it = child_frame_sinks_.find(child_frame_sink_id);
   DCHECK(it != child_frame_sinks_.end());
   DCHECK(it->is_valid());
   surface_manager_->UnregisterFrameSinkHierarchy(frame_sink_id_,
                                                  child_frame_sink_id);
   child_frame_sinks_.erase(it);
 }
 
+void CompositorFrameSinkSupport::ForceReclaimResources() {
+  surface_factory_.ClearSurface();
+}
+
 void CompositorFrameSinkSupport::DisplayOutputSurfaceLost() {}
 
 void CompositorFrameSinkSupport::DisplayWillDrawAndSwap(
     bool will_draw_and_swap,
     const RenderPassList& render_passes) {}
 
 void CompositorFrameSinkSupport::DisplayDidDrawAndSwap() {}
 
 void CompositorFrameSinkSupport::ReturnResources(
     const ReturnedResourceArray& resources) {
@@ skipping 48 matching lines @@
     return;
 
   added_frame_observer_ = needs_begin_frame_;
   if (needs_begin_frame_)
     begin_frame_source_->AddObserver(this);
   else
     begin_frame_source_->RemoveObserver(this);
 }
 
 }  // namespace cc