diff options
| -rw-r--r-- | chrome/browser/app_controller_mac.mm | 5 | ||||
| -rw-r--r-- | chrome/browser/sync/sync_ui_util_mac.h | 31 | ||||
| -rw-r--r-- | chrome/browser/sync/sync_ui_util_mac.mm | 88 | ||||
| -rw-r--r-- | chrome/browser/sync/sync_ui_util_mac_unittest.mm | 119 | ||||
| -rw-r--r-- | chrome/browser/ui/cocoa/browser_window_controller.h | 8 | ||||
| -rw-r--r-- | chrome/browser/ui/cocoa/browser_window_controller.mm | 73 | ||||
| -rw-r--r-- | chrome/browser/ui/cocoa/browser_window_controller_unittest.mm | 173 | ||||
| -rw-r--r-- | chrome/browser/ui/cocoa/extensions/extension_action_context_menu_browsertest.mm | 18 | ||||
| -rw-r--r-- | chrome/chrome_browser.gypi | 2 | ||||
| -rw-r--r-- | chrome/chrome_tests_unit.gypi | 1 |
10 files changed, 264 insertions, 254 deletions
diff --git a/chrome/browser/app_controller_mac.mm b/chrome/browser/app_controller_mac.mm index d635850..28fec39 100644 --- a/chrome/browser/app_controller_mac.mm +++ b/chrome/browser/app_controller_mac.mm @@ -37,7 +37,6 @@ #include "chrome/browser/sessions/tab_restore_service_factory.h" #include "chrome/browser/sync/profile_sync_service.h" #include "chrome/browser/sync/sync_ui_util.h" -#include "chrome/browser/sync/sync_ui_util_mac.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_commands.h" #include "chrome/browser/ui/browser_finder.h" @@ -755,7 +754,9 @@ void RecordLastRunAppBundlePath() { } enable = lastProfile->IsSyncAccessible() && [self keyWindowIsNotModal]; - sync_ui_util::UpdateSyncItem(item, enable, lastProfile); + [BrowserWindowController updateSigninItem:item + shouldShow:enable + currentProfile:lastProfile]; break; } case IDC_FEEDBACK: diff --git a/chrome/browser/sync/sync_ui_util_mac.h b/chrome/browser/sync/sync_ui_util_mac.h deleted file mode 100644 index 09f63a1..0000000 --- a/chrome/browser/sync/sync_ui_util_mac.h +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright (c) 2012 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_SYNC_SYNC_UI_UTIL_MAC_H_ -#define CHROME_BROWSER_SYNC_SYNC_UI_UTIL_MAC_H_ - -#include <string> - -#include "chrome/browser/sync/sync_ui_util.h" - -#import <Cocoa/Cocoa.h> - -class Profile; - -namespace sync_ui_util { - -// Updates a bookmark sync UI item (expected to be a menu item). This is -// called every time a menu containing a sync UI item is displayed. -void UpdateSyncItem(id syncItem, BOOL syncEnabled, Profile* profile); - -// This function (used by UpdateSyncItem) is only exposed for testing. -// Just use UpdateSyncItem() instead. -void UpdateSyncItemForStatus(id syncItem, BOOL syncEnabled, - sync_ui_util::MessageType status, - const std::string& userName); - -} // namespace sync_ui_util - -#endif // CHROME_BROWSER_SYNC_SYNC_UI_UTIL_MAC_H_ - diff --git a/chrome/browser/sync/sync_ui_util_mac.mm b/chrome/browser/sync/sync_ui_util_mac.mm deleted file mode 100644 index dbf40f9..0000000 --- a/chrome/browser/sync/sync_ui_util_mac.mm +++ /dev/null @@ -1,88 +0,0 @@ -// Copyright (c) 2012 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/sync/sync_ui_util_mac.h" - -#import <Cocoa/Cocoa.h> - -#include "base/utf_string_conversions.h" -#include "base/logging.h" -#include "chrome/browser/prefs/pref_service.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/signin/signin_manager.h" -#include "chrome/browser/signin/signin_manager_factory.h" -#include "chrome/browser/sync/profile_sync_service.h" -#include "chrome/browser/sync/profile_sync_service_factory.h" -#include "chrome/browser/sync/sync_ui_util.h" -#include "chrome/common/pref_names.h" -#include "grit/chromium_strings.h" -#include "grit/generated_resources.h" -#include "ui/base/l10n/l10n_util.h" -#include "ui/base/l10n/l10n_util_mac.h" - -using l10n_util::GetStringUTF16; -using l10n_util::GetNSStringWithFixup; -using l10n_util::GetNSStringFWithFixup; - -namespace sync_ui_util { - -void UpdateSyncItem(id syncItem, BOOL syncEnabled, Profile* profile) { - ProfileSyncService* syncService = - ProfileSyncServiceFactory::GetInstance()->GetForProfile( - profile->GetOriginalProfile()); - SigninManager* signin = SigninManagerFactory::GetForProfile(profile); - UpdateSyncItemForStatus( - syncItem, - syncEnabled, - sync_ui_util::GetStatus(syncService, *signin), - profile->GetPrefs()->GetString(prefs::kGoogleServicesUsername)); -} - -void UpdateSyncItemForStatus(id syncItem, BOOL syncEnabled, - sync_ui_util::MessageType status, - const std::string& userName) { - DCHECK([syncItem isKindOfClass:[NSMenuItem class]]); - NSMenuItem* syncMenuItem = static_cast<NSMenuItem*>(syncItem); - // Look for a separator immediately after the menu item. - NSMenuItem* followingSeparator = nil; - NSMenu* menu = [syncItem menu]; - if (menu) { - NSInteger syncItemIndex = [menu indexOfItem:syncMenuItem]; - DCHECK_NE(syncItemIndex, -1); - if ((syncItemIndex + 1) < [menu numberOfItems]) { - NSMenuItem* menuItem = [menu itemAtIndex:(syncItemIndex + 1)]; - if ([menuItem isSeparatorItem]) { - followingSeparator = menuItem; - } - } - } - - // TODO(akalin): consolidate this code with the equivalent Windows code in - // chrome/browser/ui/views/toolbar_view.cc. - NSString* title = NULL; - switch (status) { - case sync_ui_util::SYNCED: - title = GetNSStringFWithFixup(IDS_SYNC_MENU_SYNCED_LABEL, - UTF8ToUTF16(userName)); - break; - case sync_ui_util::SYNC_ERROR: - title = GetNSStringWithFixup(IDS_SYNC_MENU_SYNC_ERROR_LABEL); - break; - case sync_ui_util::PRE_SYNCED: - case sync_ui_util::SYNC_PROMO: - default: - title = GetNSStringFWithFixup(IDS_SYNC_MENU_PRE_SYNCED_LABEL, - GetStringUTF16(IDS_SHORT_PRODUCT_NAME)); - break; - } - [syncMenuItem setTitle:title]; - - // If we don't have a sync service, hide any sync-related menu - // items. However, sync_menu_item is enabled/disabled outside of this - // function so we don't touch it here, and separators are always disabled. - [syncMenuItem setHidden:!syncEnabled]; - [followingSeparator setHidden:!syncEnabled]; -} - -} // namespace sync_ui_util diff --git a/chrome/browser/sync/sync_ui_util_mac_unittest.mm b/chrome/browser/sync/sync_ui_util_mac_unittest.mm deleted file mode 100644 index b9ddc9c..0000000 --- a/chrome/browser/sync/sync_ui_util_mac_unittest.mm +++ /dev/null @@ -1,119 +0,0 @@ -// Copyright (c) 2012 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/sync/sync_ui_util_mac.h" - -#import <Cocoa/Cocoa.h> - -#include "base/memory/scoped_nsobject.h" -#include "base/utf_string_conversions.h" -#include "chrome/app/chrome_command_ids.h" -#include "chrome/browser/ui/cocoa/cocoa_test_helper.h" -#include "grit/chromium_strings.h" -#include "grit/generated_resources.h" -#include "testing/gtest/include/gtest/gtest.h" -#include "ui/base/l10n/l10n_util.h" -#include "ui/base/l10n/l10n_util_mac.h" - -namespace { - -class SyncStatusUIHelperMacTest : public CocoaTest { -}; - -TEST_F(SyncStatusUIHelperMacTest, UpdateSyncItem) { - scoped_nsobject<NSMenuItem> syncMenuItem( - [[NSMenuItem alloc] initWithTitle:@"" - action:@selector(commandDispatch) - keyEquivalent:@""]); - [syncMenuItem setTag:IDC_SHOW_SYNC_SETUP]; - - std::string userName = "foo@example.com"; - - NSString* bookmarksSynced = - l10n_util::GetNSStringFWithFixup(IDS_SYNC_MENU_SYNCED_LABEL, - UTF8ToUTF16(userName)); - NSString* bookmarkSyncError = - l10n_util::GetNSStringWithFixup(IDS_SYNC_MENU_SYNC_ERROR_LABEL); - NSString* startSync = - l10n_util::GetNSStringFWithFixup( - IDS_SYNC_MENU_PRE_SYNCED_LABEL, - l10n_util::GetStringUTF16(IDS_SHORT_PRODUCT_NAME)); - - [syncMenuItem setTitle:@""]; - [syncMenuItem setHidden:NO]; - - sync_ui_util::UpdateSyncItemForStatus(syncMenuItem, NO, - sync_ui_util::PRE_SYNCED, - userName); - EXPECT_TRUE([[syncMenuItem title] isEqualTo:startSync]); - EXPECT_TRUE([syncMenuItem isHidden]); - - [syncMenuItem setTitle:@""]; - [syncMenuItem setHidden:YES]; - sync_ui_util::UpdateSyncItemForStatus(syncMenuItem, YES, - sync_ui_util::SYNC_ERROR, - userName); - EXPECT_TRUE([[syncMenuItem title] isEqualTo:bookmarkSyncError]); - EXPECT_FALSE([syncMenuItem isHidden]); - - [syncMenuItem setTitle:@""]; - [syncMenuItem setHidden:NO]; - sync_ui_util::UpdateSyncItemForStatus(syncMenuItem, NO, - sync_ui_util::SYNCED, - userName); - EXPECT_TRUE([[syncMenuItem title] isEqualTo:bookmarksSynced]); - EXPECT_TRUE([syncMenuItem isHidden]); -} - -TEST_F(SyncStatusUIHelperMacTest, UpdateSyncItemWithSeparator) { - scoped_nsobject<NSMenu> menu([[NSMenu alloc] initWithTitle:@""]); - NSMenuItem* syncMenuItem = - [menu addItemWithTitle:@"" - action:@selector(commandDispatch) - keyEquivalent:@""]; - [syncMenuItem setTag:IDC_SHOW_SYNC_SETUP]; - NSMenuItem* followingSeparator = [NSMenuItem separatorItem]; - [menu addItem:followingSeparator]; - - const sync_ui_util::MessageType kStatus = sync_ui_util::PRE_SYNCED; - - [syncMenuItem setHidden:NO]; - [followingSeparator setHidden:NO]; - sync_ui_util::UpdateSyncItemForStatus(syncMenuItem, NO, kStatus, ""); - EXPECT_FALSE([followingSeparator isEnabled]); - EXPECT_TRUE([syncMenuItem isHidden]); - EXPECT_TRUE([followingSeparator isHidden]); - - [syncMenuItem setHidden:YES]; - [followingSeparator setHidden:YES]; - sync_ui_util::UpdateSyncItemForStatus(syncMenuItem, YES, kStatus, ""); - EXPECT_FALSE([followingSeparator isEnabled]); - EXPECT_FALSE([syncMenuItem isHidden]); - EXPECT_FALSE([followingSeparator isHidden]); -} - -TEST_F(SyncStatusUIHelperMacTest, UpdateSyncItemWithNonSeparator) { - scoped_nsobject<NSMenu> menu([[NSMenu alloc] initWithTitle:@""]); - NSMenuItem* syncMenuItem = - [menu addItemWithTitle:@"" - action:@selector(commandDispatch) - keyEquivalent:@""]; - [syncMenuItem setTag:IDC_SHOW_SYNC_SETUP]; - NSMenuItem* followingNonSeparator = - [menu addItemWithTitle:@"" - action:@selector(commandDispatch) - keyEquivalent:@""]; - - const sync_ui_util::MessageType kStatus = sync_ui_util::PRE_SYNCED; - - sync_ui_util::UpdateSyncItemForStatus(syncMenuItem, NO, kStatus, ""); - EXPECT_TRUE([followingNonSeparator isEnabled]); - EXPECT_FALSE([followingNonSeparator isHidden]); - - sync_ui_util::UpdateSyncItemForStatus(syncMenuItem, YES, kStatus, ""); - EXPECT_TRUE([followingNonSeparator isEnabled]); - EXPECT_FALSE([followingNonSeparator isHidden]); -} - -} // namespace diff --git a/chrome/browser/ui/cocoa/browser_window_controller.h b/chrome/browser/ui/cocoa/browser_window_controller.h index c2f01f6..9f8eda9 100644 --- a/chrome/browser/ui/cocoa/browser_window_controller.h +++ b/chrome/browser/ui/cocoa/browser_window_controller.h @@ -14,7 +14,6 @@ #include "base/memory/scoped_nsobject.h" #include "base/memory/scoped_ptr.h" -#include "chrome/browser/sync/sync_ui_util.h" #import "chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.h" #import "chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.h" #import "chrome/browser/ui/cocoa/browser_command_executor.h" @@ -175,6 +174,13 @@ class WebContents; // BWC. This method returns nil if no window in the chain has a BWC. + (BrowserWindowController*)browserWindowControllerForView:(NSView*)view; +// Helper method used to update the "Signin" menu item to reflect the current +// signed in state. Class-level function as it's still required even when there +// are no open browser windows. ++ (void)updateSigninItem:(id)signinItem + shouldShow:(BOOL)showSigninMenuItem + currentProfile:(Profile*)profile; + // Load the browser window nib and do any Cocoa-specific initialization. // Takes ownership of |browser|. - (id)initWithBrowser:(Browser*)browser; diff --git a/chrome/browser/ui/cocoa/browser_window_controller.mm b/chrome/browser/ui/cocoa/browser_window_controller.mm index 183f440..02f4220 100644 --- a/chrome/browser/ui/cocoa/browser_window_controller.mm +++ b/chrome/browser/ui/cocoa/browser_window_controller.mm @@ -12,6 +12,7 @@ #include "base/mac/mac_util.h" #import "base/memory/scoped_nsobject.h" #include "base/sys_string_conversions.h" +#include "base/utf_string_conversions.h" #include "chrome/app/chrome_command_ids.h" // IDC_* #include "chrome/browser/bookmarks/bookmark_editor.h" #include "chrome/browser/bookmarks/bookmark_model_factory.h" @@ -21,8 +22,12 @@ #include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile_info_cache.h" #include "chrome/browser/profiles/profile_manager.h" +#include "chrome/browser/signin/signin_manager.h" +#include "chrome/browser/signin/signin_manager_factory.h" #include "chrome/browser/sync/profile_sync_service.h" -#include "chrome/browser/sync/sync_ui_util_mac.h" +#include "chrome/browser/sync/profile_sync_service_factory.h" +#include "chrome/browser/sync/sync_global_error.h" +#include "chrome/browser/sync/sync_ui_util.h" #include "chrome/browser/themes/theme_service.h" #include "chrome/browser/themes/theme_service_factory.h" #include "chrome/browser/ui/browser.h" @@ -73,12 +78,17 @@ #include "content/public/browser/render_view_host.h" #include "content/public/browser/render_widget_host_view.h" #include "content/public/browser/web_contents.h" +#include "grit/chromium_strings.h" #include "grit/generated_resources.h" #include "grit/locale_settings.h" #include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util_mac.h" #include "ui/gfx/mac/scoped_ns_disable_screen_updates.h" +using l10n_util::GetStringUTF16; +using l10n_util::GetNSStringWithFixup; +using l10n_util::GetNSStringFWithFixup; + // ORGANIZATION: This is a big file. It is (in principle) organized as follows // (in order): // 1. Interfaces. Very short, one-time-use classes may include an implementation @@ -214,6 +224,61 @@ enum { return [BrowserWindowController browserWindowControllerForWindow:window]; } ++ (void)updateSigninItem:(id)signinItem + shouldShow:(BOOL)showSigninMenuItem + currentProfile:(Profile*)profile { + DCHECK([signinItem isKindOfClass:[NSMenuItem class]]); + NSMenuItem* signinMenuItem = static_cast<NSMenuItem*>(signinItem); + + // Look for a separator immediately after the menu item so it can be hidden + // or shown appropriately along with the signin menu item. + NSMenuItem* followingSeparator = nil; + NSMenu* menu = [signinItem menu]; + if (menu) { + NSInteger signinItemIndex = [menu indexOfItem:signinMenuItem]; + DCHECK_NE(signinItemIndex, -1); + if ((signinItemIndex + 1) < [menu numberOfItems]) { + NSMenuItem* menuItem = [menu itemAtIndex:(signinItemIndex + 1)]; + if ([menuItem isSeparatorItem]) { + followingSeparator = menuItem; + } + } + } + + // Figure out what string to display in the signin menu item - depending on + // the state of the signed-in services, this will be one of: + // "Sign in to <product name>" + // "Signed in as <username>" + // "Signin Error" + ProfileSyncService* syncService = profile->IsSyncAccessible() ? + ProfileSyncServiceFactory::GetInstance()->GetForProfile( + profile->GetOriginalProfile()) : NULL; + SigninManager* signin = SigninManagerFactory::GetForProfile(profile); + std::string userName = signin->GetAuthenticatedUsername(); + NSString* title = NULL; + if (userName.empty() || signin->AuthInProgress() || + (syncService && !syncService->HasSyncSetupCompleted())) { + // Not signed in yet - display the default string. + title = GetNSStringFWithFixup(IDS_SYNC_MENU_PRE_SYNCED_LABEL, + GetStringUTF16(IDS_SHORT_PRODUCT_NAME)); + } else if (signin->signin_global_error()->HasBadge()) { + // TODO(atwilson): Change this string to a generic signin error instead of + // a "Sync error" string once we allow signin without sync. + title = GetNSStringWithFixup(IDS_SYNC_MENU_SYNC_ERROR_LABEL); + } else if (syncService && syncService->sync_global_error() && + syncService->sync_global_error()->HasBadge()) { + title = GetNSStringWithFixup(IDS_SYNC_MENU_SYNC_ERROR_LABEL); + } else { + // Signed in. + title = GetNSStringFWithFixup(IDS_SYNC_MENU_SYNCED_LABEL, + UTF8ToUTF16(userName)); + } + + [signinMenuItem setTitle:title]; + [signinMenuItem setHidden:!showSigninMenuItem]; + [followingSeparator setHidden:!showSigninMenuItem]; +} + // Load the browser window nib and do any Cocoa-specific initialization. // Takes ownership of |browser|. Note that the nib also sets this controller // up as the window's delegate. @@ -1051,11 +1116,13 @@ enum { } break; } - case IDC_SHOW_SYNC_SETUP: { + case IDC_SHOW_SIGNIN: { Profile* original_profile = browser_->profile()->GetOriginalProfile(); enable &= original_profile->IsSyncAccessible(); - sync_ui_util::UpdateSyncItem(item, enable, original_profile); + [BrowserWindowController updateSigninItem:item + shouldShow:enable + currentProfile:original_profile]; break; } default: diff --git a/chrome/browser/ui/cocoa/browser_window_controller_unittest.mm b/chrome/browser/ui/cocoa/browser_window_controller_unittest.mm index e5b0985..344a1f4 100644 --- a/chrome/browser/ui/cocoa/browser_window_controller_unittest.mm +++ b/chrome/browser/ui/cocoa/browser_window_controller_unittest.mm @@ -7,9 +7,17 @@ #include "base/mac/mac_util.h" #import "base/memory/scoped_nsobject.h" #include "base/memory/scoped_ptr.h" +#include "base/utf_string_conversions.h" #include "chrome/app/chrome_command_ids.h" #include "chrome/browser/prefs/pref_service.h" -#include "chrome/browser/sync/sync_ui_util.h" +#include "chrome/browser/signin/signin_global_error.h" +#include "chrome/browser/signin/signin_manager.h" +#include "chrome/browser/signin/signin_manager_fake.h" +#include "chrome/browser/signin/signin_manager_factory.h" +#include "chrome/browser/sync/profile_sync_service.h" +#include "chrome/browser/sync/profile_sync_service_mock.h" +#include "chrome/browser/sync/profile_sync_service_factory.h" +#include "chrome/browser/sync/sync_global_error.h" #include "chrome/browser/ui/browser_list.h" #include "chrome/browser/ui/browser_window.h" #include "chrome/browser/ui/cocoa/cocoa_profile_test.h" @@ -20,9 +28,32 @@ #include "chrome/test/base/testing_profile.h" #include "content/public/browser/notification_service.h" #include "content/public/test/test_utils.h" +#include "grit/chromium_strings.h" #include "grit/generated_resources.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util_mac.h" +using ::testing::Return; + +namespace { +class FakeAuthStatusProvider : public SigninGlobalError::AuthStatusProvider { + public: + FakeAuthStatusProvider() : auth_error_(GoogleServiceAuthError::None()) {} + + // AuthStatusProvider implementation. + GoogleServiceAuthError GetAuthStatus() const OVERRIDE { return auth_error_; } + + void set_auth_error(const GoogleServiceAuthError& error) { + auth_error_ = error; + } + + private: + GoogleServiceAuthError auth_error_; +}; + +} // namespace + @interface BrowserWindowController (JustForTesting) // Already defined in BWC. - (void)saveWindowPositionIfNeeded; @@ -632,6 +663,146 @@ TEST_F(BrowserWindowControllerTest, TestStatusBubblePositioning) { EXPECT_FALSE(NSEqualPoints(bubbleOrigin, bubbleOriginWithDevTools)); } +TEST_F(BrowserWindowControllerTest, TestSigninMenuItemNoErrors) { + scoped_nsobject<NSMenuItem> syncMenuItem( + [[NSMenuItem alloc] initWithTitle:@"" + action:@selector(commandDispatch) + keyEquivalent:@""]); + [syncMenuItem setTag:IDC_SHOW_SYNC_SETUP]; + + NSString* startSignin = + l10n_util::GetNSStringFWithFixup( + IDS_SYNC_MENU_PRE_SYNCED_LABEL, + l10n_util::GetStringUTF16(IDS_SHORT_PRODUCT_NAME)); + + // Make sure shouldShow parameter is obeyed, and we get the default + // label if not signed in. + [BrowserWindowController updateSigninItem:syncMenuItem + shouldShow:YES + currentProfile:profile()]; + + EXPECT_TRUE([[syncMenuItem title] isEqualTo:startSignin]); + EXPECT_FALSE([syncMenuItem isHidden]); + + [BrowserWindowController updateSigninItem:syncMenuItem + shouldShow:NO + currentProfile:profile()]; + EXPECT_TRUE([[syncMenuItem title] isEqualTo:startSignin]); + EXPECT_TRUE([syncMenuItem isHidden]); + + // Now sign in. + std::string username = "foo@example.com"; + NSString* alreadySignedIn = + l10n_util::GetNSStringFWithFixup(IDS_SYNC_MENU_SYNCED_LABEL, + UTF8ToUTF16(username)); + SigninManager* signin = SigninManagerFactory::GetForProfile(profile()); + signin->SetAuthenticatedUsername(username); + ProfileSyncService* sync = + ProfileSyncServiceFactory::GetForProfile(profile()); + sync->SetSyncSetupCompleted(); + [BrowserWindowController updateSigninItem:syncMenuItem + shouldShow:YES + currentProfile:profile()]; + EXPECT_TRUE([[syncMenuItem title] isEqualTo:alreadySignedIn]); + EXPECT_FALSE([syncMenuItem isHidden]); +} + +TEST_F(BrowserWindowControllerTest, TestSigninMenuItemAuthError) { + scoped_nsobject<NSMenuItem> syncMenuItem( + [[NSMenuItem alloc] initWithTitle:@"" + action:@selector(commandDispatch) + keyEquivalent:@""]); + [syncMenuItem setTag:IDC_SHOW_SYNC_SETUP]; + + // Now sign in. + std::string username = "foo@example.com"; + SigninManager* signin = SigninManagerFactory::GetForProfile(profile()); + signin->SetAuthenticatedUsername(username); + ProfileSyncService* sync = + ProfileSyncServiceFactory::GetForProfile(profile()); + sync->SetSyncSetupCompleted(); + // Force an auth error. + FakeAuthStatusProvider provider; + GoogleServiceAuthError error( + GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS); + provider.set_auth_error(error); + signin->signin_global_error()->AddProvider(&provider); + [BrowserWindowController updateSigninItem:syncMenuItem + shouldShow:YES + currentProfile:profile()]; + NSString* authError = + l10n_util::GetNSStringWithFixup(IDS_SYNC_MENU_SYNC_ERROR_LABEL); + EXPECT_TRUE([[syncMenuItem title] isEqualTo:authError]); + EXPECT_FALSE([syncMenuItem isHidden]); + + signin->signin_global_error()->RemoveProvider(&provider); +} + +// If there's a separator after the signin menu item, make sure it is hidden/ +// shown when the signin menu item is. +TEST_F(BrowserWindowControllerTest, TestSigninMenuItemWithSeparator) { + scoped_nsobject<NSMenu> menu([[NSMenu alloc] initWithTitle:@""]); + NSMenuItem* signinMenuItem = + [menu addItemWithTitle:@"" + action:@selector(commandDispatch) + keyEquivalent:@""]; + [signinMenuItem setTag:IDC_SHOW_SYNC_SETUP]; + NSMenuItem* followingSeparator = [NSMenuItem separatorItem]; + [menu addItem:followingSeparator]; + [signinMenuItem setHidden:NO]; + [followingSeparator setHidden:NO]; + + [BrowserWindowController updateSigninItem:signinMenuItem + shouldShow:NO + currentProfile:profile()]; + + EXPECT_FALSE([followingSeparator isEnabled]); + EXPECT_TRUE([signinMenuItem isHidden]); + EXPECT_TRUE([followingSeparator isHidden]); + + [BrowserWindowController updateSigninItem:signinMenuItem + shouldShow:YES + currentProfile:profile()]; + + EXPECT_FALSE([followingSeparator isEnabled]); + EXPECT_FALSE([signinMenuItem isHidden]); + EXPECT_FALSE([followingSeparator isHidden]); +} + +// If there's a non-separator item after the signin menu item, it should not +// change state when the signin menu item is hidden/shown. +TEST_F(BrowserWindowControllerTest, TestSigninMenuItemWithNonSeparator) { + scoped_nsobject<NSMenu> menu([[NSMenu alloc] initWithTitle:@""]); + NSMenuItem* signinMenuItem = + [menu addItemWithTitle:@"" + action:@selector(commandDispatch) + keyEquivalent:@""]; + [signinMenuItem setTag:IDC_SHOW_SYNC_SETUP]; + NSMenuItem* followingNonSeparator = + [menu addItemWithTitle:@"" + action:@selector(commandDispatch) + keyEquivalent:@""]; + [signinMenuItem setHidden:NO]; + [followingNonSeparator setHidden:NO]; + + [BrowserWindowController updateSigninItem:signinMenuItem + shouldShow:NO + currentProfile:profile()]; + + EXPECT_TRUE([followingNonSeparator isEnabled]); + EXPECT_TRUE([signinMenuItem isHidden]); + EXPECT_FALSE([followingNonSeparator isHidden]); + + [followingNonSeparator setHidden:YES]; + [BrowserWindowController updateSigninItem:signinMenuItem + shouldShow:YES + currentProfile:profile()]; + + EXPECT_TRUE([followingNonSeparator isEnabled]); + EXPECT_FALSE([signinMenuItem isHidden]); + EXPECT_TRUE([followingNonSeparator isHidden]); +} + @interface BrowserWindowControllerFakeFullscreen : BrowserWindowController { @private // We release the window ourselves, so we don't have to rely on the unittest diff --git a/chrome/browser/ui/cocoa/extensions/extension_action_context_menu_browsertest.mm b/chrome/browser/ui/cocoa/extensions/extension_action_context_menu_browsertest.mm index 9854e7e..d3c8cf0 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_action_context_menu_browsertest.mm +++ b/chrome/browser/ui/cocoa/extensions/extension_action_context_menu_browsertest.mm @@ -88,10 +88,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionActionContextMenuTest, BasicTest) { // Test that browser action context menus work. Browser actions have their // menus created during browser initialization, when there is no tab. This // test simulates that and checks the menu is operational. -// TODO(atwilson): Re-enable this test with a change to free the browser -// properly in a way that does not leave dangling references in the -// ToolbarController. -IN_PROC_BROWSER_TEST_F(ExtensionActionContextMenuTest, DISABLED_BrowserAction) { +IN_PROC_BROWSER_TEST_F(ExtensionActionContextMenuTest, BrowserAction) { extension_ = InstallExtension( test_data_dir_.AppendASCII("browsertest") .AppendASCII("browser_action_popup"), @@ -102,18 +99,27 @@ IN_PROC_BROWSER_TEST_F(ExtensionActionContextMenuTest, DISABLED_BrowserAction) { action_ = action_manager->GetBrowserAction(*extension_); EXPECT_TRUE(action_); - scoped_ptr<Browser> empty_browser( + Browser* empty_browser( new Browser(Browser::CreateParams(browser()->profile()))); scoped_nsobject<ExtensionActionContextMenu> menu; menu.reset([[ExtensionActionContextMenu alloc] initWithExtension:extension_ - browser:empty_browser.get() + browser:empty_browser extensionAction:action_]); NSMenuItem* inspectItem = [menu itemWithTag: extension_action_context_menu::kExtensionContextInspect]; EXPECT_TRUE(inspectItem); + + // Close the empty browser. Can't just free it directly because there are + // dangling references in the various native controllers that must be + // cleaned up first. + NSWindow* window = empty_browser->window()->GetNativeWindow(); + BrowserWindowController* wc = + [BrowserWindowController browserWindowControllerForWindow:window]; + ASSERT_TRUE(wc != NULL); + [wc destroyBrowser]; } IN_PROC_BROWSER_TEST_F(ExtensionActionContextMenuTest, RunInspectPopup) { diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index dd869d4..7ba5e26 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -2038,8 +2038,6 @@ 'browser/sync/sync_prefs.h', 'browser/sync/sync_ui_util.cc', 'browser/sync/sync_ui_util.h', - 'browser/sync/sync_ui_util_mac.h', - 'browser/sync/sync_ui_util_mac.mm', 'browser/sync/user_selectable_sync_type.h', 'browser/sync_file_system/drive_file_sync_client.cc', 'browser/sync_file_system/drive_file_sync_client.h', diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 1120fbc..3a05984 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -1125,7 +1125,6 @@ 'browser/sync/profile_sync_test_util.h', 'browser/sync/sync_global_error_unittest.cc', 'browser/sync/sync_prefs_unittest.cc', - 'browser/sync/sync_ui_util_mac_unittest.mm', 'browser/sync/sync_ui_util_unittest.cc', 'browser/sync/test/test_http_bridge_factory.cc', 'browser/sync/test/test_http_bridge_factory.h', |