Fix multiple problems with infobar/bookmark bar resizing:

  * The infobar wouldn't tell the browser it was done animating when the height hadn't also changed from the previously-calculated height, so frequently the content wouldn't properly relayout after infobar animation.  I think this was the cause of bug 79108.
  * A silly bug in ToolbarSizeChanged() meant that we never did re-layout on the content area when the toolbar stopped animating, because we were checking for "call_state == NORMAL" while the AutoReset object was still alive and guaranteeing that the state would not be NORMAL.  This caused bug 80142.
  * I reversed the enum values for the recursive call state, so recursive ToolbarSizeChanged() calls were all getting "can use fast resize" backwards.

BUG=79108,80142
TEST=Visit a page with a vertical scrollbar.  Toggle the bookmark bar on.  Once the bar stops animating the scrollbar should redraw so it's once again fully on-screen.
Review URL: http://codereview.chromium.org/6927032

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@84184 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 48 insertions, 37 deletions

diff --git a/chrome/browser/external_tab_container_win.cc b/chrome/browser/external_tab_container_win.cc
index 38fc80c..b8625fe 100644
--- a/chrome/browser/external_tab_container_win.cc
+++ b/chrome/browser/external_tab_container_win.cc
@@ -917,7 +917,7 @@ SkColor ExternalTabContainer::GetInfoBarSeparatorColor() const {
   return ResourceBundle::toolbar_separator_color;
 }
 
