[cc] Remove BeginFrameAck from BFS::DidFinishFrame and update tests.

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"
@@ skipping 68 matching lines @@
     return;
   DestroyCurrentSurface();
 }
 
 void CompositorFrameSinkSupport::SetNeedsBeginFrame(bool needs_begin_frame) {
   needs_begin_frame_ = needs_begin_frame;
   UpdateNeedsBeginFramesInternal();
 }
 
 void CompositorFrameSinkSupport::DidNotProduceFrame(const BeginFrameAck& ack) {
-  // 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);
+
+  // TODO(eseckler): Forward |ack| via SurfaceObservers.
+
   if (begin_frame_source_)
-    begin_frame_source_->DidFinishFrame(this, ack);
+    begin_frame_source_->DidFinishFrame(this);

[sunnyps, 2017/05/24 02:38:25]

nit: We don't need this. When we use back to back BFS (disable vsync) we always
instantiate it in the client.

[Eric Seckler, 2017/05/24 09:00:40]

I think this isn't true for local clients. DirectCFS currently always exposes an
ExternalBFS to its client, not the original BackToBackBFS. Thus, CFSSupport
needs to ack to the BFS on behalf of such local clients. (And this will also be
true once we stop exposing BFSes to CompositorFrameSink(Client)s.)

 }
 
 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;
+  DCHECK_LE(BeginFrameArgs::kStartingFrameNumber, ack.sequence_number);
 
   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);
 
-  // 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.
+  // TODO(eseckler): Forward |ack| via SurfaceObservers.
+
   if (begin_frame_source_)
-    begin_frame_source_->DidFinishFrame(this, ack);
+    begin_frame_source_->DidFinishFrame(this);

[sunnyps, 2017/05/24 02:38:25]

nit: same here

[Eric Seckler, 2017/05/24 09:00:40]

see above.

 }
 
 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 179 matching lines @@
     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