Fix and simplify bookmark bar detach/attach animation. (Shown on NTP)

Problem 1: the cross-fading was not executed properly. It should start by
drawing the attached bar at full opacity and transition to detached by
painting detached on top of that at partial opacity (defined by
animation state).

Problem 2: it painted as fully detached when current_state was 0. At this
point it should be painting as fully attached. This led to a single-frame
flash of lightness at the start of the attached->detached animation.

BUG=none

Review URL: https://codereview.chromium.org/1547563003

Cr-Commit-Position: refs/heads/master@{#367094}

chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc

@@ -547,6 +547,7 @@ BookmarkBarView::BookmarkBarView(Browser* browser, BrowserView* browser_view)
       browser_(browser),
       browser_view_(browser_view),
       infobar_visible_(false),
+      size_animation_(this),
       throbbing_view_(NULL),
       bookmark_bar_state_(BookmarkBar::SHOW),
       animating_detached_(false),
@@ -554,7 +555,7 @@ BookmarkBarView::BookmarkBarView(Browser* browser, BrowserView* browser_view)
   set_id(VIEW_ID_BOOKMARK_BAR);
   Init();
 
-  size_animation_->Reset(1);
+  size_animation_.Reset(1);
 }
 
 BookmarkBarView::~BookmarkBarView() {
@@ -603,19 +604,15 @@ void BookmarkBarView::SetBookmarkBarState(
     animating_detached_ = (state == BookmarkBar::DETACHED ||
                            bookmark_bar_state_ == BookmarkBar::DETACHED);
     if (state == BookmarkBar::SHOW)
-      size_animation_->Show();
+      size_animation_.Show();
     else
-      size_animation_->Hide();
+      size_animation_.Hide();
   } else {
-    size_animation_->Reset(state == BookmarkBar::SHOW ? 1 : 0);
+    size_animation_.Reset(state == BookmarkBar::SHOW ? 1 : 0);
   }
   bookmark_bar_state_ = state;
 }
 
-bool BookmarkBarView::is_animating() {
-  return size_animation_->is_animating();
-}
-
 const BookmarkNode* BookmarkBarView::GetNodeForButtonAtModelIndex(
     const gfx::Point& loc,
     int* model_start_index) {
@@ -753,11 +750,7 @@ base::string16 BookmarkBarView::CreateToolTipForURLAndTitle(
 
 bool BookmarkBarView::IsDetached() const {
   return (bookmark_bar_state_ == BookmarkBar::DETACHED) ||
-      (animating_detached_ && size_animation_->is_animating());
-}
-
-double BookmarkBarView::GetAnimationValue() const {
-  return size_animation_->GetCurrentValue();
+      (animating_detached_ && size_animation_.is_animating());
 }
 
 int BookmarkBarView::GetToolbarOverlap() const {
@@ -777,7 +770,7 @@ int BookmarkBarView::GetToolbarOverlap() const {
   // detached states.
   return detached_overlap + static_cast<int>(
       (attached_overlap - detached_overlap) *
-          size_animation_->GetCurrentValue());
+          size_animation_.GetCurrentValue());
 }
 
 gfx::Size BookmarkBarView::GetPreferredSize() const {
@@ -787,10 +780,10 @@ gfx::Size BookmarkBarView::GetPreferredSize() const {
         chrome::kBookmarkBarHeight +
         static_cast<int>(
             (chrome::kNTPBookmarkBarHeight - chrome::kBookmarkBarHeight) *
-            (1 - size_animation_->GetCurrentValue())));
+            (1 - size_animation_.GetCurrentValue())));
   } else {
     prefsize.set_height(static_cast<int>(chrome::kBookmarkBarHeight *
-                                         size_animation_->GetCurrentValue()));
+                                         size_animation_.GetCurrentValue()));
   }
   return prefsize;
 }
