Extension Toolbar redesign, part 1 (overflow)

chrome/browser/ui/views/toolbar/wrench_menu.cc

 // Copyright 2013 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/views/toolbar/wrench_menu.h"
 
 #include <algorithm>
 #include <cmath>
 #include <set>
 
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/bookmarks/bookmark_stats.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/toolbar/wrench_menu_model.h"
 #include "chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.h"
+#include "chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.h"
 #include "chrome/browser/ui/views/toolbar/wrench_menu_observer.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "content/public/browser/host_zoom_map.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
+#include "extensions/common/feature_switch.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkPaint.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/layout.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/font_list.h"
@@ skipping 630 matching lines @@
 
     fullscreen_button_->SetFocusable(true);
     fullscreen_button_->set_request_focus_on_press(false);
     fullscreen_button_->set_tag(fullscreen_index);
     fullscreen_button_->SetImageAlignment(
         ImageButton::ALIGN_CENTER, ImageButton::ALIGN_MIDDLE);
     int horizontal_padding =
         menu->use_new_menu() ? kHorizontalTouchPadding : kHorizontalPadding;
     fullscreen_button_->SetBorder(views::Border::CreateEmptyBorder(
         0, horizontal_padding, 0, horizontal_padding));
-    fullscreen_button_->set_background(
-        new InMenuButtonBackground(InMenuButtonBackground::SINGLE_BUTTON,
-                                   menu->use_new_menu()));
+    fullscreen_button_->set_background(new InMenuButtonBackground(
+        InMenuButtonBackground::SINGLE_BUTTON, menu->use_new_menu()));
     fullscreen_button_->SetAccessibleName(
         GetAccessibleNameForWrenchMenuItem(
             menu_model, fullscreen_index, IDS_ACCNAME_FULLSCREEN));
     AddChildView(fullscreen_button_);
 
     // Need to set a font list for the zoom label width calculations.
     OnNativeThemeChanged(NULL);
     UpdateZoomControls();
   }
 
@@ skipping 445 matching lines @@
 
 bool WrenchMenu::IsCommandEnabled(int command_id) const {
   if (IsBookmarkCommand(command_id))
     return true;
 
   if (command_id == 0)
     return false;  // The root item.
 
   // The items representing the cut menu (cut/copy/paste) and zoom menu
   // (increment/decrement/reset) are always enabled. The child views of these
   // items enabled state updates appropriately.

[Devlin, 2014/06/23 17:55:29]

does this comment need updating?

-  if (command_id == IDC_CUT || command_id == IDC_ZOOM_MINUS)
+  if (command_id == IDC_CUT || command_id == IDC_ZOOM_MINUS ||
+      command_id == IDC_EXTENSIONS_OVERFLOW_MENU)
     return true;
 
   const Entry& entry = command_id_to_entry_.find(command_id)->second;
   return entry.first->IsEnabledAt(entry.second);
 }
 
 void WrenchMenu::ExecuteCommand(int command_id, int mouse_event_flags) {
   if (IsBookmarkCommand(command_id)) {
     bookmark_menu_delegate_->ExecuteCommand(command_id, mouse_event_flags);
     return;
   }
 
-  if (command_id == IDC_CUT || command_id == IDC_ZOOM_MINUS) {
+  if (command_id == IDC_CUT || command_id == IDC_ZOOM_MINUS ||
+      command_id == IDC_EXTENSIONS_OVERFLOW_MENU) {
     // These items are represented by child views. If ExecuteCommand is invoked
     // it means the user clicked on the area around the buttons and we should
     // not do anyting.
     return;
   }
 
   const Entry& entry = command_id_to_entry_.find(command_id)->second;
   return entry.first->ActivatedAt(entry.second, mouse_event_flags);
 }
 
 bool WrenchMenu::GetAccelerator(int command_id,
                                 ui::Accelerator* accelerator) const {
   if (IsBookmarkCommand(command_id))
     return false;
 
-  if (command_id == IDC_CUT || command_id == IDC_ZOOM_MINUS) {
+  if (command_id == IDC_CUT || command_id == IDC_ZOOM_MINUS ||
+      command_id == IDC_EXTENSIONS_OVERFLOW_MENU) {
     // These have special child views; don't show the accelerator for them.
     return false;
   }
 
   CommandIDToEntry::const_iterator ix = command_id_to_entry_.find(command_id);
   const Entry& entry = ix->second;
   ui::Accelerator menu_accelerator;
   if (!entry.first->GetAcceleratorAt(entry.second, &menu_accelerator))
     return false;
 
@@ skipping 46 matching lines @@
                               MenuModel* model) {
   for (int i = 0, max = model->GetItemCount(); i < max; ++i) {
     // The button container menu items have a special height which we have to
     // use instead of the normal height.
     int height = 0;
     if (use_new_menu_ &&
         (model->GetCommandIdAt(i) == IDC_CUT ||
          model->GetCommandIdAt(i) == IDC_ZOOM_MINUS))
       height = kMenuItemContainingButtonsHeight;
 
+    if (model->GetCommandIdAt(i) == IDC_EXTENSIONS_OVERFLOW_MENU) {
+      extension_toolbar_menu_view_.reset(

[Devlin, 2014/06/23 17:55:29]

If this variable is only ever created and then released in PopulateMenu, does it
need to be a persistent member variable?

[Finnur, 2014/06/25 16:18:11]

Nope.

+          new ExtensionToolbarMenuView(browser_));
+      height = extension_toolbar_menu_view_->GetPreferredSize().height();
+    }
+
     // Add the menu item at the end.
     int menu_index = parent->HasSubmenu() ?
         parent->GetSubmenu()->child_count() : 0;
     MenuItemView* item = AddMenuItem(
         parent, menu_index, model, i, model->GetTypeAt(i), height);

[Finnur, 2014/06/24 17:00:50]

It crashes because parent->GetSubmenu() returns NULL on Linux and AddMenuItem
uses that pointer without NULL checking if height > 0 (on line 1347 below). Not
sure why it is different on Windows.

Maybe Scott knows.

 
     if (model->GetTypeAt(i) == MenuModel::TYPE_SUBMENU)
       PopulateMenu(item, model->GetSubmenuModelAt(i));
 
     switch (model->GetCommandIdAt(i)) {
+      case IDC_EXTENSIONS_OVERFLOW_MENU:
+        if (height > 0)
+          item->AddChildView(extension_toolbar_menu_view_.release());
+        else
+          item->SetVisible(false);
+        break;
       case IDC_CUT:
         DCHECK_EQ(MenuModel::TYPE_COMMAND, model->GetTypeAt(i));
         DCHECK_LT(i + 2, max);
         DCHECK_EQ(IDC_COPY, model->GetCommandIdAt(i + 1));
         DCHECK_EQ(IDC_PASTE, model->GetCommandIdAt(i + 2));
         item->SetTitle(l10n_util::GetStringUTF16(IDS_EDIT2));
         item->AddChildView(new CutCopyPasteView(this, model,
                                                 i, i + 1, i + 2));
         i += 2;
         break;
@@ skipping 114 matching lines @@
                                 0,
                                 BookmarkMenuDelegate::SHOW_PERMANENT_FOLDERS,
                                 BOOKMARK_LAUNCH_LOCATION_WRENCH_MENU);
 }
 
 int WrenchMenu::ModelIndexFromCommandId(int command_id) const {
   CommandIDToEntry::const_iterator ix = command_id_to_entry_.find(command_id);
   DCHECK(ix != command_id_to_entry_.end());
   return ix->second.second;
 }