(Reland) Always create top controls manager

cc/input/top_controls_manager.cc

 // Copyright 2013 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/input/top_controls_manager.h"
 
 #include <algorithm>
 
 #include "base/logging.h"
 #include "cc/animation/keyframed_animation_curve.h"
@@ skipping 51 matching lines @@
   return client_->CurrentTopControlsShownRatio();
 }
 
 float TopControlsManager::TopControlsHeight() const {
   return client_->TopControlsHeight();
 }
 
 void TopControlsManager::UpdateTopControlsState(TopControlsState constraints,
                                                 TopControlsState current,
                                                 bool animate) {
+  if (!TopControlsEnabled())

[aelias_OOO_until_Jul13, 2015/03/04 20:05:53]

This one shouldn't be suppressed by height.  We should still be respecting the
state update even at height 0 (so the ratio is correct if the height changes
later).

[bokan, 2015/03/04 21:48:30]

Done.

+    return;
+
   DCHECK(!(constraints == SHOWN && current == HIDDEN));
   DCHECK(!(constraints == HIDDEN && current == SHOWN));
 
   permitted_state_ = constraints;
 
   // Don't do anything if it doesn't matter which state the controls are in.
   if (constraints == BOTH && current == BOTH)
     return;
 
   // Don't do anything if there is no change in offset.
   float final_shown_ratio = 1.f;
   if (constraints == HIDDEN || current == HIDDEN)
     final_shown_ratio = 0.f;
   if (final_shown_ratio == TopControlsShownRatio())
     return;
 
   if (animate) {
     SetupAnimation(final_shown_ratio ? SHOWING_CONTROLS : HIDING_CONTROLS);
   } else {
     ResetAnimations();
     client_->SetCurrentTopControlsShownRatio(final_shown_ratio);
   }
 }
 
 void TopControlsManager::ScrollBegin() {
+  if (!TopControlsEnabled())

[aelias_OOO_until_Jul13, 2015/03/04 20:05:53]

I think we should remove all of the extra early-returns you added outside of
ScrollBy.  In particular, if the height ever changes to nonzero when a scroll
already is in progress, that will work better if we run as much of the usual
logic as possible.

[bokan, 2015/03/04 21:48:30]

Done.

+    return;
+
   DCHECK(!pinch_gesture_active_);
   ResetAnimations();
   ResetBaseline();
 }
 
 gfx::Vector2dF TopControlsManager::ScrollBy(
     const gfx::Vector2dF& pending_delta) {
+  if (!TopControlsEnabled())
+    return pending_delta;
+
   if (pinch_gesture_active_)
     return pending_delta;
 
   if (permitted_state_ == SHOWN && pending_delta.y() > 0)
     return pending_delta;
   else if (permitted_state_ == HIDDEN && pending_delta.y() < 0)
     return pending_delta;
 
   accumulated_scroll_delta_ += pending_delta.y();
 
   float old_offset = ContentTopOffset();
   client_->SetCurrentTopControlsShownRatio(

[aelias_OOO_until_Jul13, 2015/03/04 20:05:53]

For similar reasons of preserving state in case height changes in mid-gesture,
then could you narrowly not call this if TopControlsHeight() == 0, instead of
early-returning out of the whole method?

[bokan, 2015/03/04 21:48:30]

I agree in other cases, but I think preserving state here would look weird; when
the top controls suddenly get a height the page would jump under the users
finger as all the accumulated delta is applied. We'd also need more careful
logic in LTHI as we'd return more delta from this method than we were given.

       (baseline_content_offset_ - accumulated_scroll_delta_) /
       TopControlsHeight());
 
   // If the controls are fully visible, treat the current position as the
   // new baseline even if the gesture didn't end.
   if (TopControlsShownRatio() == 1.f)
     ResetBaseline();
 
   ResetAnimations();
 
   gfx::Vector2dF applied_delta(0.f, old_offset - ContentTopOffset());
   return pending_delta - applied_delta;
 }
 
 void TopControlsManager::ScrollEnd() {
+  if (!TopControlsEnabled())
+    return;
+
   DCHECK(!pinch_gesture_active_);
   StartAnimationIfNecessary();
 }
 
 void TopControlsManager::PinchBegin() {
+  if (!TopControlsEnabled())
+    return;
+
   DCHECK(!pinch_gesture_active_);
   pinch_gesture_active_ = true;
   StartAnimationIfNecessary();
 }
 
 void TopControlsManager::PinchEnd() {
+  if (!TopControlsEnabled())
+    return;
+
   DCHECK(pinch_gesture_active_);
   // Pinch{Begin,End} will always occur within the scope of Scroll{Begin,End},
   // so return to a state expected by the remaining scroll sequence.
   pinch_gesture_active_ = false;
   ScrollBegin();
 }
 
 void TopControlsManager::MainThreadHasStoppedFlinging() {
+  if (!TopControlsEnabled())
+    return;
+
   StartAnimationIfNecessary();
 }
 
 gfx::Vector2dF TopControlsManager::Animate(base::TimeTicks monotonic_time) {
+  DCHECK(TopControlsEnabled());

[aelias_OOO_until_Jul13, 2015/03/04 20:05:53]

I'd suggest removing this DCHECK.  If you're worried about unnecessary animation
frames (although I don't think it would happen in practice), you could change
SetupAnimation just set the ratio to 0/1 without animating if height is 0, and
also have a unit test if you do that.

[bokan, 2015/03/04 21:48:30]

Done.

+
   if (!top_controls_animation_ || !client_->HaveRootScrollLayer())
     return gfx::Vector2dF();
 
   base::TimeDelta time = monotonic_time - base::TimeTicks();
 
   float old_offset = ContentTopOffset();
   client_->SetCurrentTopControlsShownRatio(
       top_controls_animation_->GetValue(time));
 
   if (IsAnimationCompleteAtTime(monotonic_time))
@@ skipping 60 matching lines @@
     return true;
   }
   return false;
 }
 
 void TopControlsManager::ResetBaseline() {
   accumulated_scroll_delta_ = 0.f;
   baseline_content_offset_ = ContentTopOffset();
 }
 
+bool TopControlsManager::TopControlsEnabled() {

[aelias_OOO_until_Jul13, 2015/03/04 20:05:53]

I don't think we should have a method called this, it's not a mode anymore.

[bokan, 2015/03/04 21:48:30]

Done.

+  return TopControlsHeight();
+}
+
 }  // namespace cc