@@ -818,7 +811,7 @@ gfx::Size BookmarkBarView::GetMinimumSize() const {
 
   int height = chrome::kBookmarkBarHeight;
   if (IsDetached()) {
-    double current_state = 1 - size_animation_->GetCurrentValue();
+    double current_state = 1 - size_animation_.GetCurrentValue();
     width += 2 * static_cast<int>(kNewTabHorizontalPadding * current_state);
     height += static_cast<int>(
         (chrome::kNTPBookmarkBarHeight - chrome::kBookmarkBarHeight) *
@@ -866,7 +859,7 @@ void BookmarkBarView::Layout() {
   int separator_margin = kSeparatorMargin;
 
   if (IsDetached()) {
-    double current_state = 1 - size_animation_->GetCurrentValue();
+    double current_state = 1 - size_animation_.GetCurrentValue();
     x += static_cast<int>(kNewTabHorizontalPadding * current_state);
     y += (View::height() - chrome::kBookmarkBarHeight) / 2;
     width -= static_cast<int>(kNewTabHorizontalPadding * current_state);
@@ -1386,8 +1379,8 @@ void BookmarkBarView::WriteDragDataForView(View* sender,
 
 int BookmarkBarView::GetDragOperationsForView(View* sender,
                                               const gfx::Point& p) {
-  if (size_animation_->is_animating() ||
-      (size_animation_->GetCurrentValue() == 0 &&
+  if (size_animation_.is_animating() ||
+      (size_animation_.GetCurrentValue() == 0 &&
        bookmark_bar_state_ != BookmarkBar::DETACHED)) {
     // Don't let the user drag while animating open or we're closed (and not
     // detached, when detached size_animation_ is always 0). This typically is
@@ -1599,8 +1592,6 @@ void BookmarkBarView::Init() {
 
   set_context_menu_controller(this);
 
-  size_animation_.reset(new gfx::SlideAnimation(this));
-
   model_ = BookmarkModelFactory::GetForProfile(browser_->profile());
   managed_ = ManagedBookmarkServiceFactory::GetForProfile(browser_->profile());
   if (model_) {

chrome/browser/ui/views/bookmarks/bookmark_bar_view.h

@@ -20,6 +20,7 @@
 #include "components/bookmarks/browser/bookmark_model_observer.h"
 #include "components/bookmarks/browser/bookmark_node_data.h"
 #include "ui/gfx/animation/animation_delegate.h"
+#include "ui/gfx/animation/slide_animation.h"
 #include "ui/views/accessible_pane_view.h"
 #include "ui/views/context_menu_controller.h"
 #include "ui/views/controls/button/button.h"
@@ -43,10 +44,6 @@ namespace content {
 class PageNavigator;
 }
 
-namespace gfx {
-class SlideAnimation;
-}
-
 namespace views {
 class CustomButton;
 class MenuButton;
@@ -101,9 +98,6 @@ class BookmarkBarView : public views::AccessiblePaneView,
   void SetBookmarkBarState(BookmarkBar::State state,
                            BookmarkBar::AnimateChangeType animate_type);
 
-  // Whether or not we are animating.
-  bool is_animating();
-
   // If |loc| is over a bookmark button the node is returned corresponding to
   // the button and |model_start_index| is set to 0. If a overflow button is
   // showing and |loc| is over the overflow button, the bookmark bar node is
@@ -129,6 +123,10 @@ class BookmarkBarView : public views::AccessiblePaneView,
   // Returns the button used when not all the items on the bookmark bar fit.
   views::MenuButton* overflow_button() const { return overflow_button_; }
 
+  const gfx::Animation& size_animation() {
+    return size_animation_;
+  }
+
   // Returns the active MenuItemView, or NULL if a menu isn't showing.
   views::MenuItemView* GetMenu();
 
@@ -158,9 +156,6 @@ class BookmarkBarView : public views::AccessiblePaneView,
   // Returns true if Bookmarks Bar is currently detached from the Toolbar.
   bool IsDetached() const;
 
-  // Returns the current state of the resize animation (show/hide).
-  double GetAnimationValue() const;
-
   // Returns the current amount of overlap atop the browser toolbar.
   int GetToolbarOverlap() const;
 
@@ -446,7 +441,7 @@ class BookmarkBarView : public views::AccessiblePaneView,
   bool infobar_visible_;
 
   // Animation controlling showing and hiding of the bar.
-  scoped_ptr<gfx::SlideAnimation> size_animation_;
+  gfx::SlideAnimation size_animation_;
 
   // If the bookmark bubble is showing, this is the visible ancestor of the URL.
   // The visible ancestor is either the |other_bookmarks_button_|,

chrome/browser/ui/views/frame/browser_view.cc

@@ -416,55 +416,25 @@ BookmarkBarViewBackground::BookmarkBarViewBackground(
 void BookmarkBarViewBackground::Paint(gfx::Canvas* canvas,
                                       views::View* view) const {
   int toolbar_overlap = bookmark_bar_view_->GetToolbarOverlap();
-  if (!bookmark_bar_view_->IsDetached()) {
+
+  SkAlpha detached_alpha = static_cast<SkAlpha>(
+      bookmark_bar_view_->size_animation().CurrentValueBetween(0xff, 0));
+  if (detached_alpha != 0xff) {
     PaintAttachedBookmarkBar(canvas,
                              bookmark_bar_view_,
                              browser_view_,
                              browser_->host_desktop_type(),
                              toolbar_overlap);
-    return;
   }
 
-  // As 'hidden' according to the animation is the full in-tab state, we invert
-  // the value - when current_state is at '0', we expect the bar to be docked.
-  double current_state = 1 - bookmark_bar_view_->GetAnimationValue();
-
-  if (current_state == 0.0 || current_state == 1.0) {
-    PaintDetachedBookmarkBar(canvas, bookmark_bar_view_, browser_->profile());
+  if (!bookmark_bar_view_->IsDetached() || detached_alpha == 0)
     return;
-  }
+
   // While animating, set opacity to cross-fade between attached and detached
   // backgrounds including their respective separators.
-  int detached_alpha = static_cast<uint8_t>(current_state * 255);
-  int attached_alpha = 255 - detached_alpha;
-  if (browser_->bookmark_bar_state() == BookmarkBar::DETACHED) {
-    // To animate from attached to detached state:
-    // - fade out attached background
-    // - fade in detached background.
-    canvas->SaveLayerAlpha(attached_alpha);
-    PaintAttachedBookmarkBar(canvas,
-                             bookmark_bar_view_,
-                             browser_view_,
-                             browser_->host_desktop_type(),
-                             toolbar_overlap);
-    canvas->Restore();
-    canvas->SaveLayerAlpha(detached_alpha);
-    PaintDetachedBookmarkBar(canvas, bookmark_bar_view_, browser_->profile());
-  } else {
-    // To animate from detached to attached state:
-    // - fade out detached background
-    // - fade in attached background.
   canvas->SaveLayerAlpha(detached_alpha);
   PaintDetachedBookmarkBar(canvas, bookmark_bar_view_, browser_->profile());
   canvas->Restore();
-    canvas->SaveLayerAlpha(attached_alpha);
-    PaintAttachedBookmarkBar(canvas,
-                             bookmark_bar_view_,
-                             browser_view_,
-                             browser_->host_desktop_type(),
-                             toolbar_overlap);
-  }
-  canvas->Restore();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -1278,7 +1248,8 @@ bool BrowserView::IsBookmarkBarVisible() const {
 }
 
 bool BrowserView::IsBookmarkBarAnimating() const {
-  return bookmark_bar_view_.get() && bookmark_bar_view_->is_animating();
+  return bookmark_bar_view_.get() &&
+         bookmark_bar_view_->size_animation().is_animating();
 }
 
 bool BrowserView::IsTabStripEditable() const {