Implement a unit test for the toolbar controller. Plumb the profile into the loation bar instead of picking it up globally, which cannnot be unit tested.

Review URL: http://codereview.chromium.org/79041

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

7 files changed, 144 insertions, 19 deletions

diff --git a/chrome/browser/cocoa/browser_window_controller.mm b/chrome/browser/cocoa/browser_window_controller.mm
index d25681c..cad9089 100644
--- a/chrome/browser/cocoa/browser_window_controller.mm
+++ b/chrome/browser/cocoa/browser_window_controller.mm
@@ -82,7 +82,8 @@ willPositionSheet:(NSWindow *)sheet
     // registering for the appropriate command state changes from the back-end.
     toolbarController_.reset([[ToolbarController alloc]
                                initWithModel:browser->toolbar_model()
-                                    commands:browser->command_updater()]);
+                                    commands:browser->command_updater()
+                                     profile:browser->profile()]);
     [self positionToolbar];
 
     // After we've adjusted the toolbar, create a controller for the bookmark
diff --git a/chrome/browser/cocoa/location_bar_view_mac.h b/chrome/browser/cocoa/location_bar_view_mac.h
index 1a0ef8d..d8188ee 100644
--- a/chrome/browser/cocoa/location_bar_view_mac.h
+++ b/chrome/browser/cocoa/location_bar_view_mac.h
@@ -24,7 +24,8 @@ class LocationBarViewMac : public AutocompleteEditController,
  public:
   LocationBarViewMac(NSTextField* field,
                      CommandUpdater* command_updater,
-                     ToolbarModel* toolbar_model);
+                     ToolbarModel* toolbar_model,
+                     Profile* profile);
   virtual ~LocationBarViewMac();
 
   // TODO(shess): This is a placeholder for the Omnibox code.  The
@@ -59,11 +60,10 @@ class LocationBarViewMac : public AutocompleteEditController,
  private:
   scoped_ptr<AutocompleteEditViewMac> edit_view_;
 
-  NSTextField* field_;  // weak, owned by TabContentsController
-  // TODO(shess): Determine ownership of these.  We definitely
-  // shouldn't.
-  CommandUpdater* command_updater_;  // weak
-  ToolbarModel* toolbar_model_;  // weak
+  NSTextField* field_;  // weak, owned by ToolbarController nib
+  CommandUpdater* command_updater_;  // weak, owned by Browser
+  ToolbarModel* toolbar_model_;  // weak, owned by Browser
+  Profile* profile_;  // weak, outlives the Browser
 
   // When we get an OnAutocompleteAccept notification from the autocomplete
   // edit, we save the input string so we can give it back to the browser on
diff --git a/chrome/browser/cocoa/location_bar_view_mac.mm b/chrome/browser/cocoa/location_bar_view_mac.mm
index 3fe1071..d653201 100644
--- a/chrome/browser/cocoa/location_bar_view_mac.mm
+++ b/chrome/browser/cocoa/location_bar_view_mac.mm
@@ -17,10 +17,12 @@
 
 LocationBarViewMac::LocationBarViewMac(NSTextField* field,
                                        CommandUpdater* command_updater,
-                                       ToolbarModel* toolbar_model)
+                                       ToolbarModel* toolbar_model,
+                                       Profile* profile)
     : field_(field),
       command_updater_(command_updater),
       toolbar_model_(toolbar_model),
+      profile_(profile),
       disposition_(CURRENT_TAB),
       transition_(PageTransition::TYPED) {
 }
@@ -30,13 +32,11 @@ LocationBarViewMac::~LocationBarViewMac() {
 }
 
 void LocationBarViewMac::Init() {
-  // TODO(shess): deanm indicates that it's likely we will eventually
-  // get the profile somewhere between point of construction and
-  // Init(), so mirroring how the gtk code sets this up.
-  Profile* profile = [[NSApp delegate] defaultProfile];
+  // TODO(shess): Get rid of Init() so we don't have to cache all these members
+  // as we don't use them beyond this method.
   edit_view_.reset(new AutocompleteEditViewMac(this,
                                                toolbar_model_,
-                                               profile,
+                                               profile_,
                                                command_updater_));
   // TODO(shess): Include in constructor.
   edit_view_->SetField(field_);
diff --git a/chrome/browser/cocoa/toolbar_controller.h b/chrome/browser/cocoa/toolbar_controller.h
index 4388c15..8967aee 100644
--- a/chrome/browser/cocoa/toolbar_controller.h
+++ b/chrome/browser/cocoa/toolbar_controller.h
@@ -13,6 +13,7 @@
 class CommandUpdater;
 class LocationBar;
 class LocationBarViewMac;
+class Profile;
 class TabContents;
 class ToolbarModel;
 class ToolbarView;
@@ -24,9 +25,13 @@ class ToolbarView;
  @private
   ToolbarModel* toolbarModel_;  // weak, one per window
   CommandUpdater* commands_;  // weak, one per window
+  Profile* profile_;  // weak, one per window
   scoped_ptr<CommandObserverBridge> commandObserver_;
   scoped_ptr<LocationBarViewMac> locationBarView_;
 
+  // The ordering is important for unit tests. If new items are added or the
+  // ordering is changed, make sure to update |-toolbarViews| and the
+  // corresponding enum in the unit tests.
   IBOutlet NSButton* backButton_;
   IBOutlet NSButton* forwardButton_;
   IBOutlet NSButton* reloadButton_;
@@ -35,9 +40,11 @@ class ToolbarView;
   IBOutlet NSTextField* locationBar_;
 }
 
