Extract non-Mojo bits of GpuCompositorFrameSink to 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_manager.h"
+
+namespace cc {
+
+CompositorFrameSinkSupport::CompositorFrameSinkSupport(
+    CompositorFrameSinkSupportClient* client,
+    SurfaceManager* surface_manager,
+    const FrameSinkId& frame_sink_id,
+    std::unique_ptr<Display> display)
+    : client_(client),
+      surface_manager_(surface_manager),
+      frame_sink_id_(frame_sink_id),
+      display_(std::move(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);
+
+  if (display_) {
+    display_->Initialize(this, surface_manager_);
+    display_->SetVisible(true);
+  }
+}
+
+CompositorFrameSinkSupport::~CompositorFrameSinkSupport() {
+  // SurfaceFactory's destructor will attempt to return resources which will
+  // call back into here and access |client_| so we should destroy
+  // |surface_factory_|'s resources early on.
+  surface_factory_.EvictSurface();
+  surface_manager_->UnregisterSurfaceFactoryClient(frame_sink_id_);
+  surface_manager_->InvalidateFrameSinkId(frame_sink_id_);
+}
+
+void CompositorFrameSinkSupport::SetNeedsBeginFrame(bool needs_begin_frame) {
+  needs_begin_frame_ = needs_begin_frame;
+  UpdateNeedsBeginFramesInternal();
+}
+
+void CompositorFrameSinkSupport::SubmitCompositorFrame(
+    const LocalFrameId& local_frame_id,
+    CompositorFrame frame) {
+  // If the size of the CompostiorFrame has changed then destroy the existing
+  // Surface and create a new one of the appropriate size.
+  if (local_frame_id_ != local_frame_id) {
+    local_frame_id_ = local_frame_id;
+    if (display_ && !frame.render_pass_list.empty()) {
+      gfx::Size frame_size = frame.render_pass_list[0]->output_rect.size();
+      display_->Resize(frame_size);

[piman, 2016/12/02 22:44:29]

This is probably ok as a first pass, but I think we will revisit this, because
some clients may want finer control over the Display sizing - since it really
means the actual size of the native window (Linux/Win Aura) or display (Ozone) -
and we may need to either prevent or gutter Display frames until there is a
correctly-sized top-level CompositorFrame available (see
ui::Compositor::DisableSwapUntilResize), iow, we may want to resize display_
out-of-band (earlier).

[Fady Samuel, 2016/12/02 23:05:11]

I converted your feedback into a TODO(piman, fsamuel):

+    }
+  }
+  ++ack_pending_count_;
+  surface_factory_.SubmitCompositorFrame(
+      local_frame_id_, std::move(frame),
+      base::Bind(&CompositorFrameSinkSupport::DidReceiveCompositorFrameAck,
+                 weak_factory_.GetWeakPtr()));
+  if (display_) {
+    display_->SetLocalFrameId(local_frame_id_,
+                              frame.metadata.device_scale_factor);
+  }
+}
+
+void CompositorFrameSinkSupport::DidReceiveCompositorFrameAck() {
+  if (!client_)
+    return;
+  client_->DidReceiveCompositorFrameAck();
+  DCHECK_GT(ack_pending_count_, 0);
+  if (!surface_returned_resources_.empty()) {
+    client_->ReclaimResources(surface_returned_resources_);
+    surface_returned_resources_.clear();
+  }
+  ack_pending_count_--;

[piman, 2016/12/02 22:44:29]

nit: should we do this even if client_ is null, for consistency?

The pattern for DirectCompositorFrameSink makes me think that the lifetime for
CompositorFrameSinkSupportClient (that will be DirectCompositorFrameSink) and
for the CompositorFrameSinkSupport will be separate, so we may want accurate
accounting even while there is no client (may happen temporarily while the
CompositorFrameSink is lost)

[Fady Samuel, 2016/12/02 23:05:11]

Done.

+}
+
+void CompositorFrameSinkSupport::AddChildFrameSink(
+    const FrameSinkId& child_frame_sink_id) {
+  surface_manager_->RegisterFrameSinkHierarchy(frame_sink_id_,
+                                               child_frame_sink_id);
+}
+
+void CompositorFrameSinkSupport::RemoveChildFrameSink(
+    const FrameSinkId& child_frame_sink_id) {
+  surface_manager_->UnregisterFrameSinkHierarchy(frame_sink_id_,
+                                                 child_frame_sink_id);
+}
+
+void CompositorFrameSinkSupport::DisplayOutputSurfaceLost() {}
+
+void CompositorFrameSinkSupport::DisplayWillDrawAndSwap(
+    bool will_draw_and_swap,
+    const RenderPassList& render_passes) {}
+
+void CompositorFrameSinkSupport::DisplayDidDrawAndSwap() {}
+
+void CompositorFrameSinkSupport::ReturnResources(
+    const ReturnedResourceArray& resources) {
+  if (resources.empty())
+    return;
+
+  if (!ack_pending_count_ && client_) {
+    client_->ReclaimResources(resources);
+    return;
+  }
+
+  std::copy(resources.begin(), resources.end(),
+            std::back_inserter(surface_returned_resources_));
+}
+
+void CompositorFrameSinkSupport::SetBeginFrameSource(
+    BeginFrameSource* begin_frame_source) {
+  // TODO(tansell): Implement this.

[piman, 2016/12/02 22:44:29]

nit: what is "this" ?
That function looks like it has a reasonable implementation. Stale comment
maybe?

[Fady Samuel, 2016/12/02 23:05:11]

It is stale. Removed.

+  if (begin_frame_source_ && added_frame_observer_) {
+    begin_frame_source_->RemoveObserver(this);
+    added_frame_observer_ = false;
+  }
+  begin_frame_source_ = begin_frame_source;
+  UpdateNeedsBeginFramesInternal();
+}
+
+void CompositorFrameSinkSupport::OnBeginFrame(const BeginFrameArgs& args) {
+  UpdateNeedsBeginFramesInternal();
+  last_begin_frame_args_ = args;
+  if (client_)
+    client_->OnBeginFrame(args);
+}
+
+const BeginFrameArgs& CompositorFrameSinkSupport::LastUsedBeginFrameArgs()
+    const {
+  return last_begin_frame_args_;
+}
+
+void CompositorFrameSinkSupport::OnBeginFrameSourcePausedChanged(bool paused) {}
+
+void CompositorFrameSinkSupport::UpdateNeedsBeginFramesInternal() {
+  if (!begin_frame_source_)
+    return;
+
+  if (needs_begin_frame_ == added_frame_observer_)
+    return;
+
+  added_frame_observer_ = needs_begin_frame_;
+  if (needs_begin_frame_)
+    begin_frame_source_->AddObserver(this);
+  else
+    begin_frame_source_->RemoveObserver(this);
+}
+
+}  // namespace cc