[Mac] Use the ExtensionToolbarModel for ordering of the Browser Actions. Prep for drag and drop for re-ordering.

Also fixes crashers where if you tried to disable an extension via its context menu while an incognito window was key, boom.

BUG=26990
TEST=none
Review URL: http://codereview.chromium.org/595017

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@38632 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 93 insertions, 84 deletions

diff --git a/chrome/browser/cocoa/autocomplete_text_field_editor.h b/chrome/browser/cocoa/autocomplete_text_field_editor.h
index d7a5afe7..bb3afc5 100644
--- a/chrome/browser/cocoa/autocomplete_text_field_editor.h
+++ b/chrome/browser/cocoa/autocomplete_text_field_editor.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -8,6 +8,7 @@
 #import "chrome/browser/cocoa/url_drop_target.h"
 
 class AutocompleteTextFieldObserver;
+class Profile;
 
 // AutocompleteTextFieldEditor customized the AutocompletTextField
 // field editor (helper text-view used in editing).  It intercepts UI
@@ -23,7 +24,13 @@ class AutocompleteTextFieldObserver;
   // |-updateDragTypeRegistration|), since the latter results in a weird
   // start-up time regression.
   scoped_nsobject<URLDropTargetHandler> dropHandler_;
+
+  // The browser profile for the editor. Weak.
+  Profile* profile_;
 }
+
+@property(nonatomic) Profile* profile;
+
 @end
 
 @interface AutocompleteTextFieldEditor(PrivateTestMethods)
diff --git a/chrome/browser/cocoa/autocomplete_text_field_editor.mm b/chrome/browser/cocoa/autocomplete_text_field_editor.mm
index 2673b86..b4e0a5e 100644
--- a/chrome/browser/cocoa/autocomplete_text_field_editor.mm
+++ b/chrome/browser/cocoa/autocomplete_text_field_editor.mm
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -28,6 +28,8 @@ class Extension;
 
 @implementation AutocompleteTextFieldEditor
 
+@synthesize profile = profile_;
+
 - (id)initWithFrame:(NSRect)frameRect {
   if ((self = [super initWithFrame:frameRect]))
     dropHandler_.reset([[URLDropTargetHandler alloc] initWithView:self]);
@@ -37,9 +39,8 @@ class Extension;
 - (void)copy:(id)sender {
   AutocompleteTextFieldObserver* observer = [self observer];
   DCHECK(observer);
-  if (observer) {
+  if (observer)
     observer->OnCopy();
-  }
 }
 
 - (void)cut:(id)sender {
@@ -97,11 +98,13 @@ class Extension;
   AutocompleteTextFieldCell* cell = [field autocompleteTextFieldCell];
   const size_t pageActionCount = [cell pageActionCount];
   BOOL flipped = [self isFlipped];
-  Browser* browser = BrowserList::GetLastActive();
-  // GetLastActive() returns NULL during testing.
-  if (!browser)
+  if (!profile_)
     return [self defaultMenuForEvent:event];
-  ExtensionsService* service = browser->profile()->GetExtensionsService();
+
+  ExtensionsService* service = profile_->GetExtensionsService();
+  if (!service)
+    return [self defaultMenuForEvent:event];
+
   for (size_t i = 0; i < pageActionCount; ++i) {
     NSRect pageActionFrame = [cell pageActionFrameForIndex:i inFrame:bounds];
     if (NSMouseInRect(location, pageActionFrame, flipped)) {
@@ -110,8 +113,8 @@ class Extension;
       DCHECK(extension);
       if (!extension)
         break;
-      return [[[ExtensionActionContextMenu alloc] initWithExtension:extension]
-          autorelease];
+      return [[[ExtensionActionContextMenu alloc]
+          initWithExtension:extension profile:profile_] autorelease];
     }
   }
 
diff --git a/chrome/browser/cocoa/extensions/browser_action_button.h b/chrome/browser/cocoa/extensions/browser_action_button.h
index d4ddba6..8321828 100644
--- a/chrome/browser/cocoa/extensions/browser_action_button.h
+++ b/chrome/browser/cocoa/extensions/browser_action_button.h
@@ -14,6 +14,7 @@
 class Extension;
 class ExtensionAction;
 class ExtensionImageTrackerBridge;
+class Profile;
 
 extern const CGFloat kBrowserActionWidth;
 
@@ -33,8 +34,8 @@ extern const CGFloat kBrowserActionWidth;
 }
 
 - (id)initWithExtension:(Extension*)extension
-                  tabId:(int)tabId
-                xOffset:(int)xOffset;
+                profile:(Profile*)profile
+                  tabId:(int)tabId;
 
 - (void)setDefaultIcon:(NSImage*)image;
 
diff --git a/chrome/browser/cocoa/extensions/browser_action_button.mm b/chrome/browser/cocoa/extensions/browser_action_button.mm
index 2c6cb67..6283708 100644
--- a/chrome/browser/cocoa/extensions/browser_action_button.mm
+++ b/chrome/browser/cocoa/extensions/browser_action_button.mm
@@ -92,9 +92,9 @@ class ExtensionImageTrackerBridge : public NotificationObserver,
 }
 
 - (id)initWithExtension:(Extension*)extension
-                  tabId:(int)tabId
-                xOffset:(int)xOffset {
-  NSRect frame = NSMakeRect(xOffset,
+                profile:(Profile*)profile
+                  tabId:(int)tabId {
+  NSRect frame = NSMakeRect(0.0,
                             kBrowserActionOriginYOffset,
                             kBrowserActionWidth,
                             kBrowserActionHeight);
@@ -114,7 +114,7 @@ class ExtensionImageTrackerBridge : public NotificationObserver,
     [self setShowsBorderOnlyWhileMouseInside:YES];
 
     [self setMenu:[[[ExtensionActionContextMenu alloc]
-        initWithExtension:extension] autorelease]];
+        initWithExtension:extension profile:profile] autorelease]];
 
     tabId_ = tabId;
     extension_ = extension;
diff --git a/chrome/browser/cocoa/extensions/browser_actions_controller.h b/chrome/browser/cocoa/extensions/browser_actions_controller.h
index 3be6814..c15138b 100644
--- a/chrome/browser/cocoa/extensions/browser_actions_controller.h
+++ b/chrome/browser/cocoa/extensions/browser_actions_controller.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -15,6 +15,7 @@ class Browser;
 @class BrowserActionsContainerView;
 class Extension;
 @class ExtensionPopupController;
+class ExtensionToolbarModel;
 class ExtensionsServiceObserverBridge;
 class Profile;
 
@@ -33,6 +34,9 @@ extern NSString* const kBrowserActionsChangedNotification;
   // The current profile. Weak.
   Profile* profile_;
 
+  // The model that tracks the order of the toolbar icons. Weak.
+  ExtensionToolbarModel* toolbarModel_;
+
   // The observer for the ExtensionsService we're getting events from.
   scoped_ptr<ExtensionsServiceObserverBridge> observer_;
 
@@ -57,10 +61,6 @@ extern NSString* const kBrowserActionsChangedNotification;
 // Update the display of all buttons.
 - (void)update;
 
-// Marks the container view for redraw. Called by the extension service
-// notification bridge.
-- (void)browserActionVisibilityHasChanged;
-
 // Returns the current number of browser action buttons within the container,
 // whether or not they are displayed.
 - (int)buttonCount;
diff --git a/chrome/browser/cocoa/extensions/browser_actions_controller.mm b/chrome/browser/cocoa/extensions/browser_actions_controller.mm
index 79f3724..cc73d61 100644
--- a/chrome/browser/cocoa/extensions/browser_actions_controller.mm
+++ b/chrome/browser/cocoa/extensions/browser_actions_controller.mm
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -12,6 +12,7 @@
 #include "chrome/browser/cocoa/extensions/browser_actions_container_view.h"
 #include "chrome/browser/cocoa/extensions/extension_popup_controller.h"
 #include "chrome/browser/extensions/extension_browser_event_router.h"
+#include "chrome/browser/extensions/extension_toolbar_model.h"
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
@@ -24,7 +25,8 @@ extern const CGFloat kBrowserActionButtonPadding = 3;
 NSString* const kBrowserActionsChangedNotification = @"BrowserActionsChanged";
 
 @interface BrowserActionsController(Private)
-- (void)createActionButtonForExtension:(Extension*)extension;
+- (void)createActionButtonForExtension:(Extension*)extension
+                             withIndex:(int)index;
 - (void)removeActionButtonForExtension:(Extension*)extension;
 - (void)repositionActionButtons;
 - (int)currentTabId;
@@ -32,40 +34,20 @@ NSString* const kBrowserActionsChangedNotification = @"BrowserActionsChanged";
 
 // A helper class to proxy extension notifications to the view controller's
 // appropriate methods.
-class ExtensionsServiceObserverBridge : public NotificationObserver {
+class ExtensionsServiceObserverBridge : public NotificationObserver,
+                                        public ExtensionToolbarModel::Observer {
  public:
   ExtensionsServiceObserverBridge(BrowserActionsController* owner,
                                   Profile* profile) : owner_(owner) {
-    registrar_.Add(this, NotificationType::EXTENSION_LOADED,
-                   Source<Profile>(profile));
-    registrar_.Add(this, NotificationType::EXTENSION_UNLOADED,
-                   Source<Profile>(profile));
-    registrar_.Add(this, NotificationType::EXTENSION_UNLOADED_DISABLED,
-                   Source<Profile>(profile));
     registrar_.Add(this, NotificationType::EXTENSION_HOST_VIEW_SHOULD_CLOSE,
                    Source<Profile>(profile));
   }
 
-  // Runs |owner_|'s method corresponding to the event type received from the
-  // notification system.
   // Overridden from NotificationObserver.
   void Observe(NotificationType type,
                const NotificationSource& source,
                const NotificationDetails& details) {
     switch (type.value) {
-      case NotificationType::EXTENSION_LOADED: {
-        Extension* extension = Details<Extension>(details).ptr();
-        [owner_ createActionButtonForExtension:extension];
-        [owner_ browserActionVisibilityHasChanged];
-        break;
-      }
-      case NotificationType::EXTENSION_UNLOADED:
-      case NotificationType::EXTENSION_UNLOADED_DISABLED: {
-        Extension* extension = Details<Extension>(details).ptr();
-        [owner_ removeActionButtonForExtension:extension];
-        [owner_ browserActionVisibilityHasChanged];
-        break;
-      }
       case NotificationType::EXTENSION_HOST_VIEW_SHOULD_CLOSE: {
         ExtensionPopupController* popup = [ExtensionPopupController popup];
         if (popup && ![popup isClosing])
@@ -78,6 +60,15 @@ class ExtensionsServiceObserverBridge : public NotificationObserver {
     }
   }
 
+  // ExtensionToolbarModel::Observer implementation.
+  void BrowserActionAdded(Extension* extension, int index) {
+    [owner_ createActionButtonForExtension:extension withIndex:index];
+  }
+
+  void BrowserActionRemoved(Extension* extension) {
+    [owner_ removeActionButtonForExtension:extension];
+  }
+
  private:
   // The object we need to inform when we get a notification. Weak. Owns us.
   BrowserActionsController* owner_;
@@ -98,9 +89,17 @@ class ExtensionsServiceObserverBridge : public NotificationObserver {
     browser_ = browser;
     profile_ = browser->profile();
 
+    observer_.reset(new ExtensionsServiceObserverBridge(self, profile_));
+    ExtensionsService* extensionsService = profile_->GetExtensionsService();
+    // |extensionsService| can be NULL in Incognito.
+    if (extensionsService) {
+      toolbarModel_ = extensionsService->toolbar_model();
+      toolbarModel_->AddObserver(observer_.get());
+    }
+
     containerView_ = container;
     [containerView_ setHidden:YES];
-    observer_.reset(new ExtensionsServiceObserverBridge(self, profile_));
+
     buttons_.reset([[NSMutableDictionary alloc] init]);
     buttonOrder_.reset([[NSMutableArray alloc] init]);
   }
@@ -109,6 +108,9 @@ class ExtensionsServiceObserverBridge : public NotificationObserver {
 }
 
 - (void)dealloc {
+  if (toolbarModel_)
+    toolbarModel_->RemoveObserver(observer_.get());
+
   [[NSNotificationCenter defaultCenter] removeObserver:self];
   [super dealloc];
 }
@@ -120,49 +122,43 @@ class ExtensionsServiceObserverBridge : public NotificationObserver {
   }
 }
 
-- (void)browserActionVisibilityHasChanged {
-  [containerView_ setNeedsDisplay:YES];
-}
-
 - (void)createButtons {
-  ExtensionsService* extensionsService = profile_->GetExtensionsService();
-  if (!extensionsService)  // |extensionsService| can be NULL in Incognito.
+  // No extensions in incognito mode.
+  if (!toolbarModel_)
     return;
 
-  for (size_t i = 0; i < extensionsService->extensions()->size(); ++i) {
-    Extension* extension = extensionsService->GetExtensionById(
-        extensionsService->extensions()->at(i)->id(), false);
-    if (extension->browser_action()) {
-      [self createActionButtonForExtension:extension];
-    }
+  int i = 0;
+  for (ExtensionList::iterator iter = toolbarModel_->begin();
+       iter != toolbarModel_->end(); ++iter) {
+    [self createActionButtonForExtension:*iter withIndex:i++];
   }
 }
 
-- (void)createActionButtonForExtension:(Extension*)extension {
+- (void)createActionButtonForExtension:(Extension*)extension
+                             withIndex:(int)index {
   if (!extension->browser_action())
     return;
 
-  if ([buttons_ count] == 0) {
-    // Only call if we're adding our first button, otherwise it will be shown
-    // already.
+  // Show the container if it's the first button. Otherwise it will be shown
+  // already.
+  if ([buttons_ count] == 0)
     [containerView_ setHidden:NO];
-  }
 
-  int xOffset =
-      [buttons_ count] * (kBrowserActionWidth + kBrowserActionButtonPadding);
-  BrowserActionButton* newButton =
-      [[[BrowserActionButton alloc] initWithExtension:extension
-                                                tabId:[self currentTabId]
-                                              xOffset:xOffset] autorelease];
+  BrowserActionButton* newButton = [[[BrowserActionButton alloc]
+      initWithExtension:extension
+                profile:profile_
+                  tabId:[self currentTabId]] autorelease];
   [newButton setTarget:self];
   [newButton setAction:@selector(browserActionClicked:)];
   NSString* buttonKey = base::SysUTF8ToNSString(extension->id());
   [buttons_ setObject:newButton forKey:buttonKey];
-  [buttonOrder_ addObject:newButton];
+  [buttonOrder_ insertObject:newButton atIndex:index];
   [containerView_ addSubview:newButton];
+  [self repositionActionButtons];
 
   [[NSNotificationCenter defaultCenter]
       postNotificationName:kBrowserActionsChangedNotification object:self];
+  [containerView_ setNeedsDisplay:YES];
 }
 
 - (void)removeActionButtonForExtension:(Extension*)extension {
@@ -183,14 +179,11 @@ class ExtensionsServiceObserverBridge : public NotificationObserver {
     // No more buttons? Hide the container.
     [containerView_ setHidden:YES];
   } else {
-    // repositionActionButtons only needs to be called if removing a browser
-    // action button because adding one will always append to the end of the
-    // container, while removing one may require that those to the right of it
-    // be shifted to the left.
     [self repositionActionButtons];
   }
   [[NSNotificationCenter defaultCenter]
       postNotificationName:kBrowserActionsChangedNotification object:self];
+  [containerView_ setNeedsDisplay:YES];
 }
 
 - (void)repositionActionButtons {
diff --git a/chrome/browser/cocoa/extensions/extension_action_context_menu.h b/chrome/browser/cocoa/extensions/extension_action_context_menu.h
index ffae3ed..31c36cf 100644
--- a/chrome/browser/cocoa/extensions/extension_action_context_menu.h
+++ b/chrome/browser/cocoa/extensions/extension_action_context_menu.h
@@ -9,8 +9,9 @@
 
 #import <Cocoa/Cocoa.h>
 
-class Extension;
 class AsyncUninstaller;
+class Extension;
+class Profile;
 
 // A context menu used by the Browser and Page Action components that appears
 // if a user right-clicks the view of the given extension.
@@ -18,13 +19,16 @@ class AsyncUninstaller;
   // The extension that this menu belongs to. Weak.
   Extension* extension_;
 
+  // The browser profile of the window that contains this extension. Weak.
+  Profile* profile_;
+
   // Used to load the extension icon asynchronously on the I/O thread then show
   // the uninstall confirmation dialog.
   scoped_refptr<AsyncUninstaller> uninstaller_;
 }
 
-// Initializes and returns a context menu for the given extension.
-- (id)initWithExtension:(Extension*)extension;
+// Initializes and returns a context menu for the given extension and profile.
+- (id)initWithExtension:(Extension*)extension profile:(Profile*)profile;
 
 @end
 
diff --git a/chrome/browser/cocoa/extensions/extension_action_context_menu.mm b/chrome/browser/cocoa/extensions/extension_action_context_menu.mm
index ff7cd40..dae5fac 100644
--- a/chrome/browser/cocoa/extensions/extension_action_context_menu.mm
+++ b/chrome/browser/cocoa/extensions/extension_action_context_menu.mm
@@ -105,9 +105,10 @@ enum {
 };
 }  // namespace
 
-- (id)initWithExtension:(Extension*)extension {
+- (id)initWithExtension:(Extension*)extension profile:(Profile*)profile {
   if ((self = [super initWithTitle:@""])) {
     extension_ = extension;
+    profile_ = profile;
 
     NSArray* menuItems = [NSArray arrayWithObjects:
         base::SysUTF8ToNSString(extension->name()),
@@ -148,13 +149,10 @@ enum {
 }
 
 - (void)dispatch:(id)menuItem {
-  Browser* browser = BrowserList::GetLastActive();
-  // GetLastActive() returns NULL during testing.
+  Browser* browser = BrowserList::FindBrowserWithProfile(profile_);
   if (!browser)
     return;
 
-  Profile* profile = browser->profile();
-
   NSMenuItem* item = (NSMenuItem*)menuItem;
   switch ([item tag]) {
     case kExtensionContextName: {
@@ -170,8 +168,10 @@ enum {
       break;
     }
     case kExtensionContextDisable: {
-      ExtensionsService* extension_service = profile->GetExtensionsService();
-      extension_service->DisableExtension(extension_->id());
+      ExtensionsService* extensionService = profile_->GetExtensionsService();
+      if (!extensionService)
+        return; // Incognito mode.
+      extensionService->DisableExtension(extension_->id());
       break;
     }
     case kExtensionContextUninstall: {
diff --git a/chrome/browser/cocoa/toolbar_controller.mm b/chrome/browser/cocoa/toolbar_controller.mm
index 2ca18eb..eb7cd90 100644
--- a/chrome/browser/cocoa/toolbar_controller.mm
+++ b/chrome/browser/cocoa/toolbar_controller.mm
@@ -477,6 +477,7 @@ class PrefObserverBridge : public NotificationObserver {
     if (autocompleteTextFieldEditor_.get() == nil) {
       autocompleteTextFieldEditor_.reset(
           [[AutocompleteTextFieldEditor alloc] init]);
+      [autocompleteTextFieldEditor_ setProfile:profile_];
     }
 
     // This needs to be called every time, otherwise notifications