cc: Don't swap throttle BeginMainFrame just after swapping

cc/scheduler/scheduler_state_machine.cc

 // Copyright 2011 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/scheduler/scheduler_state_machine.h"
 
 #include "base/debug/trace_event.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
@@ skipping 446 matching lines @@
     return true;
 
   // After this point, we only start a commit once per frame.
   if (HasSentBeginMainFrameThisFrame())
     return false;
 
   // We shouldn't normally accept commits if there isn't an OutputSurface.
   if (!HasInitializedOutputSurface())
     return false;
 
-  // SwapAck throttle the BeginMainFrames
+  // SwapAck throttle the BeginMainFrames unless we just swapped.

[danakj, 2014/06/11 23:08:57]

This sounds like the opposite of what you have below, more like "unless we
didn't swap inside this deadline yet".

But then, doesn't that make the concept of pending swaps kinda pointless here?
We never wanna commit more than once in a deadline, and this would less us
commit every impl deadline?

[brianderson, 2014/06/11 23:25:49]

I think the comment and logic do match, it's just a little confusing. I actually
got the logic wrong at first until I updated the unit test.
It disables the swap throttling only immediately after a swap within the same
deadline. All subsequent deadlines will not send BeginMainFrames if we remain
swap throttled.

[Sami, 2014/06/13 10:39:12]

The conceptual double negatives here make your head spin don't they :) The logic
below looks correct to me, but I really hope we can make this a little less
ad-hoc in the future. Maybe it's time to consider not throttling at all here, or
even getting rid of smoothness mode.

[brianderson, 2014/06/13 18:19:29]

I've been trying to figure out why getting rid of this restriction causes
browser test flakiness in
https://code.google.com/p/chromium/issues/detail?id=368116. I really really want
to get rid of this check.

   // TODO(brianderson): Remove this restriction to improve throughput.
-  if (pending_swaps_ >= max_pending_swaps_)
+  bool just_swapped_in_deadline =
+      begin_impl_frame_state_ == BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE &&
+      HasSwappedThisFrame();
+  if (pending_swaps_ >= max_pending_swaps_ && !just_swapped_in_deadline)
     return false;
 
   if (skip_begin_main_frame_to_reduce_latency_)
     return false;
 
   return true;
 }
 
 bool SchedulerStateMachine::ShouldCommit() const {
   if (commit_state_ != COMMIT_STATE_READY_TO_COMMIT)
@@ skipping 587 matching lines @@
     case OUTPUT_SURFACE_ACTIVE:
     case OUTPUT_SURFACE_WAITING_FOR_FIRST_COMMIT:
     case OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION:
       return true;
   }
   NOTREACHED();
   return false;
 }
 
 }  // namespace cc