-// Initialize the toolbar and register for command updates.
+// Initialize the toolbar and register for command updates. The profile is
+// needed for initializing the location bar.
 - (id)initWithModel:(ToolbarModel*)model
-           commands:(CommandUpdater*)commands;
+           commands:(CommandUpdater*)commands
+            profile:(Profile*)profile;
 
 // Get the C++ bridge object representing the location bar for this tab.
 - (LocationBar*)locationBar;
@@ -58,4 +65,10 @@ class ToolbarView;
 
 @end
 
+// A set of private methods used by tests, in the absence of "friends" in ObjC.
+@interface ToolbarController(PrivateTestMethods)
+// Returns an array of views in the order of the outlets above.
+- (NSArray*)toolbarViews;
+@end
+
 #endif  // CHROME_BROWSER_COCOA_TOOLBAR_CONTROLLER_H_
diff --git a/chrome/browser/cocoa/toolbar_controller.mm b/chrome/browser/cocoa/toolbar_controller.mm
index a44cccd..9cd7739 100644
--- a/chrome/browser/cocoa/toolbar_controller.mm
+++ b/chrome/browser/cocoa/toolbar_controller.mm
@@ -4,6 +4,7 @@
 
 #import "chrome/browser/cocoa/toolbar_controller.h"
 
+#include "base/mac_util.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/app/chrome_dll_resource.h"
 #import "chrome/browser/cocoa/location_bar_view_mac.h"
@@ -20,11 +21,14 @@ static NSString* const kStarredImageName = @"starred";
 @implementation ToolbarController
 
 - (id)initWithModel:(ToolbarModel*)model
