diff options
Diffstat (limited to 'chrome/browser')
17 files changed, 257 insertions, 176 deletions
diff --git a/chrome/browser/app_controller_mac.mm b/chrome/browser/app_controller_mac.mm index 6b2bf48..435bc60 100644 --- a/chrome/browser/app_controller_mac.mm +++ b/chrome/browser/app_controller_mac.mm @@ -150,14 +150,11 @@ } - (Profile*)defaultProfile { - // Only expected to return NULL in a unit test (if g_browser_process - // is a TestingBrowserProcess). - // TODO(jrg): DCHECK() to confirm that. // TODO(jrg): Find a better way to get the "default" profile. if (g_browser_process->profile_manager()) return* g_browser_process->profile_manager()->begin(); - return NULL; + return NULL; } // Various methods to open URLs that we get in a native fashion. We use diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h index fde6482..eea3f0c 100644 --- a/chrome/browser/cocoa/bookmark_bar_controller.h +++ b/chrome/browser/cocoa/bookmark_bar_controller.h @@ -22,9 +22,11 @@ class Profile; BookmarkModel* bookmarkModel_; // weak; part of the profile owned by the // top-level Browser object. - // Controller for bookmark bar state, shared among all TabContents. - scoped_nsobject<BookmarkBarStateController> bookmarkBarStateController_; + // Currently these two are always the same, but they mean slightly + // different things. contentAreaHasOffset_ is an implementation + // detail of bookmark bar visibility. BOOL contentAreaHasOffset_; + BOOL barIsVisible_; // TODO(jrg): write a BookmarkView IBOutlet ToolbarView* /* BookmarkView* */ bookmarkView_; diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm index 95195c8..48eeba8 100644 --- a/chrome/browser/cocoa/bookmark_bar_controller.mm +++ b/chrome/browser/cocoa/bookmark_bar_controller.mm @@ -4,9 +4,10 @@ #import "chrome/browser/cocoa/bookmark_bar_controller.h" -#import "chrome/browser/cocoa/bookmark_bar_state_controller.h" #import "chrome/browser/cocoa/toolbar_view.h" #include "chrome/browser/profile.h" +#include "chrome/common/pref_names.h" +#include "chrome/common/pref_service.h" @interface BookmarkBarController(Private) - (void)applyContentAreaOffset:(BOOL)apply; @@ -21,14 +22,12 @@ if ((self = [super init])) { bookmarkModel_ = profile->GetBookmarkModel(); contentArea_ = content; - bookmarkBarStateController_.reset([[BookmarkBarStateController alloc] - initWithProfile:profile]); // Create and initialize the view's position. Show it if appropriate. // TODO(jrg): put it in a nib if necessary. NSRect frame = NSMakeRect(0, 0, 0, 30); bookmarkView_ = [[ToolbarView alloc] initWithFrame:frame]; [self positionBar]; - if ([self isBookmarkBarVisible]) + if (profile->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar)) [self showBookmarkBar:YES]; [[[contentArea_ window] contentView] addSubview:bookmarkView_]; } @@ -41,7 +40,7 @@ - (void)positionBar { // Hide or show bar based on initial visibility and set the resize flags. [bookmarkView_ setAutoresizingMask:NSViewWidthSizable | NSViewMinYMargin]; - [bookmarkView_ setHidden:[self isBookmarkBarVisible] ? NO : YES]; + [bookmarkView_ setHidden:!barIsVisible_]; // Set the bar's height to zero and position it at the top of the // content area, within the window's content view (as opposed to the @@ -76,6 +75,7 @@ // TODO(jrg): don't draw a border around it // TODO(jrg): ... } + barIsVisible_ = enable; } // Apply a contents box offset to make (or remove) room for the @@ -105,13 +105,14 @@ } - (BOOL)isBookmarkBarVisible { - return [bookmarkBarStateController_ visible]; + return barIsVisible_; } +// We don't change a preference; we only change visibility. +// Preference changing (global state) is handled in +// BrowserWindowCocoa::ToggleBookmarkBar(). - (void)toggleBookmarkBar { - [bookmarkBarStateController_ toggleBookmarkBar]; - BOOL visible = [self isBookmarkBarVisible]; - [self showBookmarkBar:visible ? YES : NO]; + [self showBookmarkBar:!barIsVisible_]; } - (NSView*)view { diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm index e2ba9aa..421867b 100644 --- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm +++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm @@ -20,7 +20,7 @@ class BookmarkBarControllerTest : public testing::Test { NSRect content_frame = NSMakeRect(0, 0, 800, kContentAreaHeight); content_area_.reset([[NSView alloc] initWithFrame:content_frame]); bar_.reset( - [[BookmarkBarController alloc] initWithProfile:helper_.GetProfile() + [[BookmarkBarController alloc] initWithProfile:helper_.profile() contentArea:content_area_.get()]); NSView* parent = cocoa_helper_.contentView(); [parent addSubview:content_area_.get()]; diff --git a/chrome/browser/cocoa/bookmark_bar_state_controller.h b/chrome/browser/cocoa/bookmark_bar_state_controller.h deleted file mode 100644 index 761d907..0000000 --- a/chrome/browser/cocoa/bookmark_bar_state_controller.h +++ /dev/null @@ -1,36 +0,0 @@ -// 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. - -#ifndef CHROME_BROWSER_COCOA_BOOKMARK_BAR_STATE_CONTROLLER_H_ -#define CHROME_BROWSER_COCOA_BOOKMARK_BAR_STATE_CONTROLLER_H_ - -#import <Cocoa/Cocoa.h> - -class Profile; - -// A class to manage bookmark bar state (visible or not). State is -// shared among all tabs and saved in a preference. -@interface BookmarkBarStateController : NSObject { - @private - Profile* profile_; - BOOL visible_; -} - -- (id)initWithProfile:(Profile *)browser; - -// Return YES or NO reflecting visibility state of the bookmark bar. -- (BOOL)visible; - -// Toggle (on or off) the bookmark bar visibility. -- (void)toggleBookmarkBar; - -@end // BookmarkBarStateController - - -@interface BookmarkBarStateController (Private) -// Internal method exposed for unit test convenience. -- (void)togglePreference; -@end - -#endif // CHROME_BROWSER_COCOA_BOOKMARK_BAR_STATE_CONTROLLER_H_ diff --git a/chrome/browser/cocoa/bookmark_bar_state_controller.mm b/chrome/browser/cocoa/bookmark_bar_state_controller.mm deleted file mode 100644 index e4f9d11a..0000000 --- a/chrome/browser/cocoa/bookmark_bar_state_controller.mm +++ /dev/null @@ -1,41 +0,0 @@ -// 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 "chrome/browser/cocoa/bookmark_bar_state_controller.h" - -#include "chrome/browser/bookmarks/bookmark_utils.h" -#include "chrome/browser/browser.h" -#include "chrome/browser/profile.h" -#include "chrome/common/pref_names.h" -#include "chrome/common/pref_service.h" - -@implementation BookmarkBarStateController - -- (id)initWithProfile:(Profile*)profile { - if ((self = [super init])) { - profile_ = profile; - // Initial visibility state comes from our preference. - if (profile_->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar)) { - visible_ = YES; - } - } - return self; -} - -- (BOOL)visible { - return visible_; -} - -// Whack and save a preference change. On Windows this call -// is made from BookmarkBarView. -- (void)togglePreference { - bookmark_utils::ToggleWhenVisible(profile_); -} - -- (void)toggleBookmarkBar { - visible_ = visible_ ? NO : YES; - [self togglePreference]; -} - -@end // BookmarkBarStateController diff --git a/chrome/browser/cocoa/bookmark_bar_state_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_state_controller_unittest.mm deleted file mode 100644 index 31ce077..0000000 --- a/chrome/browser/cocoa/bookmark_bar_state_controller_unittest.mm +++ /dev/null @@ -1,45 +0,0 @@ -// 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. - -#include "chrome/browser/browser.h" -#include "chrome/browser/cocoa/bookmark_bar_state_controller.h" -#include "chrome/browser/cocoa/browser_test_helper.h" -#include "testing/gtest/include/gtest/gtest.h" - -@interface NoPrefSaveBBStateController : BookmarkBarStateController { - @public - BOOL toggled_; -} -@end - -@implementation NoPrefSaveBBStateController -- (void)togglePreference { - toggled_ = !toggled_; -} -@end - - -class BookmarkBarStateControllerTest : public testing::Test { - public: - BrowserTestHelper browser_test_helper_; -}; - - -TEST_F(BookmarkBarStateControllerTest, MainTest) { - Browser* browser = browser_test_helper_.GetBrowser(); - NoPrefSaveBBStateController *c = [[[NoPrefSaveBBStateController alloc] - initWithProfile:browser->profile()] - autorelease]; - EXPECT_TRUE(c); - EXPECT_FALSE(c->toggled_); - BOOL old_visible = [c visible]; - - [c toggleBookmarkBar]; - EXPECT_TRUE(c->toggled_ != NO); - EXPECT_NE((bool)old_visible, (bool)[c visible]); - - [c toggleBookmarkBar]; - EXPECT_FALSE(c->toggled_); - EXPECT_EQ((bool)old_visible, (bool)[c visible]); -} diff --git a/chrome/browser/cocoa/bookmark_menu_bridge.mm b/chrome/browser/cocoa/bookmark_menu_bridge.mm index f2754a0..eea9b413 100644 --- a/chrome/browser/cocoa/bookmark_menu_bridge.mm +++ b/chrome/browser/cocoa/bookmark_menu_bridge.mm @@ -145,24 +145,49 @@ void BookmarkMenuBridge::ClearBookmarkMenu(NSMenu* menu) { } } +namespace { + +// Menus more than this many chars long will get trimmed +const NSUInteger kMaximumMenuWidthInChars = 65; + +// When trimming, use this many chars from each side +const NSUInteger kMenuTrimSizeInChars = 30; + +} + void BookmarkMenuBridge::AddNodeToMenu(BookmarkNode* node, NSMenu* menu) { for (int i = 0; i < node->GetChildCount(); i++) { BookmarkNode* child = node->GetChild(i); - // TODO(jrg): Should we limit the title length? - // For the Bookmark Bar under windows, items appear trimmed to ~19 - // chars (looks like a pixel width limit). - NSString* title = base::SysWideToNSString(child->GetTitle()); + NSString* full_title = base::SysWideToNSString(child->GetTitle()); + NSString* title = full_title; + if ([title length] > kMaximumMenuWidthInChars) { + // TODO(jrg): add a better heuristic? I'd really like to trim this + // by pixels, not by chars (font is not fixed width). + // For Safari, it appears that menu names >60 chars get split up to + // 30char + "..." + 30char. + title = [NSString stringWithFormat:@"%@…%@", + [title substringToIndex:kMenuTrimSizeInChars], + [title substringFromIndex:([title length] - + kMenuTrimSizeInChars)]]; + } NSMenuItem* item = [[[NSMenuItem alloc] initWithTitle:title action:nil keyEquivalent:@""] autorelease]; - [item setTarget:controller_]; - [item setAction:@selector(openBookmarkMenuItem:)]; - [item setTag:child->id()]; [menu addItem:item]; if (child->is_folder()) { NSMenu* submenu = [[[NSMenu alloc] initWithTitle:title] autorelease]; [menu setSubmenu:submenu forItem:item]; AddNodeToMenu(child, submenu); // recursive call + } else { + [item setTarget:controller_]; + [item setAction:@selector(openBookmarkMenuItem:)]; + [item setTag:child->id()]; + // Add a tooltip + std::string url_string = child->GetURL().possibly_invalid_spec(); + NSString* tooltip = [NSString stringWithFormat:@"%@\n%s", full_title, + url_string.c_str()]; + [item setToolTip:tooltip]; + } } } diff --git a/chrome/browser/cocoa/bookmark_menu_bridge_unittest.mm b/chrome/browser/cocoa/bookmark_menu_bridge_unittest.mm index ac37a72..2fe6127 100644 --- a/chrome/browser/cocoa/bookmark_menu_bridge_unittest.mm +++ b/chrome/browser/cocoa/bookmark_menu_bridge_unittest.mm @@ -66,7 +66,7 @@ TEST_F(BookmarkMenuBridgeTest, TestClearBookmarkMenu) { // Test that AddNodeToMenu() properly adds bookmark nodes as menus, // including the recursive case. TEST_F(BookmarkMenuBridgeTest, TestAddNodeToMenu) { - Profile* profile = browser_test_helper_.GetProfile(); + Profile* profile = browser_test_helper_.profile(); scoped_ptr<BookmarkMenuBridge> bridge(new BookmarkMenuBridge()); EXPECT_TRUE(bridge.get()); @@ -77,12 +77,23 @@ TEST_F(BookmarkMenuBridgeTest, TestAddNodeToMenu) { BookmarkNode* root = new BookmarkNode(model, GURL()); EXPECT_TRUE(model && root); - // 3 nodes; middle one has a child + const char* short_url = "http://foo/"; + const char* long_url = "http://super-duper-long-url--." + "that.cannot.possibly.fit.even-in-80-columns" + "or.be.reasonably-displayed-in-a-menu" + "without.looking-ridiculous.com/"; // 140 chars total + + // 3 nodes; middle one has a child, last one has a HUGE URL + // Set their titles to be the same as the URLs BookmarkNode* node = NULL; - for (int x = 0; x < 3; x++) { - node = new BookmarkNode(model, (x==1 ? GURL() : GURL("http://foo"))); - root->Add(x, node); - } + root->Add(0, new BookmarkNode(model, GURL(short_url))); + root->Add(1, new BookmarkNode(model, GURL())); + root->Add(2, new BookmarkNode(model, GURL(long_url))); + + root->GetChild(0)->SetTitle(ASCIIToWide(short_url)); + root->GetChild(2)->SetTitle(ASCIIToWide(long_url)); + + // And the submenu fo the middle one node = new BookmarkNode(model, GURL("http://sub")); root->GetChild(1)->Add(0, node); @@ -90,16 +101,37 @@ TEST_F(BookmarkMenuBridgeTest, TestAddNodeToMenu) { AddNodeToMenu(bridge.get(), root, menu); EXPECT_EQ(3, [menu numberOfItems]); - for (int x=0; x < 3; x++) { - NSMenuItem* item = [menu itemAtIndex:x]; - EXPECT_EQ(@selector(openBookmarkMenuItem:), [item action]); + + // Confirm for just the first and last (index 0 and 2) + for (int x=0; x<3; x+=2) { + EXPECT_EQ(@selector(openBookmarkMenuItem:), [[menu itemAtIndex:x] action]); + EXPECT_EQ(NO, [[menu itemAtIndex:x] hasSubmenu]); } - EXPECT_EQ(NO, [[menu itemAtIndex:0] hasSubmenu]); - EXPECT_EQ(NO, [[menu itemAtIndex:2] hasSubmenu]); + + // Now confirm the middle one (index 1) NSMenuItem* middle = [menu itemAtIndex:1]; EXPECT_NE(NO, [middle hasSubmenu]); EXPECT_EQ(1, [[middle submenu] numberOfItems]); + // Confirm 1st and 3rd item have the same action (that we specified). + // Make sure the submenu item does NOT have an bookmark action. + EXPECT_EQ([[menu itemAtIndex:0] action], [[menu itemAtIndex:2] action]); + EXPECT_NE([[menu itemAtIndex:0] action], [[menu itemAtIndex:1] action]); + + // Make sure a short title looks fine + NSString* s = [[menu itemAtIndex:0] title]; + EXPECT_TRUE([s isEqual:[NSString stringWithUTF8String:short_url]]); + + // Make sure a super-long title gets trimmed + s = [[menu itemAtIndex:0] title]; + EXPECT_TRUE([s length] < strlen(long_url)); + + // Confirm tooltips and confirm they are not trimmed (like the item + // name might be). Add tolerance for URL fixer-upping; + // e.g. http://foo becomes http://foo/) + EXPECT_GE([[[menu itemAtIndex:0] toolTip] length], (2*strlen(short_url) - 5)); + EXPECT_GE([[[menu itemAtIndex:2] toolTip] length], (2*strlen(long_url) - 5)); + delete root; // deletes all its kids delete model; } diff --git a/chrome/browser/cocoa/bookmark_menu_cocoa_controller_unittest.mm b/chrome/browser/cocoa/bookmark_menu_cocoa_controller_unittest.mm index f8f5f53..d70f51a 100644 --- a/chrome/browser/cocoa/bookmark_menu_cocoa_controller_unittest.mm +++ b/chrome/browser/cocoa/bookmark_menu_cocoa_controller_unittest.mm @@ -21,7 +21,7 @@ - (id)init { if ((self = [super init])) { helper_ = new BrowserTestHelper(); - BookmarkModel* model = helper_->GetBrowser()->profile()->GetBookmarkModel(); + BookmarkModel* model = helper_->browser()->profile()->GetBookmarkModel(); nodes_[0] = new BookmarkNode(model, GURL("http://0.com")); nodes_[1] = new BookmarkNode(model, GURL("http://1.com")); } @@ -63,5 +63,3 @@ TEST(BookmarkMenuCocoaControllerTest, TestOpenItem) { } [c release]; } - - diff --git a/chrome/browser/cocoa/browser_test_helper.h b/chrome/browser/cocoa/browser_test_helper.h index 6427547..19ce3b5 100644 --- a/chrome/browser/cocoa/browser_test_helper.h +++ b/chrome/browser/cocoa/browser_test_helper.h @@ -28,8 +28,8 @@ class BrowserTestHelper { delete profile_; } - Browser* GetBrowser() { return browser_; } - Profile* GetProfile() { return profile_; } + Browser* browser() { return browser_; } + Profile* profile() { return profile_; } private: Browser* browser_; diff --git a/chrome/browser/cocoa/browser_window_cocoa.h b/chrome/browser/cocoa/browser_window_cocoa.h index 355884f..661dca3 100644 --- a/chrome/browser/cocoa/browser_window_cocoa.h +++ b/chrome/browser/cocoa/browser_window_cocoa.h @@ -18,7 +18,8 @@ class Browser; // the Cocoa NSWindow. Cross-platform code will interact with this object when // it needs to manipulate the window. -class BrowserWindowCocoa : public BrowserWindow { +class BrowserWindowCocoa : public BrowserWindow, + public NotificationObserver { public: BrowserWindowCocoa(Browser* browser, BrowserWindowController* controller, @@ -66,6 +67,11 @@ class BrowserWindowCocoa : public BrowserWindow { virtual void ShowHTMLDialog(HtmlDialogUIDelegate* delegate, void* parent_window); + // Overridden from NotificationObserver + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details); + // Adds the given FindBar cocoa controller to this browser window. void AddFindBar(FindBarCocoaController* find_bar_cocoa_controller); diff --git a/chrome/browser/cocoa/browser_window_cocoa.mm b/chrome/browser/cocoa/browser_window_cocoa.mm index 1ad2e9b..de0f738 100644 --- a/chrome/browser/cocoa/browser_window_cocoa.mm +++ b/chrome/browser/cocoa/browser_window_cocoa.mm @@ -4,17 +4,30 @@ #include "base/gfx/rect.h" #include "base/logging.h" +#include "chrome/browser/bookmarks/bookmark_utils.h" #include "chrome/browser/cocoa/browser_window_cocoa.h" #include "chrome/browser/cocoa/browser_window_controller.h" #include "chrome/browser/browser.h" +#include "chrome/common/notification_service.h" +#include "chrome/common/pref_names.h" +#include "chrome/common/pref_service.h" +#include "chrome/browser/profile.h" BrowserWindowCocoa::BrowserWindowCocoa(Browser* browser, BrowserWindowController* controller, NSWindow* window) : window_(window), browser_(browser), controller_(controller) { + // This pref applies to all windows, so all must watch for it. + NotificationService* ns = NotificationService::current(); + ns->AddObserver(this, NotificationType::BOOKMARK_BAR_VISIBILITY_PREF_CHANGED, + NotificationService::AllSources()); } BrowserWindowCocoa::~BrowserWindowCocoa() { + NotificationService* ns = NotificationService::current(); + ns->RemoveObserver(this, + NotificationType::BOOKMARK_BAR_VISIBILITY_PREF_CHANGED, + NotificationService::AllSources()); } void BrowserWindowCocoa::Show() { @@ -133,18 +146,15 @@ void BrowserWindowCocoa::FocusToolbar() { } bool BrowserWindowCocoa::IsBookmarkBarVisible() const { - // Conversion from ObjC BOOL to C++ bool. - return [controller_ isBookmarkBarVisible] ? true : false; + return browser_->profile()->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar); } -// This is a little awkward. Internal to Chrome, V and C (in the MVC -// sense) tend to smear together. Thus, we have a call chain of -// C(browser_window)--> -// V(me;right here)--> -// C(BrowserWindowController)--> -// C(TabStripController) --> ... +// This is called from Browser, which in turn is called directly from +// a menu option. All we do here is set a preference. The act of +// setting the preference sends notifications to all windows who then +// know what to do. void BrowserWindowCocoa::ToggleBookmarkBar() { - [controller_ toggleBookmarkBar]; + bookmark_utils::ToggleWhenVisible(browser_->profile()); } void BrowserWindowCocoa::AddFindBar( @@ -203,6 +213,21 @@ void BrowserWindowCocoa::ShowHTMLDialog(HtmlDialogUIDelegate* delegate, NOTIMPLEMENTED(); } +void BrowserWindowCocoa::Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + switch (type.value) { + // Only the key window gets a direct toggle from the menu. + // Other windows hear about it from the notification. + case NotificationType::BOOKMARK_BAR_VISIBILITY_PREF_CHANGED: + [controller_ toggleBookmarkBar]; + break; + default: + NOTREACHED(); // we don't ask for anything else! + break; + } +} + void BrowserWindowCocoa::DestroyBrowser() { [controller_ destroyBrowser]; diff --git a/chrome/browser/cocoa/browser_window_cocoa_unittest.mm b/chrome/browser/cocoa/browser_window_cocoa_unittest.mm new file mode 100644 index 0000000..4308bc2 --- /dev/null +++ b/chrome/browser/cocoa/browser_window_cocoa_unittest.mm @@ -0,0 +1,93 @@ +// 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. + +#include "base/scoped_nsobject.h" +#include "base/scoped_nsautorelease_pool.h" +#include "base/scoped_ptr.h" +#include "base/string_util.h" +#include "chrome/browser/bookmarks/bookmark_utils.h" +#include "chrome/browser/cocoa/browser_test_helper.h" +#include "chrome/browser/cocoa/browser_window_cocoa.h" +#include "chrome/browser/cocoa/browser_window_controller.h" +#include "chrome/browser/cocoa/cocoa_test_helper.h" +#include "chrome/common/notification_type.h" +#include "testing/gtest/include/gtest/gtest.h" + +// A BrowserWindowCocoa that goes PONG when +// BOOKMARK_BAR_VISIBILITY_PREF_CHANGED is sent. This is so we can be +// sure we are observing it. +class BrowserWindowCocoaPong : public BrowserWindowCocoa { + public: + BrowserWindowCocoaPong(Browser* browser, + BrowserWindowController* controller, + NSWindow* window) : + BrowserWindowCocoa(browser, controller, window) { + pong_ = false; + } + virtual ~BrowserWindowCocoaPong() { } + + void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + if (type.value == NotificationType::BOOKMARK_BAR_VISIBILITY_PREF_CHANGED) + pong_ = true; + BrowserWindowCocoa::Observe(type, source, details); + } + + bool pong_; +}; + +// Main test class. +class BrowserWindowCocoaTest : public testing::Test { + + virtual void SetUp() { + controller_.reset([[BrowserWindowController alloc] + initWithBrowser:browser_helper_.browser() + takeOwnership:NO]); + } + + public: + // Order is very important here. We want the controller deleted + // before the pool, and want the pool deleted before + // BrowserTestHelper. + CocoaTestHelper cocoa_helper_; + BrowserTestHelper browser_helper_; + base::ScopedNSAutoreleasePool pool_; + scoped_nsobject<BrowserWindowController> controller_; +}; + + +TEST_F(BrowserWindowCocoaTest, TestNotification) { + BrowserWindowCocoaPong *bwc = new BrowserWindowCocoaPong( + browser_helper_.browser(), + controller_.get(), + cocoa_helper_.window()); + + EXPECT_FALSE(bwc->pong_); + bookmark_utils::ToggleWhenVisible(browser_helper_.profile()); + // Confirm we are listening + EXPECT_TRUE(bwc->pong_); + + delete bwc; + // If this does NOT crash it confirms we stopped listening in the destructor. + bookmark_utils::ToggleWhenVisible(browser_helper_.profile()); +} + + +TEST_F(BrowserWindowCocoaTest, TestBookmarkBarVisible) { + BrowserWindowCocoaPong *bwc = new BrowserWindowCocoaPong( + browser_helper_.browser(), + controller_.get(), + cocoa_helper_.window()); + scoped_ptr<BrowserWindowCocoaPong> scoped_bwc(bwc); + + bool before = bwc->IsBookmarkBarVisible(); + bookmark_utils::ToggleWhenVisible(browser_helper_.profile()); + EXPECT_NE(before, bwc->IsBookmarkBarVisible()); + + bookmark_utils::ToggleWhenVisible(browser_helper_.profile()); + EXPECT_EQ(before, bwc->IsBookmarkBarVisible()); +} + +/* TODO(???): test other methods of BrowserWindowCocoa */ diff --git a/chrome/browser/cocoa/browser_window_controller.h b/chrome/browser/cocoa/browser_window_controller.h index 337570a..0ccd73a 100644 --- a/chrome/browser/cocoa/browser_window_controller.h +++ b/chrome/browser/cocoa/browser_window_controller.h @@ -50,6 +50,7 @@ class TabStripModelObserverBridge; scoped_nsobject<TabStripController> tabStripController_; scoped_nsobject<FindBarCocoaController> findBarCocoaController_; scoped_ptr<StatusBubble> statusBubble_; + BOOL ownsBrowser_; // Only ever NO when testing } // Load the browser window nib and do any Cocoa-specific initialization. @@ -103,4 +104,12 @@ class TabStripModelObserverBridge; @end + +@interface BrowserWindowController(TestingAPI) + +// Allows us to initWithBrowser withOUT taking ownership of the browser. +- (id)initWithBrowser:(Browser*)browser takeOwnership:(BOOL)ownIt; + +@end // BrowserWindowController(TestingAPI) + #endif // CHROME_BROWSER_COCOA_BROWSER_WINDOW_CONTROLLER_H_ diff --git a/chrome/browser/cocoa/browser_window_controller.mm b/chrome/browser/cocoa/browser_window_controller.mm index 4a318f7..048b641 100644 --- a/chrome/browser/cocoa/browser_window_controller.mm +++ b/chrome/browser/cocoa/browser_window_controller.mm @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/mac_util.h" #include "base/sys_string_conversions.h" #include "chrome/app/chrome_dll_resource.h" // IDC_* #include "chrome/browser/browser.h" @@ -57,9 +58,20 @@ willPositionSheet:(NSWindow *)sheet // Takes ownership of |browser|. Note that the nib also sets this controller // up as the window's delegate. - (id)initWithBrowser:(Browser*)browser { - if ((self = [super initWithWindowNibName:@"BrowserWindow"])) { + return [self initWithBrowser:browser takeOwnership:YES]; +} + +// Private (TestingAPI) init routine with testing options. +- (id)initWithBrowser:(Browser*)browser takeOwnership:(BOOL)ownIt { + // Use initWithWindowNibPath:: instead of initWithWindowNibName: so we + // can override it in a unit test. + NSString *nibpath = [mac_util::MainAppBundle() + pathForResource:@"BrowserWindow" + ofType:@"nib"]; + if ((self = [super initWithWindowNibPath:nibpath owner:self])) { DCHECK(browser); browser_.reset(browser); + ownsBrowser_ = ownIt; tabObserver_.reset( new TabStripModelObserverBridge(browser->tabstrip_model(), self)); windowShim_.reset(new BrowserWindowCocoa(browser, self, [self window])); @@ -119,6 +131,9 @@ willPositionSheet:(NSWindow *)sheet - (void)dealloc { browser_->CloseAllTabs(); + // Under certain testing configurations we may not actually own the browser. + if (ownsBrowser_ == NO) + browser_.release(); [super dealloc]; } diff --git a/chrome/browser/cocoa/toolbar_controller_unittest.mm b/chrome/browser/cocoa/toolbar_controller_unittest.mm index 6f8e618..646a8ea 100644 --- a/chrome/browser/cocoa/toolbar_controller_unittest.mm +++ b/chrome/browser/cocoa/toolbar_controller_unittest.mm @@ -24,7 +24,7 @@ class ToolbarControllerTest : public testing::Test { }; ToolbarControllerTest() { - Browser* browser = helper_.GetBrowser(); + Browser* browser = helper_.browser(); 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 @@ -33,7 +33,7 @@ class ToolbarControllerTest : public testing::Test { bar_.reset( [[ToolbarController alloc] initWithModel:browser->toolbar_model() commands:browser->command_updater() - profile:helper_.GetProfile()]); + profile:helper_.profile()]); EXPECT_TRUE([bar_ view]); NSView* parent = [cocoa_helper_.window() contentView]; [parent addSubview:[bar_ view]]; @@ -60,14 +60,14 @@ class ToolbarControllerTest : public testing::Test { // Test the initial state that everything is sync'd up TEST_F(ToolbarControllerTest, InitialState) { - CommandUpdater* updater = helper_.GetBrowser()->command_updater(); + CommandUpdater* updater = helper_.browser()->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(); + CommandUpdater* updater = helper_.browser()->command_updater(); EXPECT_FALSE(updater->IsCommandEnabled(IDC_BACK)); EXPECT_FALSE(updater->IsCommandEnabled(IDC_FORWARD)); updater->UpdateCommandEnabled(IDC_BACK, true); |