-void ExternalTabContainer::InfoBarContainerHeightChanged(bool is_animating) {
+void ExternalTabContainer::InfoBarContainerStateChanged(bool is_animating) {
   if (external_tab_view_)
     external_tab_view_->Layout();
 }
diff --git a/chrome/browser/external_tab_container_win.h b/chrome/browser/external_tab_container_win.h
index 6d9657f..100ddb2b 100644
--- a/chrome/browser/external_tab_container_win.h
+++ b/chrome/browser/external_tab_container_win.h
@@ -205,7 +205,7 @@ class ExternalTabContainer : public TabContentsDelegate,
 
   // InfoBarContainer::Delegate overrides
   virtual SkColor GetInfoBarSeparatorColor() const OVERRIDE;
-  virtual void InfoBarContainerHeightChanged(bool is_animating) OVERRIDE;
+  virtual void InfoBarContainerStateChanged(bool is_animating) OVERRIDE;
   virtual bool DrawInfoBarArrows(int* x) const OVERRIDE;
 
   virtual void TabContentsCreated(TabContents* new_contents);
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc
index 4e69a22..263d218 100644
--- a/chrome/browser/ui/views/frame/browser_view.cc
+++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -645,17 +645,20 @@ void BrowserView::ToolbarSizeChanged(bool is_animating) {
   // Inform the InfoBarContainer that the distance to the location icon may have
   // changed.  We have to do this after the block above so that the toolbars are
   // laid out correctly for calculating the maximum arrow height below.
-  AutoReset<CallState> resetter(&call_state,
-      is_animating ? REENTRANT : REENTRANT_FORCE_FAST_RESIZE);
-  const LocationIconView* location_icon_view =
-      toolbar_->location_bar()->location_icon_view();
-  // The +1 in the next line creates a 1-px gap between icon and arrow tip.
-  gfx::Point icon_bottom(0, location_icon_view->GetImageBounds().bottom() -
-      LocationBarView::kIconInternalPadding + 1);
-  ConvertPointToView(location_icon_view, this, &icon_bottom);
-  gfx::Point infobar_top(0, infobar_container_->GetVerticalOverlap(NULL));
-  ConvertPointToView(infobar_container_, this, &infobar_top);
-  infobar_container_->SetMaxTopArrowHeight(infobar_top.y() - icon_bottom.y());
+  {
+    const LocationIconView* location_icon_view =
+        toolbar_->location_bar()->location_icon_view();
+    // The +1 in the next line creates a 1-px gap between icon and arrow tip.
+    gfx::Point icon_bottom(0, location_icon_view->GetImageBounds().bottom() -
+        LocationBarView::kIconInternalPadding + 1);
+    ConvertPointToView(location_icon_view, this, &icon_bottom);
+    gfx::Point infobar_top(0, infobar_container_->GetVerticalOverlap(NULL));
+    ConvertPointToView(infobar_container_, this, &infobar_top);
+
+    AutoReset<CallState> resetter(&call_state,
+        is_animating ? REENTRANT_FORCE_FAST_RESIZE : REENTRANT);
+    infobar_container_->SetMaxTopArrowHeight(infobar_top.y() - icon_bottom.y());
+  }
 
   // When transitioning from animating to not animating we need to make sure the
   // contents_container_ gets layed out. If we don't do this and the bounds
@@ -1710,7 +1713,7 @@ SkColor BrowserView::GetInfoBarSeparatorColor() const {
       ResourceBundle::toolbar_separator_color : SK_ColorBLACK;
 }
 
-void BrowserView::InfoBarContainerHeightChanged(bool is_animating) {
+void BrowserView::InfoBarContainerStateChanged(bool is_animating) {
   ToolbarSizeChanged(is_animating);
 }
 
diff --git a/chrome/browser/ui/views/frame/browser_view.h b/chrome/browser/ui/views/frame/browser_view.h
index a484cc0..0184d14 100644
--- a/chrome/browser/ui/views/frame/browser_view.h
+++ b/chrome/browser/ui/views/frame/browser_view.h
@@ -377,7 +377,7 @@ class BrowserView : public BrowserBubbleHost,
 
   // InfoBarContainer::Delegate overrides
   virtual SkColor GetInfoBarSeparatorColor() const OVERRIDE;
-  virtual void InfoBarContainerHeightChanged(bool is_animating) OVERRIDE;
+  virtual void InfoBarContainerStateChanged(bool is_animating) OVERRIDE;
   virtual bool DrawInfoBarArrows(int* x) const OVERRIDE;
 
   // views::SingleSplitView::Observer overrides:
diff --git a/chrome/browser/ui/views/infobars/infobar.cc b/chrome/browser/ui/views/infobars/infobar.cc
index 49ca2bc..e31b161 100644
--- a/chrome/browser/ui/views/infobars/infobar.cc
+++ b/chrome/browser/ui/views/infobars/infobar.cc
@@ -51,12 +51,12 @@ void InfoBar::SetArrowTargetHeight(int height) {
   // target height.
   if ((arrow_target_height_ != height) && !animation()->IsClosing()) {
     arrow_target_height_ = height;
-    RecalculateHeights();
+    RecalculateHeights(false);
   }
 }
 
 void InfoBar::AnimationProgressed(const ui::Animation* animation) {
-  RecalculateHeights();
+  RecalculateHeights(false);
 }
 
 void InfoBar::RemoveInfoBar() {
@@ -67,7 +67,7 @@ void InfoBar::RemoveInfoBar() {
 void InfoBar::SetBarTargetHeight(int height) {
   if (bar_target_height_ != height) {
     bar_target_height_ = height;
-    RecalculateHeights();
+    RecalculateHeights(false);
   }
 }
 
@@ -78,11 +78,15 @@ int InfoBar::OffsetY(const gfx::Size& prefsize) const {
 }
 
 void InfoBar::AnimationEnded(const ui::Animation* animation) {
-  RecalculateHeights();
+  // When the animation ends, we must ensure the container is notified even if
+  // the heights haven't changed, lest it never get an "animation finished"
+  // notification.  (If the browser doesn't get this notification, it will not
+  // bother to re-layout the content area for the new infobar size.)
+  RecalculateHeights(true);
   MaybeDelete();
 }
 
-void InfoBar::RecalculateHeights() {
+void InfoBar::RecalculateHeights(bool force_notify) {
   int old_arrow_height = arrow_height_;
   int old_bar_height = bar_height_;
 
@@ -117,11 +121,13 @@ void InfoBar::RecalculateHeights() {
 
   // Don't re-layout if nothing has changed, e.g. because the animation step was
   // not large enough to actually change the heights by at least a pixel.
-  if ((old_arrow_height != arrow_height_) || (old_bar_height != bar_height_)) {
+  bool heights_differ =
+      (old_arrow_height != arrow_height_) || (old_bar_height != bar_height_);
+  if (heights_differ)
     PlatformSpecificOnHeightsRecalculated();
-    if (container_)
-      container_->OnInfoBarHeightChanged(animation_->is_animating());
-  }
+
+  if (container_ && (heights_differ || force_notify))
+    container_->OnInfoBarStateChanged(animation_->is_animating());
 }
 
 void InfoBar::MaybeDelete() {
diff --git a/chrome/browser/ui/views/infobars/infobar.h b/chrome/browser/ui/views/infobars/infobar.h
index c863ce9..cb4aef7 100644
--- a/chrome/browser/ui/views/infobars/infobar.h
+++ b/chrome/browser/ui/views/infobars/infobar.h
@@ -86,9 +86,10 @@ class InfoBar : public ui::AnimationDelegate {
   virtual void AnimationEnded(const ui::Animation* animation) OVERRIDE;
 
   // Finds the new desired arrow and bar heights, and if they differ from the
-  // current ones, calls PlatformSpecificOnHeightRecalculated() and informs our
-  // container our height has changed.
-  void RecalculateHeights();
+  // current ones, calls PlatformSpecificOnHeightRecalculated().  Informs our
+  // container our state has changed if either the heights have changed or
+  // |force_notify| is set.
+  void RecalculateHeights(bool force_notify);
 
   // Checks whether we're closed.  If so, notifies the container that it should
   // remove us (which will cause the platform-specific code to asynchronously
diff --git a/chrome/browser/ui/views/infobars/infobar_container.cc b/chrome/browser/ui/views/infobars/infobar_container.cc
index b451ae7..2e87678 100644
--- a/chrome/browser/ui/views/infobars/infobar_container.cc
+++ b/chrome/browser/ui/views/infobars/infobar_container.cc
@@ -58,7 +58,7 @@ void InfoBarContainer::ChangeTabContents(TabContents* contents) {
   }
 
   // Now that everything is up to date, signal the delegate to re-layout.
-  OnInfoBarHeightChanged(false);
+  OnInfoBarStateChanged(false);
 }
 
 int InfoBarContainer::GetVerticalOverlap(int* total_height) {
@@ -88,9 +88,9 @@ void InfoBarContainer::SetMaxTopArrowHeight(int height) {
   UpdateInfoBarArrowTargetHeights();
 }
 
-void InfoBarContainer::OnInfoBarHeightChanged(bool is_animating) {
+void InfoBarContainer::OnInfoBarStateChanged(bool is_animating) {
   if (delegate_)
-    delegate_->InfoBarContainerHeightChanged(is_animating);
+    delegate_->InfoBarContainerStateChanged(is_animating);
 }
 
 void InfoBarContainer::RemoveDelegate(InfoBarDelegate* delegate) {
@@ -108,7 +108,7 @@ void InfoBarContainer::RemoveInfoBar(InfoBar* infobar) {
 void InfoBarContainer::RemoveAllInfoBarsForDestruction() {
   // Before we remove any children, we reset |delegate_|, so that no removals
   // will result in us trying to call
-  // delegate_->InfoBarContainerHeightChanged().  This is important because at
+  // delegate_->InfoBarContainerStateChanged().  This is important because at
   // this point |delegate_| may be shutting down, and it's at best unimportant
   // and at worst disastrous to call that.
   delegate_ = NULL;
diff --git a/chrome/browser/ui/views/infobars/infobar_container.h b/chrome/browser/ui/views/infobars/infobar_container.h
index 35c789d..33bcdb6 100644
--- a/chrome/browser/ui/views/infobars/infobar_container.h
+++ b/chrome/browser/ui/views/infobars/infobar_container.h
@@ -31,8 +31,9 @@ class InfoBarContainer : public NotificationObserver {
     // The separator color may vary depending on where the container is hosted.
     virtual SkColor GetInfoBarSeparatorColor() const = 0;
 
-    // The delegate is notified each time the infobar container changes height.
-    virtual void InfoBarContainerHeightChanged(bool is_animating) = 0;
+    // The delegate is notified each time the infobar container changes height,
+    // as well as when it stops animating.
+    virtual void InfoBarContainerStateChanged(bool is_animating) = 0;
 
     // The delegate needs to tell us whether "unspoofable" arrows should be
     // drawn, and if so, at what |x| coordinate.  |x| may not be NULL.
@@ -62,14 +63,14 @@ class InfoBarContainer : public NotificationObserver {
   // desired.
   //
   // IMPORTANT: This MUST NOT result in a call back to
-  // Delegate::InfoBarContainerHeightChanged() unless it causes an actual
+  // Delegate::InfoBarContainerStateChanged() unless it causes an actual
   // change, lest we infinitely recurse.
   void SetMaxTopArrowHeight(int height);
 
   // Called when a contained infobar has animated or by some other means changed
-  // its height.  The container is expected to do anything necessary to respond,
-  // e.g. re-layout.
-  void OnInfoBarHeightChanged(bool is_animating);
+  // its height, or when it stops animating.  The container is expected to do
+  // anything necessary to respond, e.g. re-layout.
+  void OnInfoBarStateChanged(bool is_animating);
 
   // Removes the specified InfoBarDelegate from the selected TabContents.  This
   // will notify us back and cause us to close the InfoBar.  This is called from