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 443 matching lines @@
   return entry.first->IsItemCheckedAt(entry.second);
 }
 
 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.
-  if (command_id == IDC_CUT || command_id == IDC_ZOOM_MINUS)
+  // The items representing the cut menu (cut/copy/paste), zoom menu
+  // (increment/decrement/reset) and extension toolbar view are always enabled.
+  // The child views of these items enabled state updates appropriately.
+  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;
 
+    scoped_ptr<ExtensionToolbarMenuView> extension_toolbar_menu_view;
+    if (model->GetCommandIdAt(i) == IDC_EXTENSIONS_OVERFLOW_MENU) {
+      extension_toolbar_menu_view.reset(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);
 
     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 53 matching lines @@
     }
     command_id_to_entry_[command_id].first = model;
     command_id_to_entry_[command_id].second = model_index;
   }
 
   MenuItemView* menu_item = NULL;
   if (height > 0) {
     // For menu items with a special menu height we use our special class to be
     // able to modify the item height.
     menu_item = new ButtonContainerMenuItemView(parent, command_id, height);
+    if (!parent->GetSubmenu())
+      parent->CreateSubmenu();

[Finnur, 2014/06/26 14:55:48]

This is what fixes your crash. The AddMenuItem functions implicitly add the
submenu if it hasn't been created before, but if the *first* menu in the list
has a non-zero height, then this doesn't happen (because we don't take the else
clause here in that case).

The reason it happened on Linux and not Windows is because Windows Chrome has a
hidden top menu item (Show Incompatibilities), which implicitly adds a submenu
for the parent, so it doesn't crash in this location. When I commented that menu
out, it started crashing on Windows as well.

But that's no longer a concern. If the submenu is missing for the parent, we add
it.

     parent->GetSubmenu()->AddChildViewAt(menu_item, menu_index);
   } else {
     // For all other cases we use the more generic way to add menu items.
     menu_item = views::MenuModelAdapter::AddMenuItemFromModelAt(
         model, model_index, parent, menu_index, command_id);
   }
 
   if (menu_item) {
     // Flush all buttons to the right side of the menu for the new menu type.
     menu_item->set_use_right_margin(!use_new_menu_);
@@ skipping 41 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;
 }