alternate ntp: toolbar background and separator animation

chrome/browser/ui/search/toolbar_search_animator.cc

Index: chrome/browser/ui/search/toolbar_search_animator.cc
diff --git a/chrome/browser/ui/search/toolbar_search_animator.cc b/chrome/browser/ui/search/toolbar_search_animator.cc
index c8160fb047e1318caf60f21b74858725baffa66d..e8d74261561ca2bb751c84206ebf593eb7c5d4f3 100644
--- a/chrome/browser/ui/search/toolbar_search_animator.cc
+++ b/chrome/browser/ui/search/toolbar_search_animator.cc
@@ -16,6 +16,8 @@ namespace {
 const int kBackgroundChangeDelayMs = 100;
 const int kBackgroundChangeDurationMs = 200;
 
+const int kSeparatorFadeDurationMs = 100;
+
 const double kMinOpacity = 0.0f;
 const double kMaxOpacity = 1.0f;
 
@@ -26,32 +28,53 @@ namespace search {
 
 ToolbarSearchAnimator::ToolbarSearchAnimator(SearchModel* search_model)
     : search_model_(search_model),
-      animate_state_(ANIMATE_STATE_NONE) {
+      background_animate_state_(ANIMATE_STATE_NONE),
+      separator_animate_state_(ANIMATE_STATE_NONE),
+      ALLOW_THIS_IN_INITIALIZER_LIST(background_animation_(this)),
+      ALLOW_THIS_IN_INITIALIZER_LIST(separator_animation_(this)),
+      from_mode_(Mode::MODE_DEFAULT),
+      to_mode_(Mode::MODE_DEFAULT),
+      modes_initialized_(false) {
   search_model_->AddObserver(this);
+
+  background_animation_.SetTweenType(ui::Tween::LINEAR);
+  background_animation_.SetSlideDuration(
+      kBackgroundChangeDurationMs * InstantUI::GetSlowAnimationScaleFactor());
+
+  separator_animation_.SetTweenType(ui::Tween::LINEAR);
+  separator_animation_.SetSlideDuration(
+      kSeparatorFadeDurationMs * InstantUI::GetSlowAnimationScaleFactor());
 }
 
 ToolbarSearchAnimator::~ToolbarSearchAnimator() {
+  background_animation_.Stop();
+  separator_animation_.Stop();
   search_model_->RemoveObserver(this);
 }
 
-void ToolbarSearchAnimator::GetCurrentBackgroundState(
-    BackgroundState* background_state,
-    double* search_background_opacity) const {
-  // Should only be called for SEARCH mode.
-  DCHECK(search_model_->mode().is_search());
-  *background_state = BACKGROUND_STATE_DEFAULT;
-  *search_background_opacity = -1.0f;
-  switch (animate_state_) {
+double ToolbarSearchAnimator::GetGradientOpacity() const {
+  switch (background_animate_state_) {
     case ANIMATE_STATE_WAITING:
-      *background_state = BACKGROUND_STATE_NTP;
-      break;
+      return kMinOpacity;
     case ANIMATE_STATE_RUNNING:
-      *background_state = BACKGROUND_STATE_NTP_SEARCH;
-      *search_background_opacity = background_animation_->CurrentValueBetween(
-          kMinOpacity, kMaxOpacity);
-      break;
+      return background_animation_.CurrentValueBetween(kMinOpacity,
+                                                       kMaxOpacity);
     case ANIMATE_STATE_NONE:
-      break;
+    default:

[dhollowa, 2012/07/24 00:24:49]

You can remove |default:|.

[kuan, 2012/07/30 23:21:02]

doesn't build if removed: "control reaches end of non-void function".

+      return to_mode_ == Mode::MODE_NTP ? kMinOpacity : kMaxOpacity;
+  }
+}
+
+double ToolbarSearchAnimator::GetSeparatorOpacity() const {
+  switch (separator_animate_state_) {
+    case ANIMATE_STATE_RUNNING:
+      return separator_animation_.CurrentValueBetween(kMinOpacity, kMaxOpacity);
+    case ANIMATE_STATE_NONE:
+    // ANIMATE_STATE_WAITING is not applicable for separator fading, but specify
+    // here to be complete.
+    case ANIMATE_STATE_WAITING:
+    default:

[dhollowa, 2012/07/24 00:24:49]

You can remove |default:|.

[kuan, 2012/07/30 23:21:02]

ditto.

+      return to_mode_ == Mode::MODE_DEFAULT ? kMaxOpacity : kMinOpacity;
   }
 }
 
@@ -70,70 +93,158 @@ void ToolbarSearchAnimator::RemoveObserver(
 }
 
 void ToolbarSearchAnimator::ModeChanged(const Mode& mode) {
-  int delay_ms = kBackgroundChangeDelayMs *
-      InstantUI::GetSlowAnimationScaleFactor();
-  if (mode.is_search() && mode.animate &&
-      animate_state_ == ANIMATE_STATE_NONE) {
+  // Only check to animate if modes have been intiailized via the very first
+  // ModeChanged call; this prevents the undesirable effect of animating from
+  // the default initialized mode of |DEFAULT| to NTP or SEARCH mode, e.g. to

[dhollowa, 2012/07/24 00:24:49]

nit: ...|NTP| or |SEARCH|...  and various comments below.

[kuan, 2012/07/30 23:21:02]

Done.

+  // fade out toolbar separator.
+  if (!modes_initialized_) {
+    from_mode_ = mode.mode;

[dhollowa, 2012/07/24 00:24:49]

nit: from_mode_ = to_mode_ = mode.mode;

[kuan, 2012/07/30 23:21:02]

Done.

+    to_mode_ = from_mode_;
+    modes_initialized_ = true;
+    return;
+  }
+
+  Mode::Type old_to_mode = to_mode_;
+  to_mode_ = mode.mode;
+
+  // If mode transitions from NTP to SEARCH and we're not animating background,
+  // start timer to fade in gradient background.
+  // TODO(kuan): check with UX if we need to animate from gradient to flat when
+  // mode transitions from SEARCH/DEFAULT to NTP.
+  if (mode.animate && from_mode_ == Mode::MODE_NTP && mode.is_search() &&

[dhollowa, 2012/07/24 00:24:49]

This would read more clearly if you used s/mode.is_search()/to_mode_ ==
Mode::MODE_NTP/.

[kuan, 2012/07/30 23:21:02]

Done.

+      background_animate_state_ == ANIMATE_STATE_NONE) {
     background_change_timer_.Start(
         FROM_HERE,
-        base::TimeDelta::FromMilliseconds(delay_ms),
+        base::TimeDelta::FromMilliseconds(kBackgroundChangeDelayMs *
+            InstantUI::GetSlowAnimationScaleFactor()),
         this,
         &ToolbarSearchAnimator::StartBackgroundChange);
-    animate_state_ = ANIMATE_STATE_WAITING;
+    background_animate_state_ = ANIMATE_STATE_WAITING;
+    return;
+  }
+
+  // If mode transitions from non-DEFAULT to DEFAULT, fade in separator;
+  // if we're already fading out separator, reverse to fade it in.
+  // Note that if we're still fading in gradient background when this mode
+  // transition happens (e.g. when user quickly hits enter on the auto-completed
+  // query in omnibox, triggering mode to transition from SEARCH to DEFAULT),
+  // we should continue fading in the gradient background to its full course.
+  // TODO(kuan): however, for now, sometimes, the first auto-completed query in
+  // omnibox is pre-rendered and gets swapped in when chosen by user, causing
+  // OnTabDetached for original tab to be fired, and hence causing us to Reset,
+  // immediately jumping to end state of background animation.  I'll look into
+  // this together with the above TODO for reverse background animation.
+  if (mode.animate &&
+      ((from_mode_ != Mode::MODE_DEFAULT && mode.is_default()) ||

[dhollowa, 2012/07/24 00:24:49]

You're guaranteed that from_mode_ is not equal to to_mode_, right?  So this
clause could simply be |to_mode_ == Mode::MODE_DEFAULT|.

[kuan, 2012/07/30 23:21:02]

Done.

+       (separator_animate_state_ == ANIMATE_STATE_RUNNING &&
+        separator_animation_.IsClosing()))) {
+    if (separator_animate_state_ == ANIMATE_STATE_RUNNING) {
+      from_mode_ = old_to_mode;

[dhollowa, 2012/07/24 00:24:49]

This conditional setting of the |from_mode_| variable seems fragile and
complicated.  Would it simplify things if the SearchModelObserver interface gave
it to you instead?  Seems like it might.

[kuan, 2012/07/30 23:21:02]

Done.

+    } else {
+      separator_animation_.Reset(0.0);
+      separator_animate_state_ = ANIMATE_STATE_RUNNING;
+    }
+    separator_animation_.Show();
+    return;
+  }
+
+  // If mode transitions from DEFAULT to non-DEFAULT, fade out separator;
+  // if we're already fading in separator, reverse to fade it out.
+  // TODO(kuan): for now, if we're still fading in gradient background when this
+  // mode transition happens (e.g. when user quickly hits back button on a
+  // website that he/she just entered from a SEARCH mode, triggering mode to
+  // transition from DEFAULT to NTP), we'll continue fading in the gradient
+  // background to its full course, which is undesirable.  I'll handle this
+  // together with the above TODO for reverse background animation.
+  if (mode.animate &&
+      ((from_mode_ == Mode::MODE_DEFAULT && !mode.is_default()) ||
+       (separator_animate_state_ == ANIMATE_STATE_RUNNING &&
+        separator_animation_.IsShowing()))) {
+    if (separator_animate_state_ == ANIMATE_STATE_RUNNING) {
+      from_mode_ = old_to_mode;
+    } else {
+      separator_animation_.Reset(1.0);
+      separator_animate_state_ = ANIMATE_STATE_RUNNING;
+    }
+    separator_animation_.Hide();
     return;
   }
-  // For all other cases, reset |animate_state_| and stop timer or animation.
+
+  // For all other cases, reset animate states and stop timer or animation(s).
   // Stopping animation will jump to the end of it.
   Reset(NULL);
 }
 
 void ToolbarSearchAnimator::AnimationProgressed(
     const ui::Animation* animation) {
-  DCHECK_EQ(animation, background_animation_.get());
-  animate_state_ = ANIMATE_STATE_RUNNING;
-  FOR_EACH_OBSERVER(ToolbarSearchAnimatorObserver, observers_,
-                    OnToolbarBackgroundAnimatorProgressed());
+  if (animation == &background_animation_) {
+    FOR_EACH_OBSERVER(ToolbarSearchAnimatorObserver, observers_,
+                      OnToolbarBackgroundAnimatorProgressed());
+    return;
+  }
+
+  if (animation == &separator_animation_) {
+    FOR_EACH_OBSERVER(ToolbarSearchAnimatorObserver, observers_,
+                      OnToolbarSeparatorAnimatorProgressed());
+  }
 }
 
 void ToolbarSearchAnimator::AnimationEnded(const ui::Animation* animation) {
-  DCHECK_EQ(animation, background_animation_.get());
-  // Only notify observers via OnToolbarBackgroundAnimatorProgressed if the
-  // animation has runs its full course i.e |animate_state_| is still
-  // ANIMATE_STATE_RUNNING.
-  // Animation that is canceled, i.e. |animate_state_| has been set to
+  from_mode_ = to_mode_;
+
+  // Only notify observers via OnToolbarBackgroundAnimatorProgressed or
+  // OnToolbarSeparatorAnimatorProgressed if the animation has runs its full
+  // course i.e animate state is still ANIMATE_STATE_RUNNING.
+  // Animation that is canceled, i.e. animate state has been set to
   // ANIMATE_STATE_NONE in Reset, should notify observers via
-  // OnToolbarBackgroundAnimatorCanceled.
-  if (animate_state_ == ANIMATE_STATE_RUNNING) {
-    animate_state_ = ANIMATE_STATE_NONE;
-    FOR_EACH_OBSERVER(ToolbarSearchAnimatorObserver, observers_,
-                      OnToolbarBackgroundAnimatorProgressed());
+  // OnToolbarBackgroundAnimatorCanceled or OnToolbarSeparatorAnimatorCanceled.
+  if (animation == &background_animation_) {
+    if (background_animate_state_ == ANIMATE_STATE_RUNNING) {
+      background_animate_state_ = ANIMATE_STATE_NONE;
+      FOR_EACH_OBSERVER(ToolbarSearchAnimatorObserver, observers_,
+                        OnToolbarBackgroundAnimatorProgressed());
+    }
+    return;
+  }
+
+  if (animation == &separator_animation_) {
+    if (separator_animate_state_ == ANIMATE_STATE_RUNNING) {
+      separator_animate_state_ = ANIMATE_STATE_NONE;
+      FOR_EACH_OBSERVER(ToolbarSearchAnimatorObserver, observers_,
+                        OnToolbarSeparatorAnimatorProgressed());
+    }
   }
 }
 
 void ToolbarSearchAnimator::StartBackgroundChange() {
-  if (!background_animation_.get()) {
-    background_animation_.reset(new ui::SlideAnimation(this));
-    background_animation_->SetTweenType(ui::Tween::LINEAR);
-    background_animation_->SetSlideDuration(
-        kBackgroundChangeDurationMs * InstantUI::GetSlowAnimationScaleFactor());
-  }
-  background_animation_->Reset(0.0);
-  background_animation_->Show();
+  background_animation_.Reset(0.0);
+  background_animation_.Show();
+  background_animate_state_ = ANIMATE_STATE_RUNNING;
 }
 
 void ToolbarSearchAnimator::Reset(TabContents* tab_contents) {
-  bool notify_observers = animate_state_ != ANIMATE_STATE_NONE;
-  animate_state_ = ANIMATE_STATE_NONE;
+  bool notify_background_observers =
+      background_animate_state_ != ANIMATE_STATE_NONE;
+  bool notify_separator_observers =
+      separator_animate_state_ != ANIMATE_STATE_NONE;
+  background_animate_state_ = ANIMATE_STATE_NONE;
+  separator_animate_state_ = ANIMATE_STATE_NONE;
+  from_mode_ = to_mode_;
   background_change_timer_.Stop();
   // If animation is still running, stopping it will trigger AnimationEnded
-  // where we've prevented from notifying observers via BackgroundChanged;
-  // see comments in AnimationEnded.
-  if (background_animation_.get())
-    background_animation_->Stop();
-  if (notify_observers) {
+  // where we've prevented from firing OnToolbarBackgroundAnimatorProgressed
+  // and OnToolbarSeparatorAnimatorProgressed notifications; see comments in
+  // AnimationEnded.
+  background_animation_.Stop();
+  separator_animation_.Stop();
+  if (notify_background_observers) {
     FOR_EACH_OBSERVER(ToolbarSearchAnimatorObserver, observers_,
                       OnToolbarBackgroundAnimatorCanceled(tab_contents));
   }
+  if (notify_separator_observers) {
+    FOR_EACH_OBSERVER(ToolbarSearchAnimatorObserver, observers_,
+                      OnToolbarSeparatorAnimatorCanceled());
+  }
 }
 
 }  // namespace search