diff options
31 files changed, 518 insertions, 309 deletions
diff --git a/build/common.gypi b/build/common.gypi index 0a6f41c..058a45e 100644 --- a/build/common.gypi +++ b/build/common.gypi @@ -434,6 +434,10 @@ 'webui_task_manager%': 1, }], + ['OS=="win" or OS=="mac" or (OS=="linux" and use_aura==0)', { + 'enable_one_click_signin%': 1, + }], + ['OS=="android"', { 'proprietary_codecs%': 1, 'enable_webrtc%': 0, diff --git a/chrome/app/chromium_strings.grd b/chrome/app/chromium_strings.grd index db47f71..0445d96 100644 --- a/chrome/app/chromium_strings.grd +++ b/chrome/app/chromium_strings.grd @@ -784,6 +784,9 @@ The extension "<ph name="EXTENSION_NAME">$1<ex>Gmail Checker</ex></ph>" has been <message name="IDS_ONE_CLICK_SIGNIN_INFOBAR_MESSAGE" desc="The string shown in the infobar explaining that the user can connect his profile to a Google account instead of logging in only here."> Use this Google Account to sync all your Chromium stuff? </message> + <message name="IDS_ONE_CLICK_SIGNIN_BUBBLE_MESSAGE" desc="The body of the sync promo NTP bubble."> + You're now signed in to Chromium! Your bookmarks, history, and other settings will be synced to your Google Account. + </message> <message name="IDS_ONE_CLICK_SIGNIN_DIALOG_HEADING" desc="The heading of the reverse auto-login dialog."> Sign in to Chromium </message> diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index b961958..47a6c2b 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -9390,6 +9390,9 @@ experiment id: "<ph name="EXPERIMENT_ID">$5<ex>ar1</ex></ph>" <message name="IDS_ONE_CLICK_SIGNIN_DIALOG_TITLE" desc="The title of the modal dialog window that opens when the user chooses to use one click sign in."> Sign in </message> + <message name="IDS_ONE_CLICK_BUBBLE_UNDO" desc="The text for the undo button in the one click signin bubble."> + Undo + </message> <message name="IDS_ONE_CLICK_SIGNIN_DIALOG_MESSAGE" desc="The message of the one click sign in dialog."> Signing in will sync your data with your Google Account so you can access your bookmarks, history, and other settings on any device. </message> diff --git a/chrome/app/google_chrome_strings.grd b/chrome/app/google_chrome_strings.grd index baf8661..4a57843 100644 --- a/chrome/app/google_chrome_strings.grd +++ b/chrome/app/google_chrome_strings.grd @@ -709,6 +709,9 @@ The extension "<ph name="EXTENSION_NAME">$1<ex>Gmail Checker</ex></ph>" has been <message name="IDS_ONE_CLICK_SIGNIN_INFOBAR_MESSAGE" desc="The string shown in the infobar explaining that the user can connect his profile to a Google account instead of logging in only here."> Use this Google Account to sync all your Chrome stuff? </message> + <message name="IDS_ONE_CLICK_SIGNIN_BUBBLE_MESSAGE" desc="The body of the sync promo NTP bubble."> + You're now signed in to Chrome! Your bookmarks, history, and other settings will be synced to your Google Account. + </message> <message name="IDS_ONE_CLICK_SIGNIN_DIALOG_HEADING" desc="The heading of the reverse auto-login dialog."> Sign in to Chrome </message> diff --git a/chrome/browser/ui/browser_window.h b/chrome/browser/ui/browser_window.h index 508249a..0753c33 100644 --- a/chrome/browser/ui/browser_window.h +++ b/chrome/browser/ui/browser_window.h @@ -11,6 +11,7 @@ #include "chrome/browser/ui/base_window.h" #include "chrome/browser/ui/bookmarks/bookmark_bar.h" #include "chrome/browser/ui/fullscreen_exit_bubble_type.h" +#include "chrome/browser/ui/sync/one_click_signin_sync_starter.h" #include "chrome/common/content_settings_types.h" #include "ui/gfx/native_widget_types.h" #include "webkit/glue/window_open_disposition.h" @@ -221,11 +222,15 @@ class BrowserWindow : public BaseWindow { virtual void ShowChromeToMobileBubble() = 0; #if defined(ENABLE_ONE_CLICK_SIGNIN) - // Shows the one-click sign in bubble. The given closures are run - // when their corresponding links are clicked. + // Callback type used with the ShowOneClickSigninBubble() method. If the + // user chooses to accept the sign in, the callback is called to start the + // sync process. + typedef base::Callback<void(OneClickSigninSyncStarter::StartSyncMode)> + StartSyncCallback; + + // Shows the one-click sign in bubble. virtual void ShowOneClickSigninBubble( - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback) = 0; + const StartSyncCallback& start_sync_callback) = 0; #endif // Whether or not the shelf view is visible. diff --git a/chrome/browser/ui/cocoa/browser_window_cocoa.h b/chrome/browser/ui/cocoa/browser_window_cocoa.h index d7640f4..c4398b2 100644 --- a/chrome/browser/ui/cocoa/browser_window_cocoa.h +++ b/chrome/browser/ui/cocoa/browser_window_cocoa.h @@ -99,8 +99,7 @@ class BrowserWindowCocoa : public BrowserWindow, virtual void ShowChromeToMobileBubble() OVERRIDE; #if defined(ENABLE_ONE_CLICK_SIGNIN) virtual void ShowOneClickSigninBubble( - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback) OVERRIDE; + const StartSyncCallback& start_sync_callback) OVERRIDE; #endif virtual bool IsDownloadShelfVisible() const OVERRIDE; virtual DownloadShelf* GetDownloadShelf() OVERRIDE; diff --git a/chrome/browser/ui/cocoa/browser_window_cocoa.mm b/chrome/browser/ui/cocoa/browser_window_cocoa.mm index 344aadc..ec84653 100644 --- a/chrome/browser/ui/cocoa/browser_window_cocoa.mm +++ b/chrome/browser/ui/cocoa/browser_window_cocoa.mm @@ -452,15 +452,12 @@ void BrowserWindowCocoa::ShowChromeToMobileBubble() { } #if defined(ENABLE_ONE_CLICK_SIGNIN) - void BrowserWindowCocoa::ShowOneClickSigninBubble( - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback) { + const StartSyncCallback& start_sync_callback) { OneClickSigninBubbleController* bubble_controller = [[OneClickSigninBubbleController alloc] initWithBrowserWindowController:cocoa_controller() - learnMoreCallback:learn_more_callback - advancedCallback:advanced_callback]; + start_sync_callback:start_sync_callback]; [bubble_controller showWindow:nil]; } #endif diff --git a/chrome/browser/ui/cocoa/one_click_signin_bubble_controller.h b/chrome/browser/ui/cocoa/one_click_signin_bubble_controller.h index 0a0261b..c3f2f11 100644 --- a/chrome/browser/ui/cocoa/one_click_signin_bubble_controller.h +++ b/chrome/browser/ui/cocoa/one_click_signin_bubble_controller.h @@ -9,6 +9,7 @@ #import <Cocoa/Cocoa.h> #include "base/callback.h" +#include "chrome/browser/ui/browser_window.h" #import "chrome/browser/ui/cocoa/base_bubble_controller.h" @class BrowserWindowController; @@ -18,11 +19,14 @@ @interface OneClickSigninBubbleController : BaseBubbleController { @private IBOutlet NSTextField* messageField_; + // TODO(akalin): learnMoreLink_ needs to be removed, but it can't be until + // the nib is changed too. IBOutlet NSButton* learnMoreLink_; IBOutlet NSButton* advancedLink_; - base::Closure learnMoreCallback_; - base::Closure advancedCallback_; + // TODO(akalin): Make sure this callback is called only once, like on + // other platforms. + BrowserWindow::StartSyncCallback start_sync_callback_; } // Initializes with a browser window controller, under whose wrench @@ -32,15 +36,13 @@ // The bubble is not automatically displayed; call showWindow:id to // display. The bubble is auto-released on close. - (id)initWithBrowserWindowController:(BrowserWindowController*)controller - learnMoreCallback:(const base::Closure&)learnMoreCallback - advancedCallback:(const base::Closure&)advancedCallback; + start_sync_callback: + (const BrowserWindow::StartSyncCallback&) + start_sync_callback; -// Just closes the bubble. +// Starts sync and closes the bubble. - (IBAction)ok:(id)sender; -// Calls |learnMoreCallback_|. -- (IBAction)onClickLearnMoreLink:(id)sender; - // Calls |advancedCallback_|. - (IBAction)onClickAdvancedLink:(id)sender; diff --git a/chrome/browser/ui/cocoa/one_click_signin_bubble_controller.mm b/chrome/browser/ui/cocoa/one_click_signin_bubble_controller.mm index d5c0dd7..273f56f 100644 --- a/chrome/browser/ui/cocoa/one_click_signin_bubble_controller.mm +++ b/chrome/browser/ui/cocoa/one_click_signin_bubble_controller.mm @@ -29,8 +29,8 @@ void ShiftOriginY(NSView* view, CGFloat amount) { @implementation OneClickSigninBubbleController - (id)initWithBrowserWindowController:(BrowserWindowController*)controller - learnMoreCallback:(const base::Closure&)learnMoreCallback - advancedCallback:(const base::Closure&)advancedCallback { + start_sync_callback:(const BrowserWindow::StartSyncCallback&) + start_sync_callback { NSWindow* parentWindow = [controller window]; // Set the anchor point to right below the wrench menu. @@ -43,26 +43,32 @@ void ShiftOriginY(NSView* view, CGFloat amount) { if (self = [super initWithWindowNibPath:@"OneClickSigninBubble" parentWindow:parentWindow anchoredAt:anchorPoint]) { - learnMoreCallback_ = learnMoreCallback; - advancedCallback_ = advancedCallback; - DCHECK(!learnMoreCallback_.is_null()); - DCHECK(!advancedCallback_.is_null()); + start_sync_callback_ = start_sync_callback; + DCHECK(!start_sync_callback_.is_null()); } return self; } - (IBAction)ok:(id)sender { + start_sync_callback_.Run( + OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS); [self close]; } - (IBAction)onClickLearnMoreLink:(id)sender { - learnMoreCallback_.Run(); + // TODO(akalin): this method should be removed once the UI elements are + // removed. } - (IBAction)onClickAdvancedLink:(id)sender { - advancedCallback_.Run(); + start_sync_callback_.Run( + OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST); + [self close]; } +// TODO(rogerta): if the bubble is closed without interaction, need to call +// the callback with argument set to SYNC_WITH_DEFAULT_SETTINGS. + - (void)awakeFromNib { [super awakeFromNib]; diff --git a/chrome/browser/ui/cocoa/one_click_signin_bubble_controller_unittest.mm b/chrome/browser/ui/cocoa/one_click_signin_bubble_controller_unittest.mm index 93f7ed1..9a23523 100644 --- a/chrome/browser/ui/cocoa/one_click_signin_bubble_controller_unittest.mm +++ b/chrome/browser/ui/cocoa/one_click_signin_bubble_controller_unittest.mm @@ -12,6 +12,7 @@ #include "base/memory/weak_ptr.h" #import "chrome/browser/ui/cocoa/browser_window_cocoa.h" #include "chrome/browser/ui/cocoa/cocoa_profile_test.h" +#include "chrome/browser/ui/sync/one_click_signin_sync_starter.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #import "testing/gtest_mac.h" @@ -22,11 +23,8 @@ class OneClickSigninBubbleControllerTest : public CocoaProfileTest { public: OneClickSigninBubbleControllerTest() : weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)), - learn_more_callback_( - base::Bind(&OneClickSigninBubbleControllerTest::OnLearnMore, - weak_ptr_factory_.GetWeakPtr())), - advanced_callback_( - base::Bind(&OneClickSigninBubbleControllerTest::OnAdvanced, + start_sync_callback_( + base::Bind(&OneClickSigninBubbleControllerTest::OnStartSync, weak_ptr_factory_.GetWeakPtr())) {} virtual void SetUp() { @@ -36,8 +34,7 @@ class OneClickSigninBubbleControllerTest : public CocoaProfileTest { controller_.reset( [[OneClickSigninBubbleController alloc] initWithBrowserWindowController:browser_window->cocoa_controller() - learnMoreCallback:learn_more_callback_ - advancedCallback:advanced_callback_]); + start_sync_callback:start_sync_callback_]); } virtual void TearDown() { @@ -45,70 +42,43 @@ class OneClickSigninBubbleControllerTest : public CocoaProfileTest { CocoaProfileTest::TearDown(); } - MOCK_METHOD0(OnLearnMore, void()); - MOCK_METHOD0(OnAdvanced, void()); + MOCK_METHOD1(OnStartSync, void(OneClickSigninSyncStarter::StartSyncMode)); protected: base::WeakPtrFactory<OneClickSigninBubbleControllerTest> weak_ptr_factory_; - base::Closure learn_more_callback_; - base::Closure advanced_callback_; + BrowserWindow::StartSyncCallback start_sync_callback_; scoped_nsobject<OneClickSigninBubbleController> controller_; }; // Test that the dialog loads from its nib properly. TEST_F(OneClickSigninBubbleControllerTest, NibLoad) { - EXPECT_CALL(*this, OnLearnMore()).Times(0); - EXPECT_CALL(*this, OnAdvanced()).Times(0); + EXPECT_CALL(*this, OnStartSync(testing::_)).Times(0); // Force nib load. [controller_ window]; EXPECT_NSEQ(@"OneClickSigninBubble", [controller_ windowNibName]); } -// Test that the dialog doesn't call any callback if the OK button is -// clicked. +// Test that the dialog calls the callback if the OK button is clicked. TEST_F(OneClickSigninBubbleControllerTest, ShowAndOK) { - EXPECT_CALL(*this, OnLearnMore()).Times(0); - EXPECT_CALL(*this, OnAdvanced()).Times(0); + EXPECT_CALL(*this, OnStartSync( + OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS)).Times(1); [controller_ showWindow:nil]; [controller_.release() ok:nil]; } -// Test that the learn more callback is run if its corresponding +// TODO(akalin): Test that the dialog doesn't call the callback if the Undo // button is clicked. -TEST_F(OneClickSigninBubbleControllerTest, ShowAndClickLearnMore) { - EXPECT_CALL(*this, OnLearnMore()).Times(1); - EXPECT_CALL(*this, OnAdvanced()).Times(0); - - [controller_ showWindow:nil]; - [controller_ onClickLearnMoreLink:nil]; - [controller_.release() ok:nil]; -} // Test that the advanced callback is run if its corresponding button // is clicked. TEST_F(OneClickSigninBubbleControllerTest, ShowAndClickAdvanced) { - EXPECT_CALL(*this, OnLearnMore()).Times(0); - EXPECT_CALL(*this, OnAdvanced()).Times(1); + EXPECT_CALL(*this, OnStartSync( + OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST)).Times(1); [controller_ showWindow:nil]; - [controller_ onClickAdvancedLink:nil]; - [controller_.release() ok:nil]; -} - -// Test that the callbacks can be run multiple times. -TEST_F(OneClickSigninBubbleControllerTest, ShowAndClickMultiple) { - EXPECT_CALL(*this, OnLearnMore()).Times(3); - EXPECT_CALL(*this, OnAdvanced()).Times(4); - - [controller_ showWindow:nil]; - for (int i = 0; i < 3; ++i) { - [controller_ onClickLearnMoreLink:nil]; - [controller_ onClickAdvancedLink:nil]; - } - [controller_ onClickAdvancedLink:nil]; - [controller_.release() ok:nil]; + [controller_.release() onClickAdvancedLink:nil]; } } // namespace diff --git a/chrome/browser/ui/gtk/browser_window_gtk.cc b/chrome/browser/ui/gtk/browser_window_gtk.cc index bfa8fe8..1bc976a 100644 --- a/chrome/browser/ui/gtk/browser_window_gtk.cc +++ b/chrome/browser/ui/gtk/browser_window_gtk.cc @@ -1090,10 +1090,8 @@ void BrowserWindowGtk::ShowChromeToMobileBubble() { #if defined(ENABLE_ONE_CLICK_SIGNIN) void BrowserWindowGtk::ShowOneClickSigninBubble( - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback) { - ignore_result(new OneClickSigninBubbleGtk(this, learn_more_callback, - advanced_callback)); + const StartSyncCallback& start_sync_callback) { + new OneClickSigninBubbleGtk(this, start_sync_callback); } #endif diff --git a/chrome/browser/ui/gtk/browser_window_gtk.h b/chrome/browser/ui/gtk/browser_window_gtk.h index 5924cbe..13e52a9 100644 --- a/chrome/browser/ui/gtk/browser_window_gtk.h +++ b/chrome/browser/ui/gtk/browser_window_gtk.h @@ -129,8 +129,7 @@ class BrowserWindowGtk : public BrowserWindow, virtual void ShowChromeToMobileBubble() OVERRIDE; #if defined(ENABLE_ONE_CLICK_SIGNIN) virtual void ShowOneClickSigninBubble( - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback) OVERRIDE; + const StartSyncCallback& start_sync_callback) OVERRIDE; #endif virtual bool IsDownloadShelfVisible() const OVERRIDE; virtual DownloadShelf* GetDownloadShelf() OVERRIDE; diff --git a/chrome/browser/ui/gtk/one_click_signin_bubble_gtk.cc b/chrome/browser/ui/gtk/one_click_signin_bubble_gtk.cc index 74d8dae..8ed301d 100644 --- a/chrome/browser/ui/gtk/one_click_signin_bubble_gtk.cc +++ b/chrome/browser/ui/gtk/one_click_signin_bubble_gtk.cc @@ -6,6 +6,7 @@ #include <gtk/gtk.h> +#include "base/callback_helpers.h" #include "base/i18n/rtl.h" #include "base/logging.h" #include "chrome/browser/ui/browser.h" @@ -26,11 +27,9 @@ const int kContentBorder = 7; OneClickSigninBubbleGtk::OneClickSigninBubbleGtk( BrowserWindowGtk* browser_window_gtk, - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback) + const BrowserWindow::StartSyncCallback& start_sync_callback) : bubble_(NULL), - learn_more_callback_(learn_more_callback), - advanced_callback_(advanced_callback) { + start_sync_callback_(start_sync_callback) { // Setup the BubbleGtk content. GtkWidget* bubble_content = gtk_vbox_new(FALSE, 0); @@ -49,18 +48,6 @@ OneClickSigninBubbleGtk::OneClickSigninBubbleGtk( GtkThemeService* const theme_provider = GtkThemeService::GetFrom( browser_window_gtk->browser()->profile()); - GtkWidget* learn_more_line = gtk_hbox_new(FALSE, kContentBorder); - gtk_box_pack_start(GTK_BOX(bubble_content), - learn_more_line, FALSE, FALSE, 0); - - // Learn more link. - GtkWidget* learn_more_link = theme_provider->BuildChromeLinkButton( - l10n_util::GetStringUTF8(IDS_SYNC_PROMO_NTP_BUBBLE_LEARN_MORE)); - g_signal_connect(learn_more_link, "clicked", - G_CALLBACK(OnClickLearnMoreLinkThunk), this); - gtk_box_pack_start(GTK_BOX(learn_more_line), - learn_more_link, FALSE, FALSE, 0); - GtkWidget* bottom_line = gtk_hbox_new(FALSE, kContentBorder); gtk_box_pack_start(GTK_BOX(bubble_content), bottom_line, FALSE, FALSE, 0); @@ -73,14 +60,29 @@ OneClickSigninBubbleGtk::OneClickSigninBubbleGtk( gtk_box_pack_start(GTK_BOX(bottom_line), advanced_link, FALSE, FALSE, 0); + // Make the OK and Undo buttons the same size horizontally. + GtkSizeGroup* size_group = gtk_size_group_new(GTK_SIZE_GROUP_HORIZONTAL); + // OK Button. GtkWidget* ok_button = gtk_button_new_with_label( l10n_util::GetStringUTF8(IDS_OK).c_str()); g_signal_connect(ok_button, "clicked", G_CALLBACK(OnClickOKThunk), this); + gtk_size_group_add_widget(size_group, ok_button); gtk_box_pack_end(GTK_BOX(bottom_line), ok_button, FALSE, FALSE, 0); + // Undo Button. + GtkWidget* undo_button = gtk_button_new_with_label( + l10n_util::GetStringUTF8(IDS_ONE_CLICK_BUBBLE_UNDO).c_str()); + g_signal_connect(undo_button, "clicked", + G_CALLBACK(OnClickUndoThunk), this); + gtk_size_group_add_widget(size_group, undo_button); + gtk_box_pack_end(GTK_BOX(bottom_line), + undo_button, FALSE, FALSE, 0); + + g_object_unref(size_group); + GtkWidget* const app_menu_widget = browser_window_gtk->GetToolbar()->GetAppMenuButton(); gfx::Rect bounds = gtk_util::WidgetBounds(app_menu_widget); @@ -98,33 +100,29 @@ OneClickSigninBubbleGtk::OneClickSigninBubbleGtk( void OneClickSigninBubbleGtk::BubbleClosing( BubbleGtk* bubble, bool closed_by_escape) { - delete this; -} + if (!start_sync_callback_.is_null()) + base::ResetAndReturn(&start_sync_callback_).Run( + OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS); -void OneClickSigninBubbleGtk::ClickOKForTest() { - OnClickOK(NULL); -} - -void OneClickSigninBubbleGtk::ClickLearnMoreForTest() { - OnClickLearnMoreLink(NULL); -} - -void OneClickSigninBubbleGtk::ClickAdvancedForTest() { - OnClickAdvancedLink(NULL); + delete this; } -void OneClickSigninBubbleGtk::OnClickLearnMoreLink(GtkWidget* link) { - learn_more_callback_.Run(); +void OneClickSigninBubbleGtk::OnClickAdvancedLink(GtkWidget* link) { + base::ResetAndReturn(&start_sync_callback_).Run( + OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST); bubble_->Close(); } -void OneClickSigninBubbleGtk::OnClickAdvancedLink(GtkWidget* link) { - advanced_callback_.Run(); +void OneClickSigninBubbleGtk::OnClickOK(GtkWidget* link) { + base::ResetAndReturn(&start_sync_callback_).Run( + OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS); bubble_->Close(); } -void OneClickSigninBubbleGtk::OnClickOK(GtkWidget* link) { +void OneClickSigninBubbleGtk::OnClickUndo(GtkWidget* link) { + start_sync_callback_.Reset(); bubble_->Close(); } -OneClickSigninBubbleGtk::~OneClickSigninBubbleGtk() {} +OneClickSigninBubbleGtk::~OneClickSigninBubbleGtk() { +} diff --git a/chrome/browser/ui/gtk/one_click_signin_bubble_gtk.h b/chrome/browser/ui/gtk/one_click_signin_bubble_gtk.h index 12fed10..f2ac5ad 100644 --- a/chrome/browser/ui/gtk/one_click_signin_bubble_gtk.h +++ b/chrome/browser/ui/gtk/one_click_signin_bubble_gtk.h @@ -9,6 +9,9 @@ #include "base/basictypes.h" #include "base/callback.h" #include "base/compiler_specific.h" +#include "base/gtest_prod_util.h" +#include "chrome/browser/ui/base_window.h" +#include "chrome/browser/ui/browser_window.h" #include "chrome/browser/ui/gtk/bubble/bubble_gtk.h" typedef struct _GtkWidget GtkWidget; @@ -16,34 +19,38 @@ typedef struct _GtkWindow GtkWindow; class BrowserWindowGtk; -// Displays the one-click signin confirmation bubble (after syncing +// Displays the one-click signin confirmation bubble (before syncing // has started). class OneClickSigninBubbleGtk : public BubbleDelegateGtk { public: - // Deletes self on close. The given callbacks will be called if the - // user clicks the corresponding link. - OneClickSigninBubbleGtk(BrowserWindowGtk* browser_window_gtk, - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback); + // Deletes self on close. The given callback will be called if the + // user decides to start sync. + OneClickSigninBubbleGtk( + BrowserWindowGtk* browser_window_gtk, + const BrowserWindow::StartSyncCallback& start_sync_callback); // BubbleDelegateGtk implementation. virtual void BubbleClosing( BubbleGtk* bubble, bool closed_by_escape) OVERRIDE; - void ClickOKForTest(); - void ClickLearnMoreForTest(); - void ClickAdvancedForTest(); - private: + FRIEND_TEST_ALL_PREFIXES(OneClickSigninBubbleGtkTest, ShowAndOK); + FRIEND_TEST_ALL_PREFIXES(OneClickSigninBubbleGtkTest, ShowAndUndo); + FRIEND_TEST_ALL_PREFIXES(OneClickSigninBubbleGtkTest, ShowAndClickAdvanced); + FRIEND_TEST_ALL_PREFIXES(OneClickSigninBubbleGtkTest, ShowAndClose); + virtual ~OneClickSigninBubbleGtk(); - CHROMEGTK_CALLBACK_0(OneClickSigninBubbleGtk, void, OnClickLearnMoreLink); CHROMEGTK_CALLBACK_0(OneClickSigninBubbleGtk, void, OnClickAdvancedLink); CHROMEGTK_CALLBACK_0(OneClickSigninBubbleGtk, void, OnClickOK); + CHROMEGTK_CALLBACK_0(OneClickSigninBubbleGtk, void, OnClickUndo); BubbleGtk* bubble_; - const base::Closure learn_more_callback_; - const base::Closure advanced_callback_; + + // This callback is nulled once its called, so that it is called only once. + // It will be called when the bubble is closed if it has not been called + // and nulled earlier. + BrowserWindow::StartSyncCallback start_sync_callback_; DISALLOW_COPY_AND_ASSIGN(OneClickSigninBubbleGtk); }; diff --git a/chrome/browser/ui/gtk/one_click_signin_bubble_gtk_browsertest.cc b/chrome/browser/ui/gtk/one_click_signin_bubble_gtk_browsertest.cc index d0b9934..ddbe143 100644 --- a/chrome/browser/ui/gtk/one_click_signin_bubble_gtk_browsertest.cc +++ b/chrome/browser/ui/gtk/one_click_signin_bubble_gtk_browsertest.cc @@ -11,67 +11,67 @@ #include "base/memory/weak_ptr.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/gtk/browser_window_gtk.h" +#include "chrome/browser/ui/sync/one_click_signin_sync_starter.h" #include "chrome/test/base/in_process_browser_test.h" #include "testing/gmock/include/gmock/gmock.h" -namespace { - class OneClickSigninBubbleGtkTest : public InProcessBrowserTest { public: OneClickSigninBubbleGtkTest() : weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)), - learn_more_callback_( - base::Bind(&OneClickSigninBubbleGtkTest::OnLearnMore, - weak_ptr_factory_.GetWeakPtr())), - advanced_callback_( - base::Bind(&OneClickSigninBubbleGtkTest::OnAdvanced, + start_sync_callback_( + base::Bind(&OneClickSigninBubbleGtkTest::OnStartSync, weak_ptr_factory_.GetWeakPtr())), bubble_(NULL) {} virtual OneClickSigninBubbleGtk* MakeBubble() { return new OneClickSigninBubbleGtk( static_cast<BrowserWindowGtk*>(browser()->window()), - learn_more_callback_, - advanced_callback_); + start_sync_callback_); } - MOCK_METHOD0(OnLearnMore, void()); - MOCK_METHOD0(OnAdvanced, void()); + MOCK_METHOD1(OnStartSync, void(OneClickSigninSyncStarter::StartSyncMode)); protected: base::WeakPtrFactory<OneClickSigninBubbleGtkTest> weak_ptr_factory_; - base::Closure learn_more_callback_; - base::Closure advanced_callback_; + BrowserWindow::StartSyncCallback start_sync_callback_; // Owns itself. OneClickSigninBubbleGtk* bubble_; }; -// Test that the dialog doesn't call any callback if the OK button is -// clicked. +// Test that the dialog calls the callback if the OK button is clicked. +// Callback should be called to setup sync with default settings. IN_PROC_BROWSER_TEST_F(OneClickSigninBubbleGtkTest, ShowAndOK) { - EXPECT_CALL(*this, OnLearnMore()).Times(0); - EXPECT_CALL(*this, OnAdvanced()).Times(0); + EXPECT_CALL(*this, OnStartSync( + OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS)).Times(1); - MakeBubble()->ClickOKForTest(); + MakeBubble()->OnClickOK(NULL); } -// Test that the learn more callback is run if its corresponding -// button is clicked. -IN_PROC_BROWSER_TEST_F(OneClickSigninBubbleGtkTest, ShowAndClickLearnMore) { - EXPECT_CALL(*this, OnLearnMore()).Times(1); - EXPECT_CALL(*this, OnAdvanced()).Times(0); +// Test that the dialog doesn't call the callback if the Undo button is +// clicked. +IN_PROC_BROWSER_TEST_F(OneClickSigninBubbleGtkTest, ShowAndUndo) { + EXPECT_CALL(*this, OnStartSync(testing::_)).Times(0); - MakeBubble()->ClickLearnMoreForTest(); + MakeBubble()->OnClickUndo(NULL); } -// Test that the advanced callback is run if its corresponding button -// is clicked. +// Test that the dialog calls the callback if the advanced link is clicked. +// Callback should be called to configure sync before starting. IN_PROC_BROWSER_TEST_F(OneClickSigninBubbleGtkTest, ShowAndClickAdvanced) { - EXPECT_CALL(*this, OnLearnMore()).Times(0); - EXPECT_CALL(*this, OnAdvanced()).Times(1); + EXPECT_CALL(*this, + OnStartSync(OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST)). + Times(1); - MakeBubble()->ClickAdvancedForTest(); + MakeBubble()->OnClickAdvancedLink(NULL); } -} // namespace +// Test that the dialog calls the callback if the bubble is closed. +// Callback should be called to setup sync with default settings. +IN_PROC_BROWSER_TEST_F(OneClickSigninBubbleGtkTest, ShowAndClose) { + EXPECT_CALL(*this, OnStartSync( + OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS)).Times(1); + + MakeBubble()->bubble_->Close(); +} diff --git a/chrome/browser/ui/panels/panel_browser_window.cc b/chrome/browser/ui/panels/panel_browser_window.cc index 57f8b20..7404d23 100644 --- a/chrome/browser/ui/panels/panel_browser_window.cc +++ b/chrome/browser/ui/panels/panel_browser_window.cc @@ -295,8 +295,7 @@ void PanelBrowserWindow::ShowChromeToMobileBubble() { #if defined(ENABLE_ONE_CLICK_SIGNIN) void PanelBrowserWindow::ShowOneClickSigninBubble( - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback) { + const StartSyncCallback& start_sync_callback) { NOTIMPLEMENTED(); } #endif diff --git a/chrome/browser/ui/panels/panel_browser_window.h b/chrome/browser/ui/panels/panel_browser_window.h index 2e0393f..fe973ef 100644 --- a/chrome/browser/ui/panels/panel_browser_window.h +++ b/chrome/browser/ui/panels/panel_browser_window.h @@ -97,8 +97,7 @@ class PanelBrowserWindow : public BrowserWindow, virtual void ShowChromeToMobileBubble() OVERRIDE; #if defined(ENABLE_ONE_CLICK_SIGNIN) virtual void ShowOneClickSigninBubble( - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback) OVERRIDE; + const StartSyncCallback& start_sync_callback) OVERRIDE; #endif virtual bool IsDownloadShelfVisible() const OVERRIDE; virtual DownloadShelf* GetDownloadShelf() OVERRIDE; diff --git a/chrome/browser/ui/sync/one_click_signin_helper.cc b/chrome/browser/ui/sync/one_click_signin_helper.cc index 6f4cc91..ed21ea5 100644 --- a/chrome/browser/ui/sync/one_click_signin_helper.cc +++ b/chrome/browser/ui/sync/one_click_signin_helper.cc @@ -22,7 +22,6 @@ #include "chrome/browser/tab_contents/tab_util.h" #include "chrome/browser/ui/browser_finder.h" #include "chrome/browser/ui/browser_window.h" -#include "chrome/browser/ui/sync/one_click_signin_dialog.h" #include "chrome/browser/ui/sync/one_click_signin_histogram.h" #include "chrome/browser/ui/sync/one_click_signin_sync_starter.h" #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" @@ -63,6 +62,8 @@ class OneClickLoginInfoBarDelegate : public ConfirmInfoBarDelegate { virtual string16 GetButtonLabel(InfoBarButton button) const OVERRIDE; virtual bool Accept() OVERRIDE; virtual bool Cancel() OVERRIDE; + virtual string16 GetLinkText() const OVERRIDE; + virtual bool LinkClicked(WindowOpenDisposition disposition) OVERRIDE; virtual InfoBarAutomationType GetInfoBarAutomationType() const OVERRIDE; @@ -129,47 +130,28 @@ string16 OneClickLoginInfoBarDelegate::GetButtonLabel( namespace { -void OnLearnMore(Browser* browser) { - browser->AddSelectedTabWithURL( - GURL(chrome::kSyncLearnMoreURL), - content::PAGE_TRANSITION_AUTO_BOOKMARK); -} - -void OnAdvanced(Browser* browser) { - browser->AddSelectedTabWithURL( - GURL(std::string(chrome::kChromeUISettingsURL) + - chrome::kSyncSetupSubPage), - content::PAGE_TRANSITION_AUTO_BOOKMARK); -} - // Start syncing with the given user information. void StartSync(content::WebContents* web_contents, const std::string& session_index, const std::string& email, const std::string& password, - bool use_default_settings) { + OneClickSigninSyncStarter::StartSyncMode start_mode) { // The starter deletes itself once its done. Profile* profile = Profile::FromBrowserContext(web_contents->GetBrowserContext()); - ignore_result( - new OneClickSigninSyncStarter( - profile, session_index, email, password, use_default_settings)); - - Browser* browser = browser::FindBrowserWithWebContents(web_contents); - browser->window()->ShowOneClickSigninBubble( - base::Bind(&OnLearnMore, base::Unretained(browser)), - base::Bind(&OnAdvanced, base::Unretained(browser))); + new OneClickSigninSyncStarter(profile, session_index, email, password, + start_mode); } } // namespace bool OneClickLoginInfoBarDelegate::Accept() { DisableOneClickSignIn(); + content::WebContents* web_contents = owner()->web_contents(); RecordHistogramAction(one_click_signin::HISTOGRAM_ACCEPTED); - ShowOneClickSigninDialog( - owner()->web_contents()->GetView()->GetTopLevelNativeWindow(), - base::Bind(&StartSync, owner()->web_contents(), session_index_, email_, - password_)); + browser::FindBrowserWithWebContents(web_contents)->window()-> + ShowOneClickSigninBubble(base::Bind(&StartSync, web_contents, + session_index_, email_, password_)); button_pressed_ = true; return true; } @@ -181,6 +163,21 @@ bool OneClickLoginInfoBarDelegate::Cancel() { return true; } +string16 OneClickLoginInfoBarDelegate::GetLinkText() const { + return l10n_util::GetStringUTF16(IDS_LEARN_MORE); +} + +bool OneClickLoginInfoBarDelegate::LinkClicked( + WindowOpenDisposition disposition) { + RecordHistogramAction(one_click_signin::HISTOGRAM_LEARN_MORE); + content::OpenURLParams params( + GURL(chrome::kChromeSyncLearnMoreURL), content::Referrer(), disposition, + content::PAGE_TRANSITION_LINK, false); + owner()->web_contents()->OpenURL(params); + return false; +} + + InfoBarDelegate::InfoBarAutomationType OneClickLoginInfoBarDelegate::GetInfoBarAutomationType() const { return ONE_CLICK_LOGIN_INFOBAR; diff --git a/chrome/browser/ui/sync/one_click_signin_helper_unittest.cc b/chrome/browser/ui/sync/one_click_signin_helper_unittest.cc new file mode 100644 index 0000000..7622cf2 --- /dev/null +++ b/chrome/browser/ui/sync/one_click_signin_helper_unittest.cc @@ -0,0 +1,154 @@ +// 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/content_settings/cookie_settings.h" +#include "chrome/browser/prefs/pref_service.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/profiles/profile_manager.h" +#include "chrome/browser/signin/signin_manager_factory.h" +#include "chrome/browser/signin/signin_manager_fake.h" +#include "chrome/browser/ui/sync/one_click_signin_helper.h" +#include "chrome/common/pref_names.h" +#include "chrome/test/base/testing_profile.h" +#include "content/public/browser/browser_context.h" +#include "content/public/browser/navigation_controller.h" +#include "content/public/browser/site_instance.h" +#include "content/public/browser/web_contents.h" +#include "content/test/test_browser_context.h" +#include "content/test/test_browser_thread.h" +#include "content/test/web_contents_tester.h" +#include "testing/gmock/include/gmock/gmock-actions.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +class OneClickSigninHelperTest : public testing::Test { + public: + OneClickSigninHelperTest() : signin_manager_(NULL) { + } + + virtual void TearDown() OVERRIDE; + + protected: + // Marks the current thread as the UI thread, so that calls that assume + // they are called on that thread work. + void MarkCurrentThreadAsUIThread(); + + // Creates a mock WebContents for tests. If |use_incognito| is true then + // a WebContents for an incognito profile is created. + content::WebContents* CreateMockWebContents(bool use_incognito); + + void EnableOneClick(bool enable); + + void AllowSigninCookies(bool enable); + + // Marks the profile as connected to the given account. If |username| is the + // empty string, the profile is not connected. + void ConnectProfileToAccount(const std::string& username); + + private: + // Members to fake that we are on the UI thread. + MessageLoop message_loop_; + scoped_ptr<content::TestBrowserThread> ui_thread_; + + // Mock objects used during tests. The objects need to be torn down in the + // correct order, see TearDown(). + scoped_ptr<content::WebContents> web_contents_; + scoped_ptr<TestingProfile> profile_; + SigninManager* signin_manager_; + + DISALLOW_COPY_AND_ASSIGN(OneClickSigninHelperTest); +}; + +void OneClickSigninHelperTest::TearDown() { + // Destroy things in proper order. + web_contents_.reset(); + profile_.reset(); + ui_thread_.reset(); +} + +void OneClickSigninHelperTest::MarkCurrentThreadAsUIThread() { + ui_thread_.reset(new content::TestBrowserThread( + content::BrowserThread::UI, &message_loop_)); +} + +content::WebContents* OneClickSigninHelperTest::CreateMockWebContents( + bool use_incognito) { + EXPECT_TRUE(web_contents_.get() == NULL); + + profile_.reset(new TestingProfile()); + signin_manager_ = static_cast<SigninManager*>( + SigninManagerFactory::GetInstance()->SetTestingFactoryAndUse( + profile_.get(), FakeSigninManager::Build)); + + profile_->set_incognito(use_incognito); + web_contents_.reset(content::WebContentsTester::CreateTestWebContents( + profile_.get(), NULL)); + return web_contents_.get(); +} + +void OneClickSigninHelperTest::EnableOneClick(bool enable) { + PrefService* pref_service = profile_->GetPrefs(); + pref_service->SetBoolean(prefs::kReverseAutologinEnabled, enable); +} + +void OneClickSigninHelperTest::AllowSigninCookies(bool enable) { + CookieSettings* cookie_settings = + CookieSettings::Factory::GetForProfile(profile_.get()); + cookie_settings->SetDefaultCookieSetting( + enable ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK); +} + +void OneClickSigninHelperTest::ConnectProfileToAccount( + const std::string& username) { + signin_manager_->StartSignIn(username, std::string(), std::string(), + std::string()); +} + +} // namespace + +TEST_F(OneClickSigninHelperTest, CanOfferNoContents) { + EXPECT_FALSE(OneClickSigninHelper::CanOffer(NULL, true)); + EXPECT_FALSE(OneClickSigninHelper::CanOffer(NULL, false)); +} + +TEST_F(OneClickSigninHelperTest, CanOffer) { + MarkCurrentThreadAsUIThread(); + content::WebContents* web_contents = CreateMockWebContents(false); + + EnableOneClick(true); + EXPECT_TRUE(OneClickSigninHelper::CanOffer(web_contents, true)); + EXPECT_TRUE(OneClickSigninHelper::CanOffer(web_contents, false)); + + EnableOneClick(false); + EXPECT_FALSE(OneClickSigninHelper::CanOffer(web_contents, true)); + EXPECT_FALSE(OneClickSigninHelper::CanOffer(web_contents, false)); +} + +TEST_F(OneClickSigninHelperTest, CanOfferProfileConnected) { + MarkCurrentThreadAsUIThread(); + content::WebContents* web_contents = CreateMockWebContents(false); + ConnectProfileToAccount("foo@gmail.com"); + + EXPECT_FALSE(OneClickSigninHelper::CanOffer(web_contents, true)); + EXPECT_TRUE(OneClickSigninHelper::CanOffer(web_contents, false)); +} + +TEST_F(OneClickSigninHelperTest, CanOfferIncognito) { + MarkCurrentThreadAsUIThread(); + content::WebContents* web_contents = CreateMockWebContents(true); + + EXPECT_FALSE(OneClickSigninHelper::CanOffer(web_contents, true)); + EXPECT_FALSE(OneClickSigninHelper::CanOffer(web_contents, false)); +} + +TEST_F(OneClickSigninHelperTest, CanOfferNoSigninCookies) { + MarkCurrentThreadAsUIThread(); + content::WebContents* web_contents = CreateMockWebContents(true); + AllowSigninCookies(false); + + EXPECT_FALSE(OneClickSigninHelper::CanOffer(web_contents, true)); + EXPECT_FALSE(OneClickSigninHelper::CanOffer(web_contents, false)); +} diff --git a/chrome/browser/ui/sync/one_click_signin_sync_starter.cc b/chrome/browser/ui/sync/one_click_signin_sync_starter.cc index 11c41ac..29baed14 100644 --- a/chrome/browser/ui/sync/one_click_signin_sync_starter.cc +++ b/chrome/browser/ui/sync/one_click_signin_sync_starter.cc @@ -19,14 +19,15 @@ OneClickSigninSyncStarter::OneClickSigninSyncStarter( const std::string& session_index, const std::string& email, const std::string& password, - bool use_default_settings) + StartSyncMode start_mode) : profile_(profile), signin_tracker_(profile, this), - use_default_settings_(use_default_settings) { + start_mode_(start_mode) { DCHECK(profile_); - int action = use_default_settings ? one_click_signin::HISTOGRAM_WITH_DEFAULTS - : one_click_signin::HISTOGRAM_WITH_ADVANCED; + int action = start_mode_ == SYNC_WITH_DEFAULT_SETTINGS ? + one_click_signin::HISTOGRAM_WITH_DEFAULTS : + one_click_signin::HISTOGRAM_WITH_ADVANCED; UMA_HISTOGRAM_ENUMERATION("AutoLogin.Reverse", action, one_click_signin::HISTOGRAM_MAX); @@ -57,7 +58,7 @@ void OneClickSigninSyncStarter::SigninSuccess() { ProfileSyncService* profile_sync_service = ProfileSyncServiceFactory::GetForProfile(profile_); - if (use_default_settings_) { + if (start_mode_ == SYNC_WITH_DEFAULT_SETTINGS) { // Just kick off the sync machine, no need to configure it first. profile_sync_service->SetSyncSetupCompleted(); profile_sync_service->set_setup_in_progress(false); diff --git a/chrome/browser/ui/sync/one_click_signin_sync_starter.h b/chrome/browser/ui/sync/one_click_signin_sync_starter.h index 06e8633..dd5cafd 100644 --- a/chrome/browser/ui/sync/one_click_signin_sync_starter.h +++ b/chrome/browser/ui/sync/one_click_signin_sync_starter.h @@ -17,11 +17,13 @@ class Profile; // the job is done. class OneClickSigninSyncStarter : public SigninTracker::Observer { public: + enum StartSyncMode {SYNC_WITH_DEFAULT_SETTINGS, CONFIGURE_SYNC_FIRST }; + OneClickSigninSyncStarter(Profile* profile, const std::string& session_index, const std::string& email, const std::string& password, - bool use_default_settings); + StartSyncMode start_mode); private: virtual ~OneClickSigninSyncStarter(); @@ -33,7 +35,7 @@ class OneClickSigninSyncStarter : public SigninTracker::Observer { Profile* const profile_; SigninTracker signin_tracker_; - bool use_default_settings_; + StartSyncMode start_mode_; DISALLOW_COPY_AND_ASSIGN(OneClickSigninSyncStarter); }; diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc index eff052e..a417b0b 100644 --- a/chrome/browser/ui/views/frame/browser_view.cc +++ b/chrome/browser/ui/views/frame/browser_view.cc @@ -1065,11 +1065,9 @@ void BrowserView::ShowChromeToMobileBubble() { #if defined(ENABLE_ONE_CLICK_SIGNIN) void BrowserView::ShowOneClickSigninBubble( - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback) { + const StartSyncCallback& start_sync_callback) { OneClickSigninBubbleView::ShowBubble(toolbar_->app_menu(), - learn_more_callback, - advanced_callback); + start_sync_callback); } #endif diff --git a/chrome/browser/ui/views/frame/browser_view.h b/chrome/browser/ui/views/frame/browser_view.h index 27c974a..1320262 100644 --- a/chrome/browser/ui/views/frame/browser_view.h +++ b/chrome/browser/ui/views/frame/browser_view.h @@ -276,8 +276,7 @@ class BrowserView : public BrowserWindow, virtual void ShowChromeToMobileBubble() OVERRIDE; #if defined(ENABLE_ONE_CLICK_SIGNIN) virtual void ShowOneClickSigninBubble( - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback) OVERRIDE; + const StartSyncCallback& start_sync_callback) OVERRIDE; #endif // TODO(beng): Not an override, move somewhere else. void SetDownloadShelfVisible(bool visible); diff --git a/chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc b/chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc index 0c2281d..40a9f01 100644 --- a/chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc +++ b/chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc @@ -4,6 +4,7 @@ #include "chrome/browser/ui/views/sync/one_click_signin_bubble_view.h" +#include "base/callback_helpers.h" #include "base/logging.h" #include "base/message_loop.h" #include "chrome/browser/google/google_util.h" @@ -21,11 +22,9 @@ #include "ui/views/layout/grid_layout.h" #include "ui/views/layout/layout_constants.h" -// Minimum width for the fields - they will push out the size of the bubble if -// necessary. This should be big enough so that the field pushes the right side -// of the bubble far enough so that the edit button's left edge is to the right -// of the field's left edge. -const int kMinimumFieldSize = 240; +// Minimum width for the mutli-line label. +const int kMinimumLabelWidth = 240; + // BookmarkBubbleView --------------------------------------------------------- @@ -35,14 +34,12 @@ OneClickSigninBubbleView* OneClickSigninBubbleView::bubble_view_ = NULL; // static void OneClickSigninBubbleView::ShowBubble( views::View* anchor_view, - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback) { + const BrowserWindow::StartSyncCallback& start_sync) { if (IsShowing()) return; bubble_view_ = - new OneClickSigninBubbleView(anchor_view, learn_more_callback, - advanced_callback); + new OneClickSigninBubbleView(anchor_view, start_sync); views::BubbleDelegateView::CreateBubble(bubble_view_); bubble_view_->Show(); } @@ -60,17 +57,14 @@ void OneClickSigninBubbleView::Hide() { OneClickSigninBubbleView::OneClickSigninBubbleView( views::View* anchor_view, - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback) + const BrowserWindow::StartSyncCallback& start_sync_callback) : BubbleDelegateView(anchor_view, views::BubbleBorder::TOP_RIGHT), - learn_more_link_(NULL), advanced_link_(NULL), - close_button_(NULL), - learn_more_callback_(learn_more_callback), - advanced_callback_(advanced_callback), + ok_button_(NULL), + undo_button_(NULL), + start_sync_callback_(start_sync_callback), message_loop_for_testing_(NULL) { - DCHECK(!learn_more_callback_.is_null()); - DCHECK(!advanced_callback_.is_null()); + DCHECK(!start_sync_callback_.is_null()); } OneClickSigninBubbleView::~OneClickSigninBubbleView() { @@ -95,35 +89,28 @@ void OneClickSigninBubbleView::Init() { // Column set for descriptive text and link. views::ColumnSet* cs = layout->AddColumnSet(kColumnSetFillAlign); cs->AddColumn(views::GridLayout::FILL, views::GridLayout::CENTER, 0, - views::GridLayout::USE_PREF, 0, kMinimumFieldSize); + views::GridLayout::USE_PREF, 0, kMinimumLabelWidth); cs = layout->AddColumnSet(kColumnSetControls); cs->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER, 0, views::GridLayout::USE_PREF, 0, 0); - cs->AddPaddingColumn(1, 0); + cs->AddPaddingColumn(1, views::kUnrelatedControlHorizontalSpacing); + cs->AddColumn(views::GridLayout::TRAILING, views::GridLayout::CENTER, 0, + views::GridLayout::USE_PREF, 0, 0); + cs->AddPaddingColumn(0, views::kRelatedButtonHSpacing); cs->AddColumn(views::GridLayout::TRAILING, views::GridLayout::CENTER, 0, views::GridLayout::USE_PREF, 0, 0); // Add main text description. views::Label* label = new views::Label( - l10n_util::GetStringFUTF16(IDS_SYNC_PROMO_NTP_BUBBLE_MESSAGE, - l10n_util::GetStringUTF16(IDS_SHORT_PRODUCT_NAME))); + l10n_util::GetStringUTF16(IDS_ONE_CLICK_SIGNIN_BUBBLE_MESSAGE)); label->SetMultiLine(true); label->SetHorizontalAlignment(views::Label::ALIGN_LEFT); - label->SizeToFit(kMinimumFieldSize); + label->SizeToFit(kMinimumLabelWidth); layout->StartRow(0, kColumnSetFillAlign); layout->AddView(label); - // Add link for user to learn more about sync. - learn_more_link_= new views::Link( - l10n_util::GetStringUTF16(IDS_SYNC_PROMO_NTP_BUBBLE_LEARN_MORE)); - learn_more_link_->set_listener(this); - learn_more_link_->SetHorizontalAlignment(views::Label::ALIGN_LEFT); - - layout->StartRow(0, kColumnSetFillAlign); - layout->AddView(learn_more_link_); - layout->AddPaddingRow(0, views::kRelatedControlSmallVerticalSpacing); // Add link for user to do advanced config of sync. @@ -133,13 +120,27 @@ void OneClickSigninBubbleView::Init() { advanced_link_->SetHorizontalAlignment(views::Label::ALIGN_LEFT); // Add controls at the bottom. - close_button_ = new views::NativeTextButton( - this, l10n_util::GetStringUTF16(IDS_OK)); - close_button_->SetIsDefault(true); + ok_button_ = new views::NativeTextButton(this); + ok_button_->SetIsDefault(true); + + undo_button_ = new views::NativeTextButton(this); + + // The default size of the buttons is too large. To allow them to be smaller + // ignore the minimum default size. Furthermore, to make sure they are the + // same size, SetText() is called with both strings on both buttons. + ok_button_->set_ignore_minimum_size(true); + undo_button_->set_ignore_minimum_size(true); + string16 ok_label = l10n_util::GetStringUTF16(IDS_OK); + string16 undo_label = l10n_util::GetStringUTF16(IDS_ONE_CLICK_BUBBLE_UNDO); + ok_button_->SetText(undo_label); + ok_button_->SetText(ok_label); + undo_button_->SetText(ok_label); + undo_button_->SetText(undo_label); layout->StartRow(0, kColumnSetControls); layout->AddView(advanced_link_); - layout->AddView(close_button_); + layout->AddView(ok_button_); + layout->AddView(undo_button_); AddAccelerator(ui::Accelerator(ui::VKEY_RETURN, 0)); } @@ -150,13 +151,25 @@ void OneClickSigninBubbleView::WindowClosing() { // before then. DCHECK(bubble_view_ == this); bubble_view_ = NULL; - } + + if (!start_sync_callback_.is_null()) { + base::ResetAndReturn(&start_sync_callback_).Run( + OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS); + } +} bool OneClickSigninBubbleView::AcceleratorPressed( const ui::Accelerator& accelerator) { if (accelerator.key_code() == ui::VKEY_RETURN || accelerator.key_code() == ui::VKEY_ESCAPE) { StartFade(false); + if (accelerator.key_code() == ui::VKEY_RETURN) { + base::ResetAndReturn(&start_sync_callback_).Run( + OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS); + } else { + start_sync_callback_.Reset(); + } + return true; } @@ -166,15 +179,17 @@ bool OneClickSigninBubbleView::AcceleratorPressed( void OneClickSigninBubbleView::LinkClicked(views::Link* source, int event_flags) { StartFade(false); - - if (source == learn_more_link_) - learn_more_callback_.Run(); - else - advanced_callback_.Run(); + base::ResetAndReturn(&start_sync_callback_).Run( + OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST); } void OneClickSigninBubbleView::ButtonPressed(views::Button* sender, const views::Event& event) { - DCHECK_EQ(close_button_, sender); StartFade(false); + if (ok_button_ == sender) { + base::ResetAndReturn(&start_sync_callback_).Run( + OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS); + } else { + start_sync_callback_.Reset(); + } } diff --git a/chrome/browser/ui/views/sync/one_click_signin_bubble_view.h b/chrome/browser/ui/views/sync/one_click_signin_bubble_view.h index 53abb0f..b4d2ab0 100644 --- a/chrome/browser/ui/views/sync/one_click_signin_bubble_view.h +++ b/chrome/browser/ui/views/sync/one_click_signin_bubble_view.h @@ -9,7 +9,9 @@ #include "base/basictypes.h" #include "base/callback.h" #include "base/compiler_specific.h" +#include "base/gtest_prod_util.h" #include "base/string16.h" +#include "chrome/browser/ui/browser_window.h" #include "ui/views/bubble/bubble_delegate.h" #include "ui/views/controls/button/button.h" #include "ui/views/controls/link_listener.h" @@ -28,11 +30,10 @@ class OneClickSigninBubbleView : public views::BubbleDelegateView, public views::ButtonListener { public: // Show the one-click signin bubble if not already showing. The bubble - // will be placed visually beneath |anchor_view|. The |browser| is used - // to open links. + // will be placed visually beneath |anchor_view|. |start_sync| is called + // to start sync. static void ShowBubble(views::View* anchor_view, - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback); + const BrowserWindow::StartSyncCallback& start_sync); static bool IsShowing(); @@ -42,19 +43,17 @@ class OneClickSigninBubbleView : public views::BubbleDelegateView, // method is meant to be called only from tests. static OneClickSigninBubbleView* view_for_testing() { return bubble_view_; } - // The following accessor message should only be used for testing. - views::Link* learn_more_link_for_testing() const { return learn_more_link_; } - views::Link* advanced_link_for_testing() const { return advanced_link_; } - views::TextButton* close_button_for_testing() const { return close_button_; } - void set_message_loop_for_testing(MessageLoop* loop) { - message_loop_for_testing_ = loop; - } - private: - // Creates a BookmarkBubbleView. - OneClickSigninBubbleView(views::View* anchor_view, - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback); + friend class OneClickSigninBubbleViewBrowserTest; + + FRIEND_TEST_ALL_PREFIXES(OneClickSigninBubbleViewBrowserTest, OkButton); + FRIEND_TEST_ALL_PREFIXES(OneClickSigninBubbleViewBrowserTest, UndoButton); + FRIEND_TEST_ALL_PREFIXES(OneClickSigninBubbleViewBrowserTest, AdvancedLink); + + // Creates a OneClickSigninBubbleView. + OneClickSigninBubbleView( + views::View* anchor_view, + const BrowserWindow::StartSyncCallback& start_sync_callback); virtual ~OneClickSigninBubbleView(); @@ -78,18 +77,17 @@ class OneClickSigninBubbleView : public views::BubbleDelegateView, // The bubble, if we're showing one. static OneClickSigninBubbleView* bubble_view_; - // Link to web page to learn more about sync. - views::Link* learn_more_link_; - // Link to sync setup advanced page. views::Link* advanced_link_; - // Button to close the window. - views::TextButton* close_button_; + // Controls at bottom of bubble. + views::TextButton* ok_button_; + views::TextButton* undo_button_; - // The callbacks for the links. - base::Closure learn_more_callback_; - base::Closure advanced_callback_; + // This callback is nulled once its called, so that it is called only once. + // It will be called when the bubble is closed if it has not been called + // and nulled earlier. + BrowserWindow::StartSyncCallback start_sync_callback_; // A message loop used only with unit tests. MessageLoop* message_loop_for_testing_; diff --git a/chrome/browser/ui/views/sync/one_click_signin_bubble_view_browsertest.cc b/chrome/browser/ui/views/sync/one_click_signin_bubble_view_browsertest.cc index f9541f5..a80cc5f 100644 --- a/chrome/browser/ui/views/sync/one_click_signin_bubble_view_browsertest.cc +++ b/chrome/browser/ui/views/sync/one_click_signin_bubble_view_browsertest.cc @@ -13,26 +13,38 @@ #include "ui/views/controls/button/text_button.h" #include "ui/views/events/event.h" -namespace { +class OneClickSigninBubbleViewBrowserTest : public InProcessBrowserTest { + public: + OneClickSigninBubbleViewBrowserTest() + : on_start_sync_called_(false), + mode_(OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST) { + } -void OnClickLink(Browser* browser) { - browser->AddSelectedTabWithURL(GURL("http://www.example.com"), - content::PAGE_TRANSITION_AUTO_BOOKMARK); -} + OneClickSigninBubbleView* ShowOneClickSigninBubble() { + browser()->window()->ShowOneClickSigninBubble( + base::Bind(&OneClickSigninBubbleViewBrowserTest::OnStartSync, this)); -} // namespace + ui_test_utils::RunAllPendingInMessageLoop(); + EXPECT_TRUE(OneClickSigninBubbleView::IsShowing()); -class OneClickSigninBubbleViewBrowserTest : public InProcessBrowserTest { - public: - OneClickSigninBubbleViewBrowserTest() {} + OneClickSigninBubbleView* view = + OneClickSigninBubbleView::view_for_testing(); + EXPECT_TRUE(view != NULL); - void ShowOneClickSigninBubble() { - base::Closure on_click_link_callback = - base::Bind(&OnClickLink, base::Unretained(browser())); - browser()->window()->ShowOneClickSigninBubble(on_click_link_callback, - on_click_link_callback); + // Simulate pressing a link in the bubble. This should open a new tab. + view->message_loop_for_testing_ = MessageLoop::current(); + return view; } + void OnStartSync(OneClickSigninSyncStarter::StartSyncMode mode) { + on_start_sync_called_ = true; + mode_ = mode; + } + + protected: + bool on_start_sync_called_; + OneClickSigninSyncStarter::StartSyncMode mode_; + private: DISALLOW_COPY_AND_ASSIGN(OneClickSigninBubbleViewBrowserTest); }; @@ -44,51 +56,85 @@ IN_PROC_BROWSER_TEST_F(OneClickSigninBubbleViewBrowserTest, Show) { OneClickSigninBubbleView::Hide(); ui_test_utils::RunAllPendingInMessageLoop(); + EXPECT_TRUE(on_start_sync_called_); + EXPECT_EQ(OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS, mode_); EXPECT_FALSE(OneClickSigninBubbleView::IsShowing()); } -IN_PROC_BROWSER_TEST_F(OneClickSigninBubbleViewBrowserTest, CloseButton) { - int initial_tab_count = browser()->tab_count(); - - ShowOneClickSigninBubble(); - ui_test_utils::RunAllPendingInMessageLoop(); - EXPECT_TRUE(OneClickSigninBubbleView::IsShowing()); - - OneClickSigninBubbleView* view = OneClickSigninBubbleView::view_for_testing(); - EXPECT_TRUE(view != NULL); - EXPECT_EQ(initial_tab_count, browser()->tab_count()); +IN_PROC_BROWSER_TEST_F(OneClickSigninBubbleViewBrowserTest, OkButton) { + OneClickSigninBubbleView* view = ShowOneClickSigninBubble(); // Simulate pressing the OK button. Set the message loop in the bubble // view so that it can be quit once the bubble is hidden. - view->set_message_loop_for_testing(MessageLoop::current()); views::ButtonListener* listener = view; - views::MouseEvent event(ui::ET_MOUSE_PRESSED, 0, 0, 0); - listener->ButtonPressed(view->close_button_for_testing(), event); + const views::MouseEvent event(ui::ET_MOUSE_PRESSED, 0, 0, 0); + listener->ButtonPressed(view->ok_button_, event); // View should no longer be showing. The message loop will exit once the // fade animation of the bubble is done. ui_test_utils::RunMessageLoop(); + EXPECT_TRUE(on_start_sync_called_); + EXPECT_EQ(OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS, mode_); EXPECT_FALSE(OneClickSigninBubbleView::IsShowing()); - EXPECT_EQ(initial_tab_count, browser()->tab_count()); } -IN_PROC_BROWSER_TEST_F(OneClickSigninBubbleViewBrowserTest, ViewLink) { - int initial_tab_count = browser()->tab_count(); +IN_PROC_BROWSER_TEST_F(OneClickSigninBubbleViewBrowserTest, UndoButton) { + OneClickSigninBubbleView* view = ShowOneClickSigninBubble(); - ShowOneClickSigninBubble(); - ui_test_utils::RunAllPendingInMessageLoop(); - EXPECT_TRUE(OneClickSigninBubbleView::IsShowing()); + // Simulate pressing the undo button. Set the message loop in the bubble + // view so that it can be quit once the bubble is hidden. + views::ButtonListener* listener = view; + const views::MouseEvent event(ui::ET_MOUSE_PRESSED, 0, 0, 0); + listener->ButtonPressed(view->undo_button_, event); - OneClickSigninBubbleView* view = OneClickSigninBubbleView::view_for_testing(); - EXPECT_TRUE(view != NULL); - EXPECT_EQ(initial_tab_count, browser()->tab_count()); + // View should no longer be showing. The message loop will exit once the + // fade animation of the bubble is done. + ui_test_utils::RunMessageLoop(); + EXPECT_FALSE(on_start_sync_called_); + EXPECT_FALSE(OneClickSigninBubbleView::IsShowing()); +} + +IN_PROC_BROWSER_TEST_F(OneClickSigninBubbleViewBrowserTest, AdvancedLink) { + OneClickSigninBubbleView* view = ShowOneClickSigninBubble(); // Simulate pressing a link in the bubble. This should open a new tab. views::LinkListener* listener = view; - listener->LinkClicked(view->learn_more_link_for_testing(), 0); + listener->LinkClicked(view->advanced_link_, 0); // View should no longer be showing and a new tab should be opened. - ui_test_utils::RunAllPendingInMessageLoop(); + ui_test_utils::RunMessageLoop(); + EXPECT_TRUE(on_start_sync_called_); + EXPECT_EQ(OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST, mode_); + EXPECT_FALSE(OneClickSigninBubbleView::IsShowing()); +} + +IN_PROC_BROWSER_TEST_F(OneClickSigninBubbleViewBrowserTest, PressEnterKey) { + OneClickSigninBubbleView* one_click_view = ShowOneClickSigninBubble(); + + // Simulate pressing the Enter key. + views::View* view = one_click_view; + const ui::Accelerator accelerator(ui::VKEY_RETURN, 0); + view->AcceleratorPressed(accelerator); + + // View should no longer be showing. The message loop will exit once the + // fade animation of the bubble is done. + ui_test_utils::RunMessageLoop(); + EXPECT_TRUE(on_start_sync_called_); + EXPECT_EQ(OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS, mode_); + EXPECT_FALSE(OneClickSigninBubbleView::IsShowing()); +} + +IN_PROC_BROWSER_TEST_F(OneClickSigninBubbleViewBrowserTest, PressEscapeKey) { + OneClickSigninBubbleView* one_click_view = ShowOneClickSigninBubble(); + + // Simulate pressing the Escape key. + views::View* view = one_click_view; + const ui::Accelerator accelerator(ui::VKEY_ESCAPE, 0); + view->AcceleratorPressed(accelerator); + + // View should no longer be showing. The message loop will exit once the + // fade animation of the bubble is done. + ui_test_utils::RunMessageLoop(); + EXPECT_FALSE(on_start_sync_called_); EXPECT_FALSE(OneClickSigninBubbleView::IsShowing()); - EXPECT_EQ(initial_tab_count + 1, browser()->tab_count()); } diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index ca2c726..1f6b6f6 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1751,6 +1751,7 @@ 'browser/ui/panels/panel_mouse_watcher_unittest.cc', 'browser/ui/search_engines/keyword_editor_controller_unittest.cc', 'browser/ui/select_file_dialog_unittest.cc', + 'browser/ui/sync/one_click_signin_helper_unittest.cc', 'browser/ui/tab_contents/tab_contents_iterator_unittest.cc', 'browser/ui/tabs/dock_info_unittest.cc', 'browser/ui/tabs/pinned_tab_codec_unittest.cc', @@ -2006,6 +2007,7 @@ 'browser/ui/cocoa/one_click_signin_bubble_controller_unittest.mm', 'browser/ui/cocoa/one_click_signin_dialog_controller_unittest.mm', 'browser/ui/gtk/one_click_signin_dialog_gtk_unittest.cc', + 'browser/ui/sync/one_click_signin_helper_unittest.cc', ] }], ['enable_promo_resource_service==0', { diff --git a/chrome/common/url_constants.cc b/chrome/common/url_constants.cc index 2204ea9..da738ec 100644 --- a/chrome/common/url_constants.cc +++ b/chrome/common/url_constants.cc @@ -295,6 +295,13 @@ const char kChromeHelpURL[] = "https://support.google.com/chrome/?p=help"; #endif +const char kChromeSyncLearnMoreURL[] = +#if defined(OS_CHROMEOS) + "http://support.google.com/chromeos/bin/answer.py?hl=en&answer=165139"; +#else + "http://support.google.com/chrome/bin/answer.py?hl=en&answer=165139"; +#endif + const char kSettingsSearchHelpURL[] = #if defined(OS_CHROMEOS) "https://support.google.com/chromeos/?p=settings_search_help"; diff --git a/chrome/common/url_constants.h b/chrome/common/url_constants.h index 31f6042..123f16e 100644 --- a/chrome/common/url_constants.h +++ b/chrome/common/url_constants.h @@ -271,6 +271,9 @@ extern const char kPasswordManagerLearnMoreURL[]; // General help link for Chrome. extern const char kChromeHelpURL[]; +// "Learn more" URL for the one click signin infobar. +extern const char kChromeSyncLearnMoreURL[]; + // Help URL for the settings page's search feature. extern const char kSettingsSearchHelpURL[]; diff --git a/chrome/test/base/test_browser_window.h b/chrome/test/base/test_browser_window.h index 56c7e5d..dd19895 100644 --- a/chrome/test/base/test_browser_window.h +++ b/chrome/test/base/test_browser_window.h @@ -99,8 +99,7 @@ class TestBrowserWindow : public BrowserWindow { virtual void ShowChromeToMobileBubble() OVERRIDE {} #if defined(ENABLE_ONE_CLICK_SIGNIN) virtual void ShowOneClickSigninBubble( - const base::Closure& learn_more_callback, - const base::Closure& advanced_callback) OVERRIDE {} + const StartSyncCallback& start_sync_callback) OVERRIDE {} #endif virtual bool IsDownloadShelfVisible() const OVERRIDE; virtual DownloadShelf* GetDownloadShelf() OVERRIDE; diff --git a/chrome/test/functional/PYAUTO_TESTS b/chrome/test/functional/PYAUTO_TESTS index 8664ff8..10fdfef 100644 --- a/chrome/test/functional/PYAUTO_TESTS +++ b/chrome/test/functional/PYAUTO_TESTS @@ -84,10 +84,6 @@ 'test_pyauto', 'themes', - # One-click infobars disabled on M20 by r135749. - # Re-enable for M21 after branch point. - '-infobars.OneClickInfobarTest', - # ================================================== # Disabled tests that need to be investigated/fixed. # ================================================== |