alternate ntp: implement Show/HideBars API to reduce jank when showing/hiding bars

chrome/browser/infobars/infobar_container.cc

 // Copyright (c) 2012 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 "build/build_config.h"
 
 // TODO(pkasting): Port Mac to use this.
 #if defined(TOOLKIT_VIEWS) || defined(TOOLKIT_GTK)
 
 #include "chrome/browser/infobars/infobar_container.h"
@@ skipping 27 matching lines @@
 }
 
 InfoBarContainer::~InfoBarContainer() {
   // RemoveAllInfoBarsForDestruction() should have already cleared our infobars.
   DCHECK(infobars_.empty());
   if (search_model_)
     search_model_->RemoveObserver(this);
 }
 
 void InfoBarContainer::ChangeInfoBarService(InfoBarService* infobar_service) {
-  registrar_.RemoveAll();
+  HideAllInfoBars();

[Peter Kasting, 2013/03/16 00:40:59]

We don't need to do this here.  If the search model says the top bars aren't
visible, then we must have gotten a call to ModelChanged() in the past at the
time we wanted to make the bars invisible, which would have hidden all the bars.
 Since then, we've never re-added any bars because this function will early-exit
and Observe() won't be reached (since we're not listening to any notifications).
 So there will never be any bars visible in that case, hence the early exit
before calling HideAllBars() is OK.

[kuan, 2013/03/18 15:37:29]

switching between tabs doesn't work correctly if i don't call HideAllInfoBars().
this is the scenario i tried:
1) forced a crash infobar
2) open 1st ntp, crash infobar shows: expected
3) open 2nd ntp, crash infobar still shows: this is incorrect - crash infobar
shld not show up for subsequent tabs
4) switch back to 1st tab: ChangeInfoBarService crashes due to duplicate
registration when adding notifications to registrar. 

the scenario doesn't trigger or involve ModelChanged from 1993. 
ChangeInfoBarService is called at steps 2 to 4 from
BrowserView::ActiveTabChanged, with search_model_->top_bars_visible() being
true.

+
+  infobar_service_ = infobar_service;
+
+  if (search_model_ && !search_model_->top_bars_visible())
+    return;
 
   // Note that HideAllInfoBars() sets |infobars_shown_| to false, because that's
   // what the other, Instant-related callers want; but here we actually
   // explicitly want to reset this variable to true.  So do that after calling
   // the function.
-  HideAllInfoBars();
   infobars_shown_ = true;
   infobars_shown_time_ = base::TimeTicks();
 
-  infobar_service_ = infobar_service;
   if (infobar_service_) {
     content::Source<InfoBarService> source(infobar_service_);
     registrar_.Add(this, chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_ADDED,
                    source);
     registrar_.Add(this, chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED,
                    source);
     registrar_.Add(this, chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REPLACED,
                    source);
 
     for (size_t i = 0; i < infobar_service_->GetInfoBarCount(); ++i) {
@@ skipping 62 matching lines @@
 
   for (size_t i = infobars_.size(); i > 0; --i)
     infobars_[i - 1]->CloseSoon();
 
   ChangeInfoBarService(NULL);
 }
 
 void InfoBarContainer::Observe(int type,
                                const content::NotificationSource& source,
                                const content::NotificationDetails& details) {
-  // When infobars are supposed to be hidden, we shouldn't try to hide or show
-  // anything in response to any notifications.  Once infobars get un-hidden
-  // via ChangeInfoBarService(), we'll get the updated set of visible infobars
-  // from the InfoBarService.
-  if (!infobars_shown_)
-    return;
+  // When infobars are hidden, we shouldn't be listening for notifications.
+  DCHECK(infobars_shown_);
 
   switch (type) {
     case chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_ADDED:
       AddInfoBar(
           content::Details<InfoBarAddedDetails>(details)->CreateInfoBar(
               infobar_service_),
           infobars_.size(), true, WANT_CALLBACK);
       break;
 
     case chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED: {
@@ skipping 10 matching lines @@
           HideInfoBar(replaced_details->first, false), false, WANT_CALLBACK);
       break;
     }
 
     default:
       NOTREACHED();
       break;
   }
 }
 
-void InfoBarContainer::ModeChanged(const chrome::search::Mode& old_mode,
-                                   const chrome::search::Mode& new_mode) {
-  // Hide infobars when showing Instant Extended suggestions.
-  if (new_mode.is_search_suggestions()) {
-    // If suggestions are being shown on a |DEFAULT| page, delay the hiding
-    // until notification that Instant overlay is ready is received via
-    // OverlayStateChanged(); this prevents jankiness caused by infobars hiding
-    // followed by suggestions appearing.
-    if (new_mode.is_origin_default())
-      return;
-    HideAllInfoBars();
-    OnInfoBarStateChanged(false);
-  } else {
+void InfoBarContainer::ModelChanged(
+    const chrome::search::SearchModel::State& old_state,
+    const chrome::search::SearchModel::State& new_state) {
+  if (!chrome::search::SearchModel::ShouldChangeTopBarsVisibility(old_state,
+                                                                  new_state))
+    return;
+
+  if (new_state.top_bars_visible && !infobars_shown_) {
     ChangeInfoBarService(infobar_service_);
     infobars_shown_time_ = base::TimeTicks::Now();
-  }
-}
-
-void InfoBarContainer::OverlayStateChanged(const InstantOverlayModel& model) {
-  // If suggestions are being shown on a |DEFAULT| page, hide the infobars now.
-  // See comments for ModeChanged() for explanation.
-  if (model.mode().is_search_suggestions() &&
-      model.mode().is_origin_default()) {
+  } else if (!new_state.top_bars_visible && infobars_shown_) {
     HideAllInfoBars();
     OnInfoBarStateChanged(false);
   }
 }
 
 size_t InfoBarContainer::HideInfoBar(InfoBarDelegate* delegate,
                                      bool use_animation) {
   bool should_animate = use_animation &&
       ((base::TimeTicks::Now() - infobars_shown_time_) >
           base::TimeDelta::FromMilliseconds(50));
@@ skipping 12 matching lines @@
       infobar->CloseSoon();
       UpdateInfoBarArrowTargetHeights();
       return position;
     }
   }
   NOTREACHED();
   return infobars_.size();
 }
 
 void InfoBarContainer::HideAllInfoBars() {
+  registrar_.RemoveAll();
+
   infobars_shown_ = false;
   while (!infobars_.empty()) {
     InfoBar* infobar = infobars_.front();
     // Inform the infobar that it's hidden.  If it was already closing, this
     // closes its delegate.
     infobar->Hide(false);
   }
 }
 
 void InfoBarContainer::AddInfoBar(InfoBar* infobar,
@@ skipping 29 matching lines @@
     return InfoBar::kDefaultArrowTargetHeight;
   // When the first infobar is animating closed, we animate the second infobar's
   // arrow target height from the default to the top target height.  Note that
   // the animation values here are going from 1.0 -> 0.0 as the top bar closes.
   return top_arrow_target_height_ + static_cast<int>(
       (InfoBar::kDefaultArrowTargetHeight - top_arrow_target_height_) *
           first_infobar_animation.GetCurrentValue());
 }
 
 #endif  // TOOLKIT_VIEWS || defined(TOOLKIT_GTK)