cc: Clean up max frames/swaps pending usage.

All output surfaces used by the Renderer and UI should only need a max swaps pending of 1, especially considering Surfaces and CompositorTimingHistory don't support multiple queued frames. The output suface used by the cc::Display, however, may have more than 1 swap pending - especially on platforms like CrOS where the SwapAck is deferred until the buffer is actually displayed. This patch: 1) Changes the default max pending frames/swaps from 2 to 1. 2) Changes Blimp to have only 1 pending frame. 3) DCHECKS that all Renderers and UIs have a max swaps pending of 1. 4) Sets the default value in the constructor of OutpuSurface::Capabilities, rather than through an extra check for zero + init. BUG=525756 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review URL: https://codereview.chromium.org/1435133004 Cr-Commit-Position: refs/heads/master@{#360283}
Denger-Network · Nov 18, 2015 · 0055b7e · 0055b7e
1 parent 27b48cd
commit 0055b7e
Show file tree

Hide file tree

Showing 25 changed files with 22 additions and 77 deletions.
diff --git a/blimp/client/compositor/blimp_output_surface.cc b/blimp/client/compositor/blimp_output_surface.cc
@@ -14,7 +14,6 @@ namespace blimp {
 BlimpOutputSurface::BlimpOutputSurface(
     const scoped_refptr<cc::ContextProvider>& context_provider)
     : cc::OutputSurface(context_provider) {
-  capabilities_.max_frames_pending = 2;
 }
 
 BlimpOutputSurface::~BlimpOutputSurface() {}

diff --git a/cc/output/output_surface.h b/cc/output/output_surface.h
@@ -45,10 +45,6 @@ class OutputSurfaceClient;
 //      surface (on the compositor thread) and go back to step 1.
 class CC_EXPORT OutputSurface : public base::trace_event::MemoryDumpProvider {
  public:
-  enum {
-    DEFAULT_MAX_FRAMES_PENDING = 2
-  };
-
   OutputSurface(const scoped_refptr<ContextProvider>& context_provider,
                 const scoped_refptr<ContextProvider>& worker_context_provider,
                 scoped_ptr<SoftwareOutputDevice> software_device);
@@ -67,7 +63,7 @@ class CC_EXPORT OutputSurface : public base::trace_event::MemoryDumpProvider {
   struct Capabilities {
     Capabilities()
         : delegated_rendering(false),
-          max_frames_pending(0),
+          max_frames_pending(1),
           adjust_deadline_for_parent(true),
           uses_default_gl_framebuffer(true),
           flipped_output_surface(false),

diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
@@ -193,10 +193,6 @@ void Scheduler::SetNeedsPrepareTiles() {
   ProcessScheduledActions();
 }
 
-void Scheduler::SetMaxSwapsPending(int max) {
-  state_machine_.SetMaxSwapsPending(max);
-}
-
 void Scheduler::DidSwapBuffers() {
   state_machine_.DidSwapBuffers();
 

diff --git a/cc/scheduler/scheduler.h b/cc/scheduler/scheduler.h
@@ -95,7 +95,6 @@ class CC_EXPORT Scheduler : public BeginFrameObserverBase {
 
   void SetNeedsPrepareTiles();
 
-  void SetMaxSwapsPending(int max);
   void DidSwapBuffers();
   void DidSwapBuffersComplete();
 

diff --git a/cc/scheduler/scheduler_state_machine.cc b/cc/scheduler/scheduler_state_machine.cc
@@ -13,6 +13,11 @@
 
 namespace cc {
 
+namespace {
+// Surfaces and CompositorTimingHistory don't support more than 1 pending swap.
+const int kMaxPendingSwaps = 1;
+}  // namespace
+
 SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
     : settings_(settings),
       output_surface_state_(OUTPUT_SURFACE_NONE),
@@ -32,7 +37,6 @@ SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
       invalidate_output_surface_funnel_(false),
       prepare_tiles_funnel_(0),
       consecutive_checkerboard_animations_(0),
-      max_pending_swaps_(1),
       pending_swaps_(0),
       swaps_with_current_output_surface_(0),
       needs_redraw_(false),
@@ -217,7 +221,6 @@ void SchedulerStateMachine::AsValueInto(
                     invalidate_output_surface_funnel_);
   state->SetInteger("consecutive_checkerboard_animations",
                     consecutive_checkerboard_animations_);
-  state->SetInteger("max_pending_swaps_", max_pending_swaps_);
   state->SetInteger("pending_swaps_", pending_swaps_);
   state->SetInteger("swaps_with_current_output_surface",
                     swaps_with_current_output_surface_);
@@ -912,7 +915,7 @@ bool SchedulerStateMachine::main_thread_missed_last_deadline() const {
 }
 
 bool SchedulerStateMachine::SwapThrottled() const {
-  return pending_swaps_ >= max_pending_swaps_;
+  return pending_swaps_ >= kMaxPendingSwaps;
 }
 
 void SchedulerStateMachine::SetVisible(bool visible) {
@@ -957,17 +960,12 @@ void SchedulerStateMachine::SetNeedsPrepareTiles() {
     needs_prepare_tiles_ = true;
   }
 }
-
-void SchedulerStateMachine::SetMaxSwapsPending(int max) {
-  max_pending_swaps_ = max;
-}
-
 void SchedulerStateMachine::DidSwapBuffers() {
   TRACE_EVENT_ASYNC_BEGIN0("cc", "Scheduler:pending_swaps", this);
   pending_swaps_++;
   swaps_with_current_output_surface_++;
 
-  DCHECK_LE(pending_swaps_, max_pending_swaps_);
+  DCHECK_LE(pending_swaps_, kMaxPendingSwaps);
 
   did_perform_swap_in_last_draw_ = true;
   last_frame_number_swap_performed_ = current_frame_number_;

diff --git a/cc/scheduler/scheduler_state_machine.h b/cc/scheduler/scheduler_state_machine.h
@@ -174,9 +174,6 @@ class CC_EXPORT SchedulerStateMachine {
   // PrepareTiles will occur shortly (even if no redraw is required).
   void SetNeedsPrepareTiles();
 
-  // Sets how many swaps can be pending to the OutputSurface.
-  void SetMaxSwapsPending(int max);
-
   // If the scheduler attempted to draw and swap, this provides feedback
   // regarding whether or not the swap actually occured. We might skip the
   // swap when there is not damage, for example.

diff --git a/cc/scheduler/scheduler_unittest.cc b/cc/scheduler/scheduler_unittest.cc
@@ -1471,7 +1471,6 @@ TEST_F(SchedulerTest, MainFrameNotSkippedAfterLateCommit_DrawEstimateTooLong) {
 void SchedulerTest::ImplFrameSkippedAfterLateSwapAck(
     bool swap_ack_before_deadline) {
   // To get into a high latency state, this test disables automatic swap acks.
-  scheduler_->SetMaxSwapsPending(1);
   client_->SetAutomaticSwapAck(false);
 
   // Draw and swap for first BeginFrame
@@ -1595,7 +1594,6 @@ TEST_F(SchedulerTest,
   SetUpScheduler(true);
 
   // To get into a high latency state, this test disables automatic swap acks.
-  scheduler_->SetMaxSwapsPending(1);
   client_->SetAutomaticSwapAck(false);
 
   // Even if every estimate related to the main thread is slow, we should
@@ -1659,7 +1657,6 @@ TEST_F(SchedulerTest,
 
 void SchedulerTest::ImplFrameIsNotSkippedAfterLateSwapAck() {
   // To get into a high latency state, this test disables automatic swap acks.
-  scheduler_->SetMaxSwapsPending(1);
   client_->SetAutomaticSwapAck(false);
 
   // Draw and swap for first BeginFrame
@@ -1770,7 +1767,6 @@ TEST_F(SchedulerTest,
   fake_compositor_timing_history_->SetAllEstimatesTo(slow_duration);
 
   // To get into a high latency state, this test disables automatic swap acks.
-  scheduler_->SetMaxSwapsPending(1);
   client_->SetAutomaticSwapAck(false);
 
   // Impl thread hits deadline before commit finishes to make
@@ -1899,7 +1895,6 @@ TEST_F(
 
   // Disables automatic swap acks so this test can force swap ack throttling
   // to simulate a blocked Browser ui thread.
-  scheduler_->SetMaxSwapsPending(1);
   client_->SetAutomaticSwapAck(false);
 
   // Get a new active tree in main-thread high latency mode and put us
@@ -1970,7 +1965,6 @@ TEST_F(SchedulerTest,
 
   // Disables automatic swap acks so this test can force swap ack throttling
   // to simulate a blocked Browser ui thread.
-  scheduler_->SetMaxSwapsPending(1);
   client_->SetAutomaticSwapAck(false);
 
   // Start a new commit in main-thread high latency mode and hold off on
@@ -2051,7 +2045,6 @@ TEST_F(
 
   // Disables automatic swap acks so this test can force swap ack throttling
   // to simulate a blocked Browser ui thread.
-  scheduler_->SetMaxSwapsPending(1);
   client_->SetAutomaticSwapAck(false);
 
   // Start a new commit in main-thread high latency mode and hold off on
@@ -2447,7 +2440,6 @@ void SchedulerTest::BeginFramesNotFromClient_SwapThrottled(
   fake_compositor_timing_history_->SetDrawDurationEstimate(base::TimeDelta());
 
   // To test swap ack throttling, this test disables automatic swap acks.
-  scheduler_->SetMaxSwapsPending(1);
   client_->SetAutomaticSwapAck(false);
 
   // SetNeedsBeginMainFrame should begin the frame on the next BeginImplFrame.

diff --git a/cc/surfaces/onscreen_display_client.cc b/cc/surfaces/onscreen_display_client.cc
@@ -35,10 +35,7 @@ OnscreenDisplayClient::~OnscreenDisplayClient() {
 }
 
 bool OnscreenDisplayClient::Initialize() {
-  int max_frames_pending =
-      output_surface_ ? output_surface_->capabilities().max_frames_pending : 0;
-  if (max_frames_pending <= 0)
-    max_frames_pending = OutputSurface::DEFAULT_MAX_FRAMES_PENDING;
+  DCHECK(output_surface_);
 
   BeginFrameSource* frame_source;
   if (disable_display_vsync_) {
@@ -51,8 +48,9 @@ bool OnscreenDisplayClient::Initialize() {
     frame_source = synthetic_frame_source_.get();
   }
 
-  scheduler_.reset(new DisplayScheduler(
-      display_.get(), frame_source, task_runner_.get(), max_frames_pending));
+  scheduler_.reset(
+      new DisplayScheduler(display_.get(), frame_source, task_runner_.get(),
+                           output_surface_->capabilities().max_frames_pending));
 
   return display_->Initialize(output_surface_.Pass(), scheduler_.get());
 }

diff --git a/cc/surfaces/surface_display_output_surface.cc b/cc/surfaces/surface_display_output_surface.cc
@@ -25,7 +25,6 @@ SurfaceDisplayOutputSurface::SurfaceDisplayOutputSurface(
       allocator_(allocator) {
   factory_.set_needs_sync_points(false);
   capabilities_.delegated_rendering = true;
-  capabilities_.max_frames_pending = 1;
   capabilities_.adjust_deadline_for_parent = true;
   capabilities_.can_force_reclaim_resources = true;
   // Display and SurfaceDisplayOutputSurface share a GL context, so sync

diff --git a/cc/surfaces/surface_display_output_surface_unittest.cc b/cc/surfaces/surface_display_output_surface_unittest.cc
@@ -36,6 +36,7 @@ class FakeOnscreenDisplayClient : public OnscreenDisplayClient {
     // to it now for future reference.
     fake_output_surface_ =
         static_cast<FakeOutputSurface*>(output_surface_.get());
+    fake_output_surface_->set_max_frames_pending(2);
   }
 
   FakeOutputSurface* output_surface() { return fake_output_surface_; }

diff --git a/cc/test/fake_layer_tree_host_impl_client.h b/cc/test/fake_layer_tree_host_impl_client.h
@@ -19,7 +19,6 @@ class FakeLayerTreeHostImplClient : public LayerTreeHostImplClient {
   void CommitVSyncParameters(base::TimeTicks timebase,
                              base::TimeDelta interval) override {}
   void SetEstimatedParentDrawTime(base::TimeDelta draw_time) override {}
-  void SetMaxSwapsPendingOnImplThread(int max) override {}
   void DidSwapBuffersOnImplThread() override {}
   void DidSwapBuffersCompleteOnImplThread() override {}
   void OnResourcelessSoftareDrawStateChanged(bool resourceless_draw) override {}

diff --git a/cc/test/fake_output_surface.cc b/cc/test/fake_output_surface.cc
@@ -25,10 +25,7 @@ FakeOutputSurface::FakeOutputSurface(
       suspended_for_recycle_(false),
       framebuffer_(0),
       overlay_candidate_validator_(nullptr) {
-  if (delegated_rendering) {
-    capabilities_.delegated_rendering = true;
-    capabilities_.max_frames_pending = 1;
-  }
+  capabilities_.delegated_rendering = delegated_rendering;
 }
 
 FakeOutputSurface::FakeOutputSurface(
@@ -41,10 +38,7 @@ FakeOutputSurface::FakeOutputSurface(
       suspended_for_recycle_(false),
       framebuffer_(0),
       overlay_candidate_validator_(nullptr) {
-  if (delegated_rendering) {
-    capabilities_.delegated_rendering = true;
-    capabilities_.max_frames_pending = 1;
-  }
+  capabilities_.delegated_rendering = delegated_rendering;
 }
 
 FakeOutputSurface::FakeOutputSurface(
@@ -57,10 +51,7 @@ FakeOutputSurface::FakeOutputSurface(
       suspended_for_recycle_(false),
       framebuffer_(0),
       overlay_candidate_validator_(nullptr) {
-  if (delegated_rendering) {
-    capabilities_.delegated_rendering = true;
-    capabilities_.max_frames_pending = 1;
-  }
+  capabilities_.delegated_rendering = delegated_rendering;
 }
 
 FakeOutputSurface::FakeOutputSurface(
@@ -74,10 +65,7 @@ FakeOutputSurface::FakeOutputSurface(
       suspended_for_recycle_(false),
       framebuffer_(0),
       overlay_candidate_validator_(nullptr) {
-  if (delegated_rendering) {
-    capabilities_.delegated_rendering = true;
-    capabilities_.max_frames_pending = 1;
-  }
+  capabilities_.delegated_rendering = delegated_rendering;
 }
 
 FakeOutputSurface::~FakeOutputSurface() {}

diff --git a/cc/test/fake_output_surface.h b/cc/test/fake_output_surface.h
@@ -94,6 +94,10 @@ class FakeOutputSurface : public OutputSurface {
     return surface.Pass();
   }
 
+  void set_max_frames_pending(int max) {
+    capabilities_.max_frames_pending = max;
+  }
+
   CompositorFrame& last_sent_frame() { return last_sent_frame_; }
   size_t num_sent_frames() { return num_sent_frames_; }
 

diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
@@ -2299,10 +2299,7 @@ bool LayerTreeHostImpl::InitializeRenderer(OutputSurface* output_surface) {
           : base::TimeDelta();
   client_->SetEstimatedParentDrawTime(parent_draw_time);
 
-  int max_frames_pending = output_surface_->capabilities().max_frames_pending;
-  if (max_frames_pending <= 0)
-    max_frames_pending = OutputSurface::DEFAULT_MAX_FRAMES_PENDING;
-  client_->SetMaxSwapsPendingOnImplThread(max_frames_pending);
+  DCHECK_EQ(1, output_surface_->capabilities().max_frames_pending);
   client_->OnCanDrawStateChanged(CanDraw());
 
   // There will not be anything to draw here, so set high res

diff --git a/cc/trees/layer_tree_host_impl.h b/cc/trees/layer_tree_host_impl.h
@@ -93,7 +93,6 @@ class LayerTreeHostImplClient {
   virtual void CommitVSyncParameters(base::TimeTicks timebase,
                                      base::TimeDelta interval) = 0;
   virtual void SetEstimatedParentDrawTime(base::TimeDelta draw_time) = 0;
-  virtual void SetMaxSwapsPendingOnImplThread(int max) = 0;
   virtual void DidSwapBuffersOnImplThread() = 0;
   virtual void DidSwapBuffersCompleteOnImplThread() = 0;
   virtual void OnResourcelessSoftareDrawStateChanged(

diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -114,7 +114,6 @@ class LayerTreeHostImplTest : public testing::Test,
   void CommitVSyncParameters(base::TimeTicks timebase,
                              base::TimeDelta interval) override {}
   void SetEstimatedParentDrawTime(base::TimeDelta draw_time) override {}
-  void SetMaxSwapsPendingOnImplThread(int max) override {}
   void DidSwapBuffersOnImplThread() override {}
   void DidSwapBuffersCompleteOnImplThread() override {}
   void OnResourcelessSoftareDrawStateChanged(bool resourceless_draw) override {}

diff --git a/cc/trees/single_thread_proxy.cc b/cc/trees/single_thread_proxy.cc
@@ -482,11 +482,6 @@ void SingleThreadProxy::SetEstimatedParentDrawTime(base::TimeDelta draw_time) {
     scheduler_on_impl_thread_->SetEstimatedParentDrawTime(draw_time);
 }
 
-void SingleThreadProxy::SetMaxSwapsPendingOnImplThread(int max) {
-  if (scheduler_on_impl_thread_)
-    scheduler_on_impl_thread_->SetMaxSwapsPending(max);
-}
-
 void SingleThreadProxy::DidSwapBuffersOnImplThread() {
   TRACE_EVENT0("cc", "SingleThreadProxy::DidSwapBuffersOnImplThread");
   if (scheduler_on_impl_thread_)

diff --git a/cc/trees/single_thread_proxy.h b/cc/trees/single_thread_proxy.h
@@ -85,7 +85,6 @@ class CC_EXPORT SingleThreadProxy : public Proxy,
   void CommitVSyncParameters(base::TimeTicks timebase,
                              base::TimeDelta interval) override;
   void SetEstimatedParentDrawTime(base::TimeDelta draw_time) override;
-  void SetMaxSwapsPendingOnImplThread(int max) override;
   void DidSwapBuffersOnImplThread() override;
   void DidSwapBuffersCompleteOnImplThread() override;
   void OnResourcelessSoftareDrawStateChanged(bool resourceless_draw) override;

diff --git a/cc/trees/thread_proxy.cc b/cc/trees/thread_proxy.cc
@@ -308,10 +308,6 @@ void ThreadProxy::SetEstimatedParentDrawTime(base::TimeDelta draw_time) {
   impl().scheduler->SetEstimatedParentDrawTime(draw_time);
 }
 
-void ThreadProxy::SetMaxSwapsPendingOnImplThread(int max) {
-  impl().scheduler->SetMaxSwapsPending(max);
-}
-
 void ThreadProxy::DidSwapBuffersOnImplThread() {
   impl().scheduler->DidSwapBuffers();
 }

diff --git a/cc/trees/thread_proxy.h b/cc/trees/thread_proxy.h
@@ -187,7 +187,6 @@ class CC_EXPORT ThreadProxy : public Proxy,
   void CommitVSyncParameters(base::TimeTicks timebase,
                              base::TimeDelta interval) override;
   void SetEstimatedParentDrawTime(base::TimeDelta draw_time) override;
-  void SetMaxSwapsPendingOnImplThread(int max) override;
   void DidSwapBuffersOnImplThread() override;
   void DidSwapBuffersCompleteOnImplThread() override;
   void OnResourcelessSoftareDrawStateChanged(bool resourceless_draw) override;

diff --git a/components/mus/public/cpp/lib/output_surface.cc b/components/mus/public/cpp/lib/output_surface.cc
@@ -18,7 +18,6 @@ OutputSurface::OutputSurface(
     scoped_ptr<mus::WindowSurface> surface)
     : cc::OutputSurface(context_provider), surface_(surface.Pass()) {
   capabilities_.delegated_rendering = true;
-  capabilities_.max_frames_pending = 1;
 }
 
 OutputSurface::~OutputSurface() {}

diff --git a/content/browser/compositor/browser_compositor_output_surface.cc b/content/browser/compositor/browser_compositor_output_surface.cc
@@ -56,7 +56,6 @@ BrowserCompositorOutputSurface::~BrowserCompositorOutputSurface() {
 }
 
 void BrowserCompositorOutputSurface::Initialize() {
-  capabilities_.max_frames_pending = 1;
   capabilities_.adjust_deadline_for_parent = false;
 }
 

diff --git a/content/browser/compositor/offscreen_browser_compositor_output_surface.cc b/content/browser/compositor/offscreen_browser_compositor_output_surface.cc
@@ -41,7 +41,6 @@ OffscreenBrowserCompositorOutputSurface::
       fbo_(0),
       is_backbuffer_discarded_(false),
       weak_ptr_factory_(this) {
-  capabilities_.max_frames_pending = 1;
   capabilities_.uses_default_gl_framebuffer = false;
 }
 

diff --git a/content/renderer/android/synchronous_compositor_output_surface.cc b/content/renderer/android/synchronous_compositor_output_surface.cc
@@ -79,7 +79,6 @@ SynchronousCompositorOutputSurface::SynchronousCompositorOutputSurface(
   DCHECK(registry_);
   capabilities_.adjust_deadline_for_parent = false;
   capabilities_.delegated_rendering = true;
-  capabilities_.max_frames_pending = 1;
   memory_policy_.priority_cutoff_when_visible =
       gpu::MemoryAllocation::CUTOFF_ALLOW_NICE_TO_HAVE;
 }

diff --git a/content/renderer/gpu/compositor_output_surface.cc b/content/renderer/gpu/compositor_output_surface.cc
@@ -50,7 +50,6 @@ CompositorOutputSurface::CompositorOutputSurface(
       weak_ptrs_(this) {
   DCHECK(output_surface_filter_.get());
   DCHECK(frame_swap_message_queue_.get());
-  capabilities_.max_frames_pending = 1;
   message_sender_ = RenderThreadImpl::current()->sync_message_filter();
   DCHECK(message_sender_.get());
 }