-           commands:(CommandUpdater*)commands {
-  DCHECK(model && commands);
-  if ((self = [super initWithNibName:@"Toolbar" bundle:nil])) {
+           commands:(CommandUpdater*)commands
+            profile:(Profile*)profile {
+  DCHECK(model && commands && profile);
+  if ((self = [super initWithNibName:@"Toolbar"
+                              bundle:mac_util::MainAppBundle()])) {
     toolbarModel_ = model;
     commands_ = commands;
+    profile_ = profile;
 
     // Register for notifications about state changes for the toolbar buttons
     commandObserver_.reset(new CommandObserverBridge(self, commands));
@@ -43,7 +47,7 @@ static NSString* const kStarredImageName = @"starred";
 - (void)awakeFromNib {
   [self initCommandStatus:commands_];
   locationBarView_.reset(new LocationBarViewMac(locationBar_, commands_,
-                                                toolbarModel_));
+                                                toolbarModel_, profile_));
   locationBarView_->Init();
   [locationBar_ setStringValue:@"http://dev.chromium.org"];
 }
@@ -132,4 +136,10 @@ static NSString* const kStarredImageName = @"starred";
   [goButton_ setImage:[NSImage imageNamed:imageName]];
 }
 
+// Returns an array of views in the order of the outlets above.
+- (NSArray*)toolbarViews {
+  return [NSArray arrayWithObjects:backButton_, forwardButton_, reloadButton_,
+            starButton_, goButton_, locationBar_, nil];
+}
+
 @end
diff --git a/chrome/browser/cocoa/toolbar_controller_unittest.mm b/chrome/browser/cocoa/toolbar_controller_unittest.mm
new file mode 100644
index 0000000..39559e0
--- /dev/null
+++ b/chrome/browser/cocoa/toolbar_controller_unittest.mm
@@ -0,0 +1,100 @@
+// Copyright (c) 2009 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 <Cocoa/Cocoa.h>
+
+#import "base/scoped_nsobject.h"
+#include "chrome/app/chrome_dll_resource.h"
+#include "chrome/browser/cocoa/browser_test_helper.h"
+#import "chrome/browser/cocoa/cocoa_test_helper.h"
+#import "chrome/browser/cocoa/toolbar_controller.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+class ToolbarControllerTest : public testing::Test {
+ public:
+
+  // Indexes that match the ordering returned by the private ToolbarController
+  // |-toolbarViews| method.
+  enum {
+    kBackIndex, kForwardIndex, kReloadIndex, kStarIndex, kGoIndex,
+    kLocationIndex,
+  };
+
+  ToolbarControllerTest() {
+    Browser* browser = helper_.GetBrowser();
+    CommandUpdater* updater = browser->command_updater();
+    // The default state for the commands is true, set a couple to false to
+    // ensure they get picked up correct on initialization
+    updater->UpdateCommandEnabled(IDC_BACK, false);
+    updater->UpdateCommandEnabled(IDC_FORWARD, false);
+    bar_.reset(
+        [[ToolbarController alloc] initWithModel:browser->toolbar_model()
+                                        commands:browser->command_updater()
+                                         profile:helper_.GetProfile()]);
+    EXPECT_TRUE([bar_ view]);
+    NSView* parent = [cocoa_helper_.window() contentView];
+    [parent addSubview:[bar_ view]];
+  }
+
+  // Make sure the enabled state of the view is the same as the corresponding
+  // command in the updater. The views are in the declaration order of outlets.
+  void CompareState(CommandUpdater* updater, NSArray* views) {
+    EXPECT_EQ(updater->IsCommandEnabled(IDC_BACK),
+              [[views objectAtIndex:kBackIndex] isEnabled] ? true : false);
+    EXPECT_EQ(updater->IsCommandEnabled(IDC_FORWARD),
+              [[views objectAtIndex:kForwardIndex] isEnabled] ? true : false);
+    EXPECT_EQ(updater->IsCommandEnabled(IDC_RELOAD),
+              [[views objectAtIndex:kReloadIndex] isEnabled] ? true : false);
+    // TODO(pinkerton): Add IDC_HOME when we get a view for it
+    EXPECT_EQ(updater->IsCommandEnabled(IDC_STAR),
+              [[views objectAtIndex:kStarIndex] isEnabled] ? true : false);
+  }
+
+  CocoaTestHelper cocoa_helper_;  // Inits Cocoa, creates window, etc...
+  BrowserTestHelper helper_;
+  scoped_nsobject<ToolbarController> bar_;
+};
+
+// Test the initial state that everything is sync'd up
+TEST_F(ToolbarControllerTest, InitialState) {
+  CommandUpdater* updater = helper_.GetBrowser()->command_updater();
+  CompareState(updater, [bar_ toolbarViews]);
+}
+
+// Make some changes to the enabled state of a few of the buttons and ensure
+// that we're still in sync.
+TEST_F(ToolbarControllerTest, UpdateEnabledState) {
+  CommandUpdater* updater = helper_.GetBrowser()->command_updater();
+  EXPECT_FALSE(updater->IsCommandEnabled(IDC_BACK));
+  EXPECT_FALSE(updater->IsCommandEnabled(IDC_FORWARD));
+  updater->UpdateCommandEnabled(IDC_BACK, true);
+  updater->UpdateCommandEnabled(IDC_FORWARD, true);
+  CompareState(updater, [bar_ toolbarViews]);
+}
+
+TEST_F(ToolbarControllerTest, StarredState) {
+  // TODO(pinkerton): I'm not sure how to test this, as the only difference
+  // in internal state is in the image used. I tried using the name of the
+  // image on the button but it doesn't seem to stick to the NSImage, even
+  // when explicitly set.
+}
+
+// Focus the location bar and make sure that it's the first responder.
+TEST_F(ToolbarControllerTest, FocusLocation) {
+  NSWindow* window = cocoa_helper_.window();
+  [window makeFirstResponder:[window contentView]];
+  EXPECT_EQ([window firstResponder], [window contentView]);
+  [bar_ focusLocationBar];
+  EXPECT_NE([window firstResponder], [window contentView]);
+  NSView* locationBar = [[bar_ toolbarViews] objectAtIndex:kLocationIndex];
+  EXPECT_EQ([window firstResponder], [(id)locationBar currentEditor]);
+}
+
+TEST_F(ToolbarControllerTest, LoadingState) {
+  // TODO(pinkerton): Same problem testing this as the starred state above.
+}
+
+}  // namespace
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index 1779bb3..e883bfb 100644
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -2133,6 +2133,7 @@
         'browser/cocoa/location_bar_view_mac_unittest.mm',
         'browser/cocoa/status_bubble_mac_unittest.mm',
         'browser/cocoa/tab_controller_unittest.mm',
+        'browser/cocoa/toolbar_controller_unittest.mm',
         'browser/command_updater_unittest.cc',
         'browser/debugger/devtools_manager_unittest.cc',
         'browser/dom_ui/dom_ui_unittest.cc',