[Extensions UI Mac] Fix crash in overflow toolbar actions bar

Fix a crash that would happen when a new action was added with the overflow
menu open.  The overflow container would try to update before adding the
new action.
Fix, and add a test.

BUG=561237

Review URL: https://codereview.chromium.org/1488703002

Cr-Commit-Position: refs/heads/master@{#362453}

3 files changed, 66 insertions, 2 deletions

diff --git a/chrome/browser/ui/cocoa/app_menu/app_menu_controller.mm b/chrome/browser/ui/cocoa/app_menu/app_menu_controller.mm
index 8574e16..4cc6cd6 100644
--- a/chrome/browser/ui/cocoa/app_menu/app_menu_controller.mm
+++ b/chrome/browser/ui/cocoa/app_menu/app_menu_controller.mm
@@ -5,7 +5,10 @@
 #import "chrome/browser/ui/cocoa/app_menu/app_menu_controller.h"
 
 #include "base/basictypes.h"
+#include "base/bind.h"
 #include "base/mac/bundle_locations.h"
+#include "base/memory/weak_ptr.h"
+#include "base/message_loop/message_loop.h"
 #include "base/scoped_observer.h"
 #include "base/strings/string16.h"
 #include "base/strings/sys_string_conversions.h"
@@ -119,7 +122,8 @@ class ToolbarActionsBarObserverHelper : public ToolbarActionsBarObserver {
   ToolbarActionsBarObserverHelper(AppMenuController* controller,
                                   ToolbarActionsBar* toolbar_actions_bar)
       : controller_(controller),
-        scoped_observer_(this) {
+        scoped_observer_(this),
+        weak_ptr_factory_(this) {
     scoped_observer_.Add(toolbar_actions_bar);
   }
   ~ToolbarActionsBarObserverHelper() override {}
@@ -130,11 +134,24 @@ class ToolbarActionsBarObserverHelper : public ToolbarActionsBarObserver {
     scoped_observer_.RemoveAll();
   }
   void OnToolbarActionsBarDidStartResize() override {
+    // No point in having multiple pending update calls.
+    weak_ptr_factory_.InvalidateWeakPtrs();
+    // Edge case: If the resize is caused by an action being added while the
+    // menu is open, we need to wait for both toolbars to be updated. This can
+    // happen if a user's data is synced with the menu open.
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&ToolbarActionsBarObserverHelper::UpdateSubmenu,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+
+  void UpdateSubmenu() {
     [controller_ updateBrowserActionsSubmenu];
   }
 
   AppMenuController* controller_;
   ScopedObserver<ToolbarActionsBar, ToolbarActionsBarObserver> scoped_observer_;
+  base::WeakPtrFactory<ToolbarActionsBarObserverHelper> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ToolbarActionsBarObserverHelper);
 };
diff --git a/chrome/browser/ui/cocoa/extensions/browser_action_button_interactive_uitest.mm b/chrome/browser/ui/cocoa/extensions/browser_action_button_interactive_uitest.mm
index bd97931..8a51d8a 100644
--- a/chrome/browser/ui/cocoa/extensions/browser_action_button_interactive_uitest.mm
+++ b/chrome/browser/ui/cocoa/extensions/browser_action_button_interactive_uitest.mm
@@ -408,3 +408,50 @@ IN_PROC_BROWSER_TEST_F(BrowserActionButtonUiTest, TestOverflowContainerLayout) {
   resizeAndActivateWrench(kNumExtensions / 2, "GlobalError Half");
   resizeAndActivateWrench(1, "GlobalError One");
 }
+
+void AddExtensionWithMenuOpen(ToolbarController* toolbarController,
+                              ExtensionService* extensionService,
+                              const base::Closure& closure) {
+  AppMenuController* appMenuController =
+      [toolbarController appMenuController];
+
+  scoped_refptr<const extensions::Extension> extension =
+      extensions::extension_action_test_util::CreateActionExtension(
+          "extension",
+          extensions::extension_action_test_util::BROWSER_ACTION);
+  extensionService->AddExtension(extension.get());
+
+  base::RunLoop().RunUntilIdle();
+
+  // Close the app menu.
+  [appMenuController cancel];
+  EXPECT_FALSE([appMenuController isMenuOpen]);
+  base::MessageLoop::current()->PostTask(FROM_HERE, closure);
+}
+
+// Test adding an extension while the app menu is open. Regression test for
+// crbug.com/561237.
+IN_PROC_BROWSER_TEST_F(BrowserActionButtonUiTest, AddExtensionWithMenuOpen) {
+  // Add an extension to ensure the overflow menu is present.
+  scoped_refptr<const extensions::Extension> extension =
+      extensions::extension_action_test_util::CreateActionExtension(
+          "original extension",
+          extensions::extension_action_test_util::BROWSER_ACTION);
+  extension_service()->AddExtension(extension.get());
+  ASSERT_EQ(1, static_cast<int>(model()->toolbar_items().size()));
+  model()->SetVisibleIconCount(0);
+
+  MoveMouseToCenter(wrenchButton());
+
+  base::RunLoop runLoop;
+  // Click on the app menu, and pass in a callback to continue the test in
+  // AddExtensionWithMenuOpen (due to the blocking nature of Cocoa menus,
+  // passing in runLoop.QuitClosure() is not sufficient here.)
+  ui_controls::SendMouseEventsNotifyWhenDone(
+      ui_controls::LEFT, ui_controls::DOWN | ui_controls::UP,
+      base::Bind(&AddExtensionWithMenuOpen,
+                 base::Unretained(toolbarController()),
+                 extension_service(),
+                 runLoop.QuitClosure()));
+  runLoop.Run();
+}
diff --git a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc b/chrome/browser/ui/toolbar/toolbar_actions_bar.cc
index a7c651d..3f6313c 100644
--- a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc
+++ b/chrome/browser/ui/toolbar/toolbar_actions_bar.cc
@@ -78,7 +78,7 @@ template<typename Type1, typename Type2, typename FunctionType>
 void SortContainer(std::vector<Type1>* to_sort,
                    const std::vector<Type2>& reference,
                    FunctionType equal) {
-  DCHECK_GE(to_sort->size(), reference.size()) <<
+  CHECK_GE(to_sort->size(), reference.size()) <<
       "|to_sort| must contain all elements in |reference|.";
   if (reference.empty())
     return;