[cc] Plumb BeginFrameAcks through SurfaceManager to DisplayScheduler.

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 <algorithm>
 #include <utility>
 
 #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_info.h"
 #include "cc/surfaces/surface_manager.h"
 #include "cc/surfaces/surface_reference.h"
 
 namespace cc {
 
 // static
 std::unique_ptr<CompositorFrameSinkSupport> CompositorFrameSinkSupport::Create(
     CompositorFrameSinkSupportClient* client,
     SurfaceManager* surface_manager,
@@ skipping 55 matching lines @@
   DestroyCurrentSurface();
 }
 
 void CompositorFrameSinkSupport::SetNeedsBeginFrame(bool needs_begin_frame) {
   needs_begin_frame_ = needs_begin_frame;
   UpdateNeedsBeginFramesInternal();
 }
 
 void CompositorFrameSinkSupport::BeginFrameDidNotSwap(
     const BeginFrameAck& ack) {
+  TRACE_EVENT2("cc", "CompositorFrameSinkSupport::BeginFrameDidNotSwap",
+               "ack.source_id", ack.source_id, "ack.sequence_number",
+               ack.sequence_number);
   // TODO(eseckler): While a pending CompositorFrame exists (see TODO below), we
   // should not acknowledge immediately. Instead, we should update the ack that
   // will be sent to DisplayScheduler when the pending frame is activated.
   DCHECK_GE(ack.sequence_number, BeginFrameArgs::kStartingFrameNumber);
 
   // |has_damage| is not transmitted, but false by default.
   DCHECK(!ack.has_damage);
+
+  last_begin_frame_ack_ = ack;
+
+  if (producer_state_ == Surface::PENDING &&

[sunnyps, 2017/05/19 03:46:33]

I was thinking more along the lines of plumbing this to
SurfaceObserver::OnSurfaceDamaged(BeginFrameAck) but only if Surface is
activated. Something like this:

if (current_surface_ && !current_surface_->HasPendingFrame()) {
  surface_manager_->SurfaceModified(surface->surface_id(), ack);
}

DisplayScheduler can do the surface state tracking internally. I think that's
better because this is really specific to DisplayScheduler and we should
encapsulate it there as much as possible. We can make more changes to it later.

+      last_begin_frame_args_.source_id == ack.source_id &&
+      last_begin_frame_args_.sequence_number == ack.sequence_number) {
+    SetProducerState(Surface::IDLE);
+  }
   if (begin_frame_source_)
     begin_frame_source_->DidFinishFrame(this, ack);
 }
 
 void CompositorFrameSinkSupport::SubmitCompositorFrame(
     const LocalSurfaceId& local_surface_id,
     CompositorFrame frame) {
   TRACE_EVENT0("cc", "CompositorFrameSinkSupport::SubmitCompositorFrame");
   DCHECK(local_surface_id.is_valid());
   DCHECK_GE(frame.metadata.begin_frame_ack.sequence_number,
             BeginFrameArgs::kStartingFrameNumber);
   DCHECK(!frame.render_pass_list.empty());
 
   ++ack_pending_count_;
 
   // |has_damage| is not transmitted.
   frame.metadata.begin_frame_ack.has_damage = true;
 
-  BeginFrameAck ack = frame.metadata.begin_frame_ack;
+  last_begin_frame_ack_ = frame.metadata.begin_frame_ack;
 
   if (!ui::LatencyInfo::Verify(frame.metadata.latency_info,
                                "RenderWidgetHostImpl::OnSwapCompositorFrame")) {
     std::vector<ui::LatencyInfo>().swap(frame.metadata.latency_info);
   }
   for (ui::LatencyInfo& latency : frame.metadata.latency_info) {
     if (latency.latency_components().size() > 0) {
       latency.AddLatencyNumber(ui::DISPLAY_COMPOSITOR_RECEIVED_FRAME_COMPONENT,
                                0, 0);
     }
@@ skipping 15 matching lines @@
                  weak_factory_.GetWeakPtr()),
       base::BindRepeating(&CompositorFrameSinkSupport::WillDrawSurface,
                           weak_factory_.GetWeakPtr()));
 
   if (current_surface_) {
     surface->SetPreviousFrameSurface(current_surface_.get());
     DestroyCurrentSurface();
   }
   current_surface_ = std::move(surface);
 
+  SetProducerState(Surface::READY);

[sunnyps, 2017/05/19 03:46:34]

Similarly here we already plumb surface activation to
SurfaceObserver::OnSurfaceModified. We can tack on the BeginFrameAck to that and
do the state tracking in DisplayScheduler.

+
   // TODO(eseckler): The CompositorFrame submitted below might not be activated
   // right away b/c of surface synchronization. We should only send the
   // BeginFrameAck to DisplayScheduler when it is activated. This also means
   // that we need to stay an active BFO while a CompositorFrame is pending.
   // See https://crbug.com/703079.
   if (begin_frame_source_)
-    begin_frame_source_->DidFinishFrame(this, ack);
+    begin_frame_source_->DidFinishFrame(this, last_begin_frame_ack_);
 }
 
 void CompositorFrameSinkSupport::UpdateSurfaceReferences(
     const SurfaceId& last_surface_id,
     const LocalSurfaceId& local_surface_id) {
   const bool surface_id_changed =
       last_surface_id.local_surface_id() != local_surface_id;
 
   // If this is a display root surface and the SurfaceId is changing, make the
   // new SurfaceId reachable from the top-level root.
@@ skipping 48 matching lines @@
   // frame if any.
   reference_tracker_.UpdateReferences(local_surface_id,
                                       active_referenced_surfaces);
 
   UpdateSurfaceReferences(last_surface_id, local_surface_id);
 }
 
 void CompositorFrameSinkSupport::DidReceiveCompositorFrameAck() {
   DCHECK_GT(ack_pending_count_, 0);
   ack_pending_count_--;
+

[sunnyps, 2017/05/19 03:46:34]

See my later comments in DisplayScheduler first.

If the begin frame ack is compared to the scheduler's begin frame args we don't
need this because the scheduler only updates its begin frame args after
finishing the previous frame.

+  if (ack_pending_count_ == 0 && added_frame_observer_ &&
+      (last_begin_frame_args_.source_id != last_begin_frame_ack_.source_id ||
+       last_begin_frame_args_.sequence_number !=
+           last_begin_frame_ack_.sequence_number)) {
+    SetProducerState(Surface::PENDING);
+  } else {
+    SetProducerState(Surface::IDLE);
+  }
+
   if (!client_)
     return;
 
   client_->DidReceiveCompositorFrameAck(surface_returned_resources_);
   surface_returned_resources_.clear();
 }
 
 void CompositorFrameSinkSupport::WillDrawSurface(
     const LocalSurfaceId& local_surface_id,
     const gfx::Rect& damage_rect) {
@@ skipping 38 matching lines @@
 
 void CompositorFrameSinkSupport::Init(SurfaceManager* surface_manager) {
   surface_manager_ = surface_manager;
   if (handles_frame_sink_id_invalidation_)
     surface_manager_->RegisterFrameSinkId(frame_sink_id_);
   surface_manager_->RegisterFrameSinkManagerClient(frame_sink_id_, this);
 }
 
 void CompositorFrameSinkSupport::OnBeginFrame(const BeginFrameArgs& args) {
   UpdateNeedsBeginFramesInternal();
+  if (producer_state_ == Surface::IDLE)
+    SetProducerState(Surface::PENDING);
   last_begin_frame_args_ = args;
   if (client_)
     client_->OnBeginFrame(args);
 }
 
 const BeginFrameArgs& CompositorFrameSinkSupport::LastUsedBeginFrameArgs()
     const {
   return last_begin_frame_args_;
 }
 
@@ skipping 39 matching lines @@
 
 std::unique_ptr<Surface> CompositorFrameSinkSupport::CreateSurface(
     const LocalSurfaceId& local_surface_id) {
   seen_first_frame_activation_ = false;
   std::unique_ptr<Surface> surface = surface_manager_->CreateSurface(
       weak_factory_.GetWeakPtr(), local_surface_id);
   return surface;
 }
 
 void CompositorFrameSinkSupport::DestroyCurrentSurface() {
+  SetProducerState(Surface::IDLE);
   surface_manager_->DestroySurface(std::move(current_surface_));
 }
 
+void CompositorFrameSinkSupport::SetProducerState(
+    Surface::ProducerState state) {
+  TRACE_EVENT1("cc", "CompositorFrameSinkSupport::SetProducerState", "state",
+               state);
+  producer_state_ = state;
+  if (current_surface_)
+    current_surface_->SetProducerState(state);
+}
+
 void CompositorFrameSinkSupport::RequestCopyOfSurface(
     std::unique_ptr<CopyOutputRequest> copy_request) {
   if (!current_surface_)
     return;
 
   DCHECK(current_surface_->compositor_frame_sink_support().get() == this);
   current_surface_->RequestCopyOfOutput(std::move(copy_request));
   surface_manager_->SurfaceModified(current_surface_->surface_id());
 }
 
 }  // namespace cc