Alternate NTP: Don't hide bookmark bar on instant

chrome/browser/ui/browser.cc

 // Copyright 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 "chrome/browser/ui/browser.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #include <shellapi.h>
 #endif  // defined(OS_WIN)
@@ skipping 1242 matching lines @@
   // instant preview state.
   // ModeChanged() updates bookmark bar state for all mode transitions except
   // when transitioning from |NTP| to |SEARCH_SUGGESTIONS|, because that needs
   // to be done when the suggestions are ready.
   // If |mode| is |SEARCH_SUGGESTIONS| and bookmark bar is still showing
   // attached, the previous mode is definitely NTP; it won't be |DEFAULT|
   // because ModeChanged() would have handled that transition by hiding the
   // bookmark bar.
   if (mode.is_search_suggestions() &&
       bookmark_bar_state_ == BookmarkBar::SHOW) {
-    UpdateBookmarkBarState(BOOKMARK_BAR_STATE_CHANGE_INSTANT_PREVIEW_STATE);
+    UpdateBookmarkBarState(BOOKMARK_BAR_STATE_CHANGE_TAB_STATE);
   }
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // Browser, content::WebContentsDelegate implementation:
 
 WebContents* Browser::OpenURLFromTab(WebContents* source,
                                      const OpenURLParams& params) {
   chrome::NavigateParams nav_params(this, params.url, params.transition);
   nav_params.source_contents = source;
@@ skipping 957 matching lines @@
     BookmarkTabHelper* bookmark_tab_helper =
         web_contents ? BookmarkTabHelper::FromWebContents(web_contents) : NULL;
     if (bookmark_tab_helper && bookmark_tab_helper->ShouldShowBookmarkBar())
       state = BookmarkBar::DETACHED;
     else
       state = BookmarkBar::HIDDEN;
   }
 
   // Don't allow the bookmark bar to be shown in suggestions mode or
   // for instant extended api, non-NTP modes when it's detached.
-  if (search_model_->mode().is_search_suggestions() ||
-      (chrome::search::IsInstantExtendedAPIEnabled(profile_) &&
-       !search_model_->mode().is_ntp() && state == BookmarkBar::DETACHED)) {
-    state = BookmarkBar::HIDDEN;
+  // TODO(sail): Stop hidding the bookmark bar on other platforms.
+#if !defined(OS_MACOSX)
+  if (chrome::search::IsInstantExtendedAPIEnabled(profile_)) {
+    if (search_model_->mode().is_search_suggestions() ||
+        (!search_model_->mode().is_ntp() && state == BookmarkBar::DETACHED)) {

[dhollowa, 2013/01/18 00:55:11]

nit: no {} needed now

[sail, 2013/01/18 01:37:09]

I find braces around if statements with multi-line conditionals very help. Mind
if I keep it this way?

[dhollowa, 2013/01/18 15:59:58]

That's fine.

+      state = BookmarkBar::HIDDEN;
+    }
   }
+#endif
 
   if (state == bookmark_bar_state_)
     return;
 
   bookmark_bar_state_ = state;
 
   if (!window_)
     return;  // This is called from the constructor when window_ is NULL.
 
   if (reason == BOOKMARK_BAR_STATE_CHANGE_TAB_SWITCH) {
     // Don't notify BrowserWindow on a tab switch as at the time this is invoked
     // BrowserWindow hasn't yet switched tabs. The BrowserWindow implementations
     // end up querying state once they process the tab switch.
     return;
   }
 
+  bool shouldAnimate = reason == BOOKMARK_BAR_STATE_CHANGE_PREF_CHANGE;
+
   // Don't animate if mode is |NTP| because the bookmark is attached at top when
   // pref is on and detached at bottom when off.
-  BookmarkBar::AnimateChangeType animate_type =
-      ((reason == BOOKMARK_BAR_STATE_CHANGE_PREF_CHANGE &&
-        !search_model_->mode().is_ntp()) ||
-       reason == BOOKMARK_BAR_STATE_CHANGE_INSTANT_PREVIEW_STATE) ?
+  // TODO(sail) Remove this once other platforms have removed the bottom
+  // bookmark bar.
+#if !defined(OS_MACOSX)
+  shouldAnimate = shouldAnimate && !search_model_->mode().is_ntp();

[dhollowa, 2013/01/18 00:55:11]

I guess this should be wrapped in an if (IsInstantExtended...) block as well
because DEFAULT mode could be because the feature is turned off.

[sail, 2013/01/18 01:37:09]

Done.
Good catch.

+#endif
+
+  window_->BookmarkBarStateChanged(shouldAnimate ?
       BookmarkBar::ANIMATE_STATE_CHANGE :
-      BookmarkBar::DONT_ANIMATE_STATE_CHANGE;
-  window_->BookmarkBarStateChanged(animate_type);
+      BookmarkBar::DONT_ANIMATE_STATE_CHANGE);
 }
 
 bool Browser::ShouldHideUIForFullscreen() const {
   // On Mac, fullscreen mode has most normal things (in a slide-down panel). On
   // other platforms, we hide some controls when in fullscreen mode.
 #if defined(OS_MACOSX)
   return false;
 #endif
   return window_ && window_->IsFullscreen();
 }
@@ skipping 72 matching lines @@
   if (contents && !allow_js_access) {
     contents->web_contents()->GetController().LoadURL(
         target_url,
         content::Referrer(),
         content::PAGE_TRANSITION_LINK,
         std::string());  // No extra headers.
   }
 
   return contents != NULL;
 }