Add a shortcut to open the Apps page from the bookmark bar.

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

Index: chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc b/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
index 9644589f961b4c9c8a66eff3514fc20af316b3cb..3263e07793627a5fbdc07fb250e064ac7daa5890 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
@@ -30,6 +30,7 @@
 #include "chrome/browser/ui/bookmarks/bookmark_utils.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/chrome_pages.h"
+#include "chrome/browser/ui/search/search.h"
 #include "chrome/browser/ui/search/search_model.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/view_ids.h"
@@ -147,13 +148,47 @@ static const int kSearchNewTabBookmarkBarHeight = 36;
 static const int kSearchNewTabHorizontalPadding = 0;
 static const int kSearchNewTabVerticalPadding = 0;
 
+// Tag for the 'Apps Shortcut' button.
+static const int kAppsShortcutButtonTag = 2;
+
 namespace {
 
+// BookmarkButtonBase -----------------------------------------------
+
+// Base class for text buttons used on the bookmark bar.
+
+class BookmarkButtonBase : public views::TextButton {
+ public:
+  BookmarkButtonBase(views::ButtonListener* listener,
+                     const string16& title)
+      : TextButton(listener, title) {
+    show_animation_.reset(new ui::SlideAnimation(this));
+    if (bookmark_utils::IsBookmarkBarViewAnimationsDisabled()) {
+      // For some reason during testing the events generated by anima
+      // throw off the test. So, don't animate while testing.
+      show_animation_->Reset(1);
+    } else {
+      show_animation_->Show();
+    }
+  }
+
+  virtual bool IsTriggerableEvent(const ui::Event& e) OVERRIDE {
+    return e.type() == ui::ET_GESTURE_TAP ||
+           e.type() == ui::ET_GESTURE_TAP_DOWN ||
+           event_utils::IsPossibleDispositionEvent(e);
+  }
+
+ private:
+  scoped_ptr<ui::SlideAnimation> show_animation_;
+
+  DISALLOW_COPY_AND_ASSIGN(BookmarkButtonBase);
+};
+
 // BookmarkButton -------------------------------------------------------------
 
 // Buttons used for the bookmarks on the bookmark bar.
 
-class BookmarkButton : public views::TextButton {
+class BookmarkButton : public BookmarkButtonBase {
  public:
   // The internal view class name.
   static const char kViewClassName[];
@@ -162,17 +197,9 @@ class BookmarkButton : public views::TextButton {
                  const GURL& url,
                  const string16& title,
                  Profile* profile)
-      : TextButton(listener, title),
+      : BookmarkButtonBase(listener, title),
         url_(url),
         profile_(profile) {
-    show_animation_.reset(new ui::SlideAnimation(this));
-    if (bookmark_utils::IsBookmarkBarViewAnimationsDisabled()) {
-      // For some reason during testing the events generated by animating
-      // throw off the test. So, don't animate while testing.
-      show_animation_->Reset(1);
-    } else {
-      show_animation_->Show();
-    }
   }
 
   virtual bool GetTooltipText(const gfx::Point& p,
@@ -184,12 +211,6 @@ class BookmarkButton : public views::TextButton {
     return !tooltip->empty();
   }
 
-  virtual bool IsTriggerableEvent(const ui::Event& e) OVERRIDE {
-    return e.type() == ui::ET_GESTURE_TAP ||
-           e.type() == ui::ET_GESTURE_TAP_DOWN ||
-           event_utils::IsPossibleDispositionEvent(e);
-  }
-
   virtual std::string GetClassName() const OVERRIDE {
     return kViewClassName;
   }
@@ -197,7 +218,6 @@ class BookmarkButton : public views::TextButton {
  private:
   const GURL& url_;
   Profile* profile_;
-  scoped_ptr<ui::SlideAnimation> show_animation_;
 
   DISALLOW_COPY_AND_ASSIGN(BookmarkButton);
 };
@@ -206,6 +226,34 @@ class BookmarkButton : public views::TextButton {
 const char BookmarkButton::kViewClassName[] =
     "browser/ui/views/bookmarks/BookmarkButton";
 
+// ShortcutButton -------------------------------------------------------------
+
+// Buttons used for the shortcuts on the bookmark bar.
+
+class ShortcutButton : public BookmarkButtonBase {
+ public:
+  // The internal view class name.
+  static const char kViewClassName[];
+
+  ShortcutButton(views::ButtonListener* listener,
+                 const string16& title)
+      : BookmarkButtonBase(listener, title) {
+  }
+
+  virtual std::string GetClassName() const OVERRIDE {
+    return kViewClassName;
+  }
+
+ private:
+  scoped_ptr<ui::SlideAnimation> show_animation_;

[sky, 2013/03/04 22:16:50]

This isn't needed, right?

[beaudoin, 2013/03/04 23:30:21]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(ShortcutButton);
+};
+
+// static for ShortcutButton
+const char ShortcutButton::kViewClassName[] =
+    "browser/ui/views/bookmarks/ShortcutButton";
+
 // BookmarkFolderButton -------------------------------------------------------
 
 // Buttons used for folders on the bookmark bar, including the 'other folders'
@@ -406,6 +454,7 @@ BookmarkBarView::BookmarkBarView(Browser* browser, BrowserView* browser_view)
       bookmark_menu_(NULL),
       bookmark_drop_menu_(NULL),
       other_bookmarked_button_(NULL),
+      apps_page_shortcut_(NULL),
       ALLOW_THIS_IN_INITIALIZER_LIST(show_folder_method_factory_(this)),
       overflow_button_(NULL),
       instructions_(NULL),
@@ -627,13 +676,21 @@ gfx::Size BookmarkBarView::GetMinimumSize() {
         browser_->search_model()->mode()) * current_state);
   }
 
-  gfx::Size other_bookmarked_pref =
-      other_bookmarked_button_->GetPreferredSize();
-  gfx::Size overflow_pref = overflow_button_->GetPreferredSize();
-  gfx::Size bookmarks_separator_pref =
-      bookmarks_separator_view_->GetPreferredSize();
-
+  gfx::Size other_bookmarked_pref;
+  if (other_bookmarked_button_->visible())
+    other_bookmarked_pref = other_bookmarked_button_->GetPreferredSize();
+  gfx::Size overflow_pref;
+  if (overflow_button_->visible())
+    overflow_pref = overflow_button_->GetPreferredSize();
+  gfx::Size bookmarks_separator_pref;
+  if (bookmarks_separator_view_->visible())
+      bookmarks_separator_pref = bookmarks_separator_view_->GetPreferredSize();

[sky, 2013/03/04 22:16:50]

spacing

[beaudoin, 2013/03/04 23:30:21]

Done.

+
+  gfx::Size apps_page_shortcut_pref;
+  if (apps_page_shortcut_->visible())
+    apps_page_shortcut_pref = apps_page_shortcut_->GetPreferredSize();
   width += other_bookmarked_pref.width() + kButtonPadding +
+      apps_page_shortcut_pref.width() + kButtonPadding +
       overflow_pref.width() + kButtonPadding +
       bookmarks_separator_pref.width();
 
@@ -1074,6 +1131,13 @@ void BookmarkBarView::OnMenuButtonClicked(views::View* view,
 
 void BookmarkBarView::ButtonPressed(views::Button* sender,
                                     const ui::Event& event) {
+  if (sender->tag() == kAppsShortcutButtonTag) {
+    chrome::ShowAppLauncherPage(browser_);
+    content::RecordAction(
+        UserMetricsAction("ClickedBookmarkBarAppsShortcutButton"));

[Alexei Svitkine (slow), 2013/03/04 20:56:04]

Can you move this UMA action to bookmark_utils.cc as a function, similar to the
other ones? This will need to be called from the Mac code too.

[tfarina, 2013/03/04 22:19:47]

if you do, can you try to put in c/b/ui/bookmark_utils rather than
c/b/bookmarks?

Thanks,

[tfarina, 2013/03/04 22:20:41]

oops, this should c/b/ui/bookmarks/bookmark_utils

[beaudoin, 2013/03/04 23:30:21]

Extracted to a method, but I did not move the bookmark_util::Record* functions
to c/b/ui/bookmarks/bookmark_utils because at least one is used in non-ui code,
specifically in
 c/b/extensions/api/bookmark_manager_private/bookmark_manager_private_api.cc

+    return;
+  }
+
   const BookmarkNode* node;
   if (sender->tag() == kOtherFolderButtonTag) {
     node = model_->other_node();
@@ -1113,11 +1177,12 @@ void BookmarkBarView::ShowContextMenuForView(views::View* source,
   if (source == other_bookmarked_button_) {
     parent = model_->other_node();
     // Do this so the user can open all bookmarks. BookmarkContextMenu makes
-    // sure the user can edit/delete the node in this case.
+    // sure the user can't edit/delete the node in this case.
     nodes.push_back(parent);
-  } else if (source != this) {
+  } else if (source != this && source != apps_page_shortcut_) {
     // User clicked on one of the bookmark buttons, find which one they
-    // clicked on.
+    // clicked on, except for the apps page shortcut, which must behave as if
+    // the user clicked on the bookmark bar background.
     int bookmark_button_index = GetIndexOf(source);
     DCHECK(bookmark_button_index != -1 &&
            bookmark_button_index < GetBookmarkButtonCount());
@@ -1161,6 +1226,15 @@ void BookmarkBarView::Init() {
   other_bookmarked_button_->SetEnabled(false);
   AddChildView(other_bookmarked_button_);
 
+  apps_page_shortcut_ = CreateAppsPageShortcutButton();
+  AddChildView(apps_page_shortcut_);
+  profile_pref_registrar_.Init(browser_->profile()->GetPrefs());
+  profile_pref_registrar_.Add(
+      prefs::kShowAppsShortcutInBookmarkBar,
+      base::Bind(&BookmarkBarView::OnAppsPageShortcutVisibilityChanged,
+      base::Unretained(this)));
+  apps_page_shortcut_->SetVisible(ShouldShowAppsShortcut());
+
   bookmarks_separator_view_ = new ButtonSeparatorView();
   AddChildView(bookmarks_separator_view_);
 #if defined(USE_ASH)
@@ -1188,8 +1262,8 @@ void BookmarkBarView::Init() {
 
 int BookmarkBarView::GetBookmarkButtonCount() {
   // We contain four non-bookmark button views: other bookmarks, bookmarks
-  // separator, chevrons (for overflow), and the instruction label.
-  return child_count() - 4;
+  // separator, chevrons (for overflow), apps page, and the instruction label.
+  return child_count() - 5;
 }
 
 views::TextButton* BookmarkBarView::GetBookmarkButton(int index) {
@@ -1259,6 +1333,20 @@ views::View* BookmarkBarView::CreateBookmarkButton(const BookmarkNode* node) {
   }
 }
 
+views::TextButton* BookmarkBarView::CreateAppsPageShortcutButton() {
+  views::TextButton* button = new ShortcutButton(
+      this, l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_APPS_SHORTCUT_NAME));
+  button->SetTooltipText(l10n_util::GetStringUTF16(
+      IDS_BOOKMARK_BAR_APPS_SHORTCUT_TOOLTIP));
+  button->set_id(VIEW_ID_BOOKMARK_BAR_ELEMENT);
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  button->SetIcon(*rb.GetImageSkiaNamed(IDR_WEBSTORE_ICON_16));
+  button->set_context_menu_controller(this);
+  button->set_tag(kAppsShortcutButtonTag);
+  button->SetEnabled(true);
+  return button;
+}
+
 void BookmarkBarView::ConfigureButton(const BookmarkNode* node,
                                       views::TextButton* button) {
   button->SetText(node->GetTitle());
@@ -1615,11 +1703,15 @@ gfx::Size BookmarkBarView::LayoutItems(bool compute_bounds_only) {
   gfx::Size overflow_pref = overflow_button_->GetPreferredSize();
   gfx::Size bookmarks_separator_pref =
       bookmarks_separator_view_->GetPreferredSize();
+  gfx::Size apps_page_shortcut_pref = apps_page_shortcut_->visible() ?
+      apps_page_shortcut_->GetPreferredSize() : gfx::Size();
 
   int max_x = width - overflow_pref.width() - kButtonPadding -
       bookmarks_separator_pref.width();
   if (other_bookmarked_button_->visible())
     max_x -= other_bookmarked_pref.width() + kButtonPadding;
+  if (apps_page_shortcut_->visible())
+    max_x -= apps_page_shortcut_pref.width() + kButtonPadding;
 
   // Next, layout out the buttons. Any buttons that are placed beyond the
   // visible region and made invisible.
@@ -1688,6 +1780,15 @@ gfx::Size BookmarkBarView::LayoutItems(bool compute_bounds_only) {
     x += other_bookmarked_pref.width() + kButtonPadding;
   }
 
+  // The app page shortcut button.
+  if (apps_page_shortcut_->visible()) {
+    if (!compute_bounds_only) {
+      apps_page_shortcut_->SetBounds(x, y, apps_page_shortcut_pref.width(),
+                                     height);
+    }
+    x += apps_page_shortcut_pref.width() + kButtonPadding;
+  }
+
   // Set the preferred size computed so far.
   if (compute_bounds_only) {
     x += kRightMargin;
@@ -1712,3 +1813,20 @@ gfx::Size BookmarkBarView::LayoutItems(bool compute_bounds_only) {
   }
   return prefsize;
 }
+
+bool BookmarkBarView::ShouldShowAppsShortcut() const {
+  return chrome::search::IsInstantExtendedAPIEnabled(browser_->profile()) &&
+      browser_->profile()->GetPrefs()->GetBoolean(
+          prefs::kShowAppsShortcutInBookmarkBar);
+}
+
+void BookmarkBarView::OnAppsPageShortcutVisibilityChanged() {
+  DCHECK(apps_page_shortcut_);
+  apps_page_shortcut_->SetVisible(ShouldShowAppsShortcut());
+  Layout();
+}
+
+bool BookmarkBarView::IsAppsShortcutVisibleForTesting() const {
+  DCHECK(apps_page_shortcut_);
+  return apps_page_shortcut_->visible();
+}