[Mac] Use the ExtensionToolbarModel for ordering of the Browser Actions. Prep...

chrome/browser/cocoa/extensions/browser_actions_controller.mm

-// 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.
 
 #import "browser_actions_controller.h"
 
 #include <string>
 
 #include "base/sys_string_conversions.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/cocoa/extensions/browser_action_button.h"
 #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"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
 
 // The padding between browser action buttons.
 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;
 @end
 
 // 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])
           [popup close];
 
         break;
       }
       default:
         NOTREACHED() << L"Unexpected notification";
     }
   }
 
+  // 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_;
 
   // Used for registering to receive notifications and automatic clean up.
   NotificationRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionsServiceObserverBridge);
 };
 
 @implementation BrowserActionsController
 
 - (id)initWithBrowser:(Browser*)browser
         containerView:(BrowserActionsContainerView*)container {
   DCHECK(browser && container);
 
   if ((self = [super init])) {
     browser_ = browser;
     profile_ = browser->profile();
+    observer_.reset(new ExtensionsServiceObserverBridge(self, profile_));
+    ExtensionsService* extensionsService = profile_->GetExtensionsService();
+    if (!extensionsService) {  // |extensionsService| can be NULL in Incognito.

[Erik does not do reviews, 2010/02/10 16:57:41]

FYI, this is going to change soon (see http://codereview.chromium.org/567037). 
This will affect this code (order and position is handled a little differently),
but no need to change anything here for now though.

+      toolbarModel_ = NULL;
+    } else {
+      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]);
   }
 
   return self;
 }
 
 - (void)dealloc {
+  if (toolbarModel_)
+    toolbarModel_->RemoveObserver(observer_.get());
+
   [[NSNotificationCenter defaultCenter] removeObserver:self];
   [super dealloc];
 }
 
 - (void)update {
   for (BrowserActionButton* button in [buttons_ allValues]) {
     [button setTabId:[self currentTabId]];
     [button updateState];
   }
 }
 
-- (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];

[Erik does not do reviews, 2010/02/10 16:57:41]

I thought adding and removing subviews always set this automatically.  It
obviously doesn't hurt, but just curious.

[Bons, 2010/02/10 17:52:30]

I don't see anything in the docs, but also, we want to redisplay _after_ we
reposition the action buttons.

 }
 
 - (void)removeActionButtonForExtension:(Extension*)extension {
   if (!extension->browser_action())
     return;
 
   NSString* buttonKey = base::SysUTF8ToNSString(extension->id());
 
   BrowserActionButton* button = [buttons_ objectForKey:buttonKey];
   if (!button) {
     NOTREACHED();
     return;
   }
   [button removeFromSuperview];
   [buttons_ removeObjectForKey:buttonKey];
   [buttonOrder_ removeObject:button];
   if ([buttons_ count] == 0) {
     // 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 {
   for (NSUInteger i = 0; i < [buttonOrder_ count]; ++i) {
     CGFloat xOffset = i * (kBrowserActionWidth + kBrowserActionButtonPadding);
     BrowserActionButton* button = [buttonOrder_ objectAtIndex:i];
     NSRect buttonFrame = [button frame];
     buttonFrame.origin.x = xOffset;
     [button setFrame:buttonFrame];
   }
@@ skipping 63 matching lines @@
   }
   NOTREACHED();
   return nil;
 }
 
 - (NSButton*)buttonWithIndex:(int)index {
   return [buttonOrder_ objectAtIndex:(NSUInteger)index];
 }
 
 @end