Skip to content

GitLab

T
tangled
Commit d51574fc authored by msarda's avatar msarda Committed by Commit bot
Browse files
Options

Remove the sign-in menu item on macOS. 

This CL removes the sign-in menu on macOS as histogram indicate it is very rarely used.

The changes in the XIB file consists in removing this menu item from the menu nib file.

BUG=608181

Review-Url: https://codereview.chromium.org/2435053002
Cr-Commit-Position: refs/heads/master@{#427046}
parent 63170658
Hide whitespace changes
Inline Side-by-side
Showing with 0 additions and 307 deletions
chrome/app/generated_resources.grd
View file @ d51574fc
...@@ -11922,24 +11922,6 @@ Tell us what happened exactly before you got the profile error message: ...@@ -11922,24 +11922,6 @@ Tell us what happened exactly before you got the profile error message:
Success! Success!
</message> </message>
<!-- Sync app menu item strings -->
<if expr="not use_titlecase">
<message name="IDS_SYNC_MENU_PRE_SYNCED_LABEL" desc="The text for the sync app menu item before an account is synced.">
Sign in to <ph name="SHORT_PRODUCT_NAME">$1<ex>Chrome</ex></ph>...
</message>
<message name="IDS_SYNC_MENU_SYNCED_LABEL" desc="The text for the sync app menu item when bookmark sync has been enabled">
Signed in as <ph name="USER_NAME">$1<ex>pat@example.com</ex></ph>...
</message>
</if>
<if expr="use_titlecase">
<message name="IDS_SYNC_MENU_PRE_SYNCED_LABEL" desc="The text for the sync app menu item before an account is synced.">
Sign in to <ph name="SHORT_PRODUCT_NAME">$1<ex>Chrome</ex></ph>...
</message>
<message name="IDS_SYNC_MENU_SYNCED_LABEL" desc="The text for the sync app menu item when bookmark sync has been enabled">
Signed in as <ph name="USER_NAME">$1<ex>pat@example.com</ex></ph>...
</message>
</if>
<!-- Sync promo page chrome://signin --> <!-- Sync promo page chrome://signin -->
<message name="IDS_SYNC_PROMO_TAB_TITLE" desc="The title of the sync promo tab."> <message name="IDS_SYNC_PROMO_TAB_TITLE" desc="The title of the sync promo tab.">
Sign in Sign in
......
chrome/app/nibs/MainMenu.xib
View file @ d51574fc
...@@ -33,12 +33,6 @@ ...@@ -33,12 +33,6 @@
</connections> </connections>
</menuItem> </menuItem>
<menuItem isSeparatorItem="YES" id="492"/> <menuItem isSeparatorItem="YES" id="492"/>
<menuItem title="^IDS_SYNC_MENU_PRE_SYNCED_LABEL$IDS_PRODUCT_NAME" tag="40030" hidden="YES" enabled="NO" id="664">
<modifierMask key="keyEquivalentModifierMask"/>
<connections>
<action selector="commandDispatch:" target="-1" id="666"/>
</connections>
</menuItem>
<menuItem title="^IDS_CLEAR_BROWSING_DATA" tag="40013" id="491"> <menuItem title="^IDS_CLEAR_BROWSING_DATA" tag="40013" id="491">
<string key="keyEquivalent" base64-UTF8="YES"> <string key="keyEquivalent" base64-UTF8="YES">
CA CA
......
chrome/browser/app_controller_mac.h
View file @ d51574fc
...@@ -119,12 +119,6 @@ class WorkAreaWatcherObserver; ...@@ -119,12 +119,6 @@ class WorkAreaWatcherObserver;
@property(readonly, nonatomic) BOOL startupComplete; @property(readonly, nonatomic) BOOL startupComplete;
@property(readonly, nonatomic) Profile* lastProfile; @property(readonly, nonatomic) Profile* lastProfile;
// Helper method used to update the "Signin" menu item in the main menu and the
// wrench menu to reflect the current signed in state.
+ (void)updateSigninItem:(id)signinItem
shouldShow:(BOOL)showSigninMenuItem
currentProfile:(Profile*)profile;
- (void)didEndMainMessageLoop; - (void)didEndMainMessageLoop;
// Try to close all browser windows, and if that succeeds then quit. // Try to close all browser windows, and if that succeeds then quit.
......
chrome/browser/app_controller_mac.mm
View file @ d51574fc
...@@ -291,33 +291,6 @@ class AppControllerProfileObserver : public ProfileAttributesStorage::Observer { ...@@ -291,33 +291,6 @@ class AppControllerProfileObserver : public ProfileAttributesStorage::Observer {
@synthesize startupComplete = startupComplete_; @synthesize startupComplete = startupComplete_;
+ (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;
}
}
}
base::string16 label = signin_ui_util::GetSigninMenuLabel(profile);
[signinMenuItem setTitle:l10n_util::FixUpWindowsStyleLabel(label)];
[signinMenuItem setHidden:!showSigninMenuItem];
[followingSeparator setHidden:!showSigninMenuItem];
}
- (void)dealloc { - (void)dealloc {
[[closeTabMenuItem_ menu] setDelegate:nil]; [[closeTabMenuItem_ menu] setDelegate:nil];
[super dealloc]; [super dealloc];
...@@ -922,27 +895,6 @@ class AppControllerProfileObserver : public ProfileAttributesStorage::Observer { ...@@ -922,27 +895,6 @@ class AppControllerProfileObserver : public ProfileAttributesStorage::Observer {
// dialog. // dialog.
enable = ![self keyWindowIsModal]; enable = ![self keyWindowIsModal];
break; break;
case IDC_SHOW_SYNC_SETUP: {
Profile* lastProfile = [self lastProfile];
// The profile may be NULL during shutdown -- see
// http://code.google.com/p/chromium/issues/detail?id=43048 .
//
// TODO(akalin,viettrungluu): Figure out whether this method
// can be prevented from being called if lastProfile is
// NULL.
if (!lastProfile) {
LOG(WARNING)
<< "NULL lastProfile detected -- not doing anything";
break;
}
SigninManager* signin = SigninManagerFactory::GetForProfile(
lastProfile->GetOriginalProfile());
enable = signin->IsSigninAllowed() && ![self keyWindowIsModal];
[AppController updateSigninItem:item
shouldShow:enable
currentProfile:lastProfile];
break;
}
default: default:
enable = menuState_->IsCommandEnabled(tag) ? enable = menuState_->IsCommandEnabled(tag) ?
![self keyWindowIsModal] : NO; ![self keyWindowIsModal] : NO;
...@@ -1088,15 +1040,6 @@ class AppControllerProfileObserver : public ProfileAttributesStorage::Observer { ...@@ -1088,15 +1040,6 @@ class AppControllerProfileObserver : public ProfileAttributesStorage::Observer {
else else
chrome::OpenHelpWindow(lastProfile, chrome::HELP_SOURCE_MENU); chrome::OpenHelpWindow(lastProfile, chrome::HELP_SOURCE_MENU);
break; break;
case IDC_SHOW_SYNC_SETUP:
if (Browser* browser = ActivateBrowser(lastProfile)) {
chrome::ShowBrowserSigninOrSettings(
browser, signin_metrics::AccessPoint::ACCESS_POINT_MENU);
} else {
chrome::OpenSyncSetupWindow(
lastProfile, signin_metrics::AccessPoint::ACCESS_POINT_MENU);
}
break;
case IDC_TASK_MANAGER: case IDC_TASK_MANAGER:
chrome::OpenTaskManager(NULL); chrome::OpenTaskManager(NULL);
break; break;
...@@ -1249,7 +1192,6 @@ class AppControllerProfileObserver : public ProfileAttributesStorage::Observer { ...@@ -1249,7 +1192,6 @@ class AppControllerProfileObserver : public ProfileAttributesStorage::Observer {
#if defined(GOOGLE_CHROME_BUILD) #if defined(GOOGLE_CHROME_BUILD)
menuState_->UpdateCommandEnabled(IDC_FEEDBACK, true); menuState_->UpdateCommandEnabled(IDC_FEEDBACK, true);
#endif #endif
menuState_->UpdateCommandEnabled(IDC_SHOW_SYNC_SETUP, true);
menuState_->UpdateCommandEnabled(IDC_TASK_MANAGER, true); menuState_->UpdateCommandEnabled(IDC_TASK_MANAGER, true);
} }
......
chrome/browser/app_controller_mac_unittest.mm
View file @ d51574fc
...@@ -91,137 +91,3 @@ TEST_F(AppControllerTest, LastProfile) { ...@@ -91,137 +91,3 @@ TEST_F(AppControllerTest, LastProfile) {
EXPECT_EQ(dest_path2, [ac lastProfile]->GetPath()); EXPECT_EQ(dest_path2, [ac lastProfile]->GetPath());
} }
TEST_F(AppControllerTest, TestSigninMenuItemNoErrors) {
base::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.
[AppController updateSigninItem:syncMenuItem
shouldShow:YES
currentProfile:profile_];
EXPECT_TRUE([[syncMenuItem title] isEqualTo:startSignin]);
EXPECT_FALSE([syncMenuItem isHidden]);
[AppController 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, base::UTF8ToUTF16(username));
SigninManager* signin = SigninManagerFactory::GetForProfile(profile_);
signin->SetAuthenticatedAccountInfo(username, username);
browser_sync::ProfileSyncService* sync =
ProfileSyncServiceFactory::GetForProfile(profile_);
sync->SetFirstSetupComplete();
[AppController updateSigninItem:syncMenuItem
shouldShow:YES
currentProfile:profile_];
EXPECT_TRUE([[syncMenuItem title] isEqualTo:alreadySignedIn]);
EXPECT_FALSE([syncMenuItem isHidden]);
}
TEST_F(AppControllerTest, TestSigninMenuItemAuthError) {
base::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->SetAuthenticatedAccountInfo(username, username);
browser_sync::ProfileSyncService* sync =
ProfileSyncServiceFactory::GetForProfile(profile_);
sync->SetFirstSetupComplete();
// Force an auth error.
FakeAuthStatusProvider provider(
SigninErrorControllerFactory::GetForProfile(profile_));
GoogleServiceAuthError error(
GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS);
provider.SetAuthError("user@gmail.com", error);
[AppController updateSigninItem:syncMenuItem
shouldShow:YES
currentProfile:profile_];
NSString* authError =
l10n_util::GetNSStringWithFixup(IDS_SYNC_SIGN_IN_ERROR_WRENCH_MENU_ITEM);
EXPECT_TRUE([[syncMenuItem title] isEqualTo:authError]);
EXPECT_FALSE([syncMenuItem isHidden]);
}
// If there's a separator after the signin menu item, make sure it is hidden/
// shown when the signin menu item is.
TEST_F(AppControllerTest, TestSigninMenuItemWithSeparator) {
base::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];
[AppController updateSigninItem:signinMenuItem
shouldShow:NO
currentProfile:profile_];
EXPECT_FALSE([followingSeparator isEnabled]);
EXPECT_TRUE([signinMenuItem isHidden]);
EXPECT_TRUE([followingSeparator isHidden]);
[AppController 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(AppControllerTest, TestSigninMenuItemWithNonSeparator) {
base::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];
[AppController updateSigninItem:signinMenuItem
shouldShow:NO
currentProfile:profile_];
EXPECT_TRUE([followingNonSeparator isEnabled]);
EXPECT_TRUE([signinMenuItem isHidden]);
EXPECT_FALSE([followingNonSeparator isHidden]);
[followingNonSeparator setHidden:YES];
[AppController updateSigninItem:signinMenuItem
shouldShow:YES
currentProfile:profile_];
EXPECT_TRUE([followingNonSeparator isEnabled]);
EXPECT_FALSE([signinMenuItem isHidden]);
EXPECT_TRUE([followingNonSeparator isHidden]);
}
chrome/browser/signin/signin_ui_util.cc
View file @ d51574fc
...@@ -34,49 +34,6 @@ ...@@ -34,49 +34,6 @@
namespace signin_ui_util { namespace signin_ui_util {
namespace {
// Maximum width of a username - we trim emails that are wider than this so
// the wrench menu doesn't get ridiculously wide.
const int kUsernameMaxWidth = 200;
// Returns all errors reported by signed in services.
std::vector<GlobalError*> GetSignedInServiceErrors(Profile* profile) {
std::vector<GlobalError*> errors;
// Chrome OS doesn't use SigninGlobalError or SyncGlobalError. Other platforms
// may use these services to show auth and sync errors in the toolbar menu.
#if !defined(OS_CHROMEOS)
// Auth errors have the highest priority - after that, individual service
// errors.
SigninGlobalError* signin_error =
SigninGlobalErrorFactory::GetForProfile(profile);
if (signin_error && signin_error->HasError())
errors.push_back(signin_error);
// No auth error - now try other services. Currently the list is just hard-
// coded but in the future if we add more we can create some kind of
// registration framework.
if (profile->IsSyncAllowed()) {
SyncGlobalError* error = SyncGlobalErrorFactory::GetForProfile(profile);
if (error && error->HasMenuItem())
errors.push_back(error);
}
#endif
return errors;
}
// If a signed in service is reporting an error, returns the GlobalError
// object associated with that service, or null if no errors are reported.
GlobalError* GetSignedInServiceError(Profile* profile) {
std::vector<GlobalError*> errors = GetSignedInServiceErrors(profile);
if (errors.empty())
return nullptr;
return errors[0];
}
} // namespace
base::string16 GetAuthenticatedUsername(const SigninManagerBase* signin) { base::string16 GetAuthenticatedUsername(const SigninManagerBase* signin) {
std::string user_display_name; std::string user_display_name;
...@@ -96,35 +53,6 @@ base::string16 GetAuthenticatedUsername(const SigninManagerBase* signin) { ...@@ -96,35 +53,6 @@ base::string16 GetAuthenticatedUsername(const SigninManagerBase* signin) {
return base::UTF8ToUTF16(user_display_name); return base::UTF8ToUTF16(user_display_name);
} }
base::string16 GetSigninMenuLabel(Profile* profile) {
GlobalError* error = signin_ui_util::GetSignedInServiceError(profile);
if (error)
return error->MenuItemLabel();
// No errors, so just display the signed in user, if any.
browser_sync::ProfileSyncService* service =
profile->IsSyncAllowed()
? ProfileSyncServiceFactory::GetForProfile(profile)
: nullptr;
// Even if the user is signed in, don't display the "signed in as..."
// label if we're still setting up sync.
if (!service || !service->IsFirstSetupInProgress()) {
std::string username;
SigninManagerBase* signin_manager =
SigninManagerFactory::GetForProfileIfExists(profile);
if (signin_manager)
username = signin_manager->GetAuthenticatedAccountInfo().email;
if (!username.empty() && !signin_manager->AuthInProgress()) {
const base::string16 elided = gfx::ElideText(base::UTF8ToUTF16(username),
gfx::FontList(), kUsernameMaxWidth, gfx::ELIDE_EMAIL);
return l10n_util::GetStringFUTF16(IDS_SYNC_MENU_SYNCED_LABEL, elided);
}
}
return l10n_util::GetStringFUTF16(IDS_SYNC_MENU_PRE_SYNCED_LABEL,
l10n_util::GetStringUTF16(IDS_SHORT_PRODUCT_NAME));
}
// Given an authentication state this helper function returns various labels // Given an authentication state this helper function returns various labels
// that can be used to display information about the state. // that can be used to display information about the state.
void GetStatusLabelsForAuthError(Profile* profile, void GetStatusLabelsForAuthError(Profile* profile,
......
chrome/browser/signin/signin_ui_util.h
View file @ d51574fc
...@@ -20,10 +20,6 @@ namespace signin_ui_util { ...@@ -20,10 +20,6 @@ namespace signin_ui_util {
// The maximum number of times to show the welcome tutorial for an upgrade user. // The maximum number of times to show the welcome tutorial for an upgrade user.
const int kUpgradeWelcomeTutorialShowMax = 1; const int kUpgradeWelcomeTutorialShowMax = 1;
// Returns the label that should be displayed in the signin menu (i.e.
// "Sign in to Chromium", "Signin Error...", etc).
base::string16 GetSigninMenuLabel(Profile* profile);
void GetStatusLabelsForAuthError(Profile* profile, void GetStatusLabelsForAuthError(Profile* profile,
const SigninManagerBase& signin_manager, const SigninManagerBase& signin_manager,
base::string16* status_label, base::string16* status_label,
......
chrome/browser/ui/cocoa/browser_window_command_handler.mm
View file @ d51574fc
...@@ -133,15 +133,6 @@ Browser* FindBrowserForSender(id sender, NSWindow* window) { ...@@ -133,15 +133,6 @@ Browser* FindBrowserForSender(id sender, NSWindow* window) {
[menuItem setTitle:GetTitleForFullscreenMenuItem(browser)]; [menuItem setTitle:GetTitleForFullscreenMenuItem(browser)];
break; break;
} }
case IDC_SHOW_SIGNIN: {
Profile* original_profile = browser->profile()->GetOriginalProfile();
[AppController updateSigninItem:item
shouldShow:enable
currentProfile:original_profile];
content::RecordAction(
base::UserMetricsAction("Signin_Impression_FromMenu"));
break;
}
case IDC_BOOKMARK_PAGE: { case IDC_BOOKMARK_PAGE: {
// Extensions have the ability to hide the bookmark page menu item. // Extensions have the ability to hide the bookmark page menu item.
// This only affects the bookmark page menu item under the main menu. // This only affects the bookmark page menu item under the main menu.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment