[Extensions Toolbar] Protect against crazy bounds

chrome/browser/ui/toolbar/wrench_menu_model.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 "chrome/browser/ui/toolbar/wrench_menu_model.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/command_line.h"
@@ skipping 18 matching lines @@
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/global_error/global_error.h"
 #include "chrome/browser/ui/global_error/global_error_service.h"
 #include "chrome/browser/ui/global_error/global_error_service_factory.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/toolbar/bookmark_sub_menu_model.h"
 #include "chrome/browser/ui/toolbar/encoding_menu_controller.h"
 #include "chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.h"
-#include "chrome/browser/ui/toolbar/toolbar_actions_model.h"
+#include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
 #include "chrome/browser/upgrade_detector.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/profiling.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/dom_distiller/core/dom_distiller_switches.h"
 #include "components/signin/core/browser/signin_manager.h"
 #include "components/signin/core/common/profile_management_switches.h"
@@ skipping 909 matching lines @@
               error->MenuItemIcon());
       menu_items_added = true;
     }
   }
   return menu_items_added;
 }
 
 void WrenchMenuModel::CreateActionToolbarOverflowMenu() {
   // We only add the extensions overflow container if there are any icons that
   // aren't shown in the main container.
-  if (!ToolbarActionsModel::Get(browser_->profile())->all_icons_visible()) {
+  // |browser_->window()| can be null during startup.

[Peter Kasting, 2015/09/15 00:13:38]

Nit: No || around a function call

[Devlin, 2015/09/15 18:22:39]

Wasn't sure about this one, because it was a simple accessor, so I was basically
referencing the variable.  But that is a little inconsistent.  Done.

+  if (browser_->window() &&
+      browser_->window()->GetToolbarActionsBar()->NeedsOverflow()) {

[Peter Kasting, 2015/09/15 00:13:38]

We now have ToolbarActionsBar::NeedsOverflow() and
ToolbarActionsdModel::all_icons_visible().  It's confusing to have both and to
know which should be used where.  Can we eliminate one?  If not can we somehow
document on these how they differ and why?

[Devlin, 2015/09/15 18:22:39]

Documented in toolbar_actions_bar.h

 #if defined(OS_MACOSX)
     // There's a bug in AppKit menus, where if a menu item with a custom view
     // (like the extensions overflow menu) is the first menu item, it is not
     // highlightable or keyboard-selectable.
     // Adding any menu item before it (even one which is never visible) prevents
     // it, so add a bogus item here that will always be hidden.
     AddItem(kEmptyMenuItemCommand, base::string16());
 #endif
     AddItem(IDC_EXTENSIONS_OVERFLOW_MENU, base::string16());
     AddSeparator(ui::UPPER_SEPARATOR);
@@ skipping 44 matching lines @@
                        ->GetZoomPercent();
   }
   zoom_label_ = l10n_util::GetStringFUTF16(
       IDS_ZOOM_PERCENT, base::IntToString16(zoom_percent));
 }
 
 void WrenchMenuModel::OnZoomLevelChanged(
     const content::HostZoomMap::ZoomLevelChange& change) {
   UpdateZoomControls();
 }