Win: Display a native bubble (instead of the JS one) after the web signin flow.

chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc

Index: chrome/browser/ui/views/sync/one_click_signin_bubble_view.cc
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 ca704334132c0ee2c197c084fb46b9c8c335f1bf..c7c2ad11c2632b3fd331652433a8fb2e478fc1ce 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
@@ -28,7 +28,7 @@
 #include "ui/views/layout/layout_constants.h"
 #include "ui/views/widget/widget.h"
 
-// Minimum width for the mutli-line label.
+// Minimum width for the multi-line label.
 const int kMinimumDialogLabelWidth = 400;
 const int kMinimumLabelWidth = 240;
 const int kDialogMargin = 16;
@@ -57,16 +57,31 @@ class OneClickSigninDialogView : public OneClickSigninBubbleView {
 
   // Overridden from OneClickSigninBubbleView:
   virtual void InitContent(views::GridLayout* layout) OVERRIDE;
+
+  virtual void InitButtons(views::GridLayout* layout) OVERRIDE;
   virtual void GetButtons(views::LabelButton** ok_button,
                           views::LabelButton** undo_button) OVERRIDE;
-  virtual views::Link* GetAdvancedLink() OVERRIDE;
+
+  // Overridden from views::ButtonListener:
+  virtual void ButtonPressed(views::Button* sender,
+                             const ui::Event& event) OVERRIDE;
 
   // Overridden from views::LinkListener:
   virtual void LinkClicked(views::Link* source, int event_flags) OVERRIDE;
 
+  // views::View method:

[tfarina, 2013/04/16 17:25:28]

be consistent with the file you are modifying. All the current above are using
"Overridden from ..." so this should too.

+  virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) OVERRIDE;
+
+  // views::WidgetDelegate method:
+  virtual void WindowClosing() OVERRIDE;
+
   const string16 email_;
-  content::WebContents* web_content_;
-  views::Link* learn_more_link_;
+
+  // 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_;
+
   views::ImageButton* close_button_;
 
   DISALLOW_COPY_AND_ASSIGN(OneClickSigninDialogView);
@@ -77,11 +92,11 @@ OneClickSigninDialogView::OneClickSigninDialogView(
     views::View* anchor_view,
     const string16& email,
     const BrowserWindow::StartSyncCallback& start_sync_callback)
-    : OneClickSigninBubbleView(anchor_view, start_sync_callback),
+    : OneClickSigninBubbleView(web_content, anchor_view, ""),

[bcwhite, 2013/04/16 15:22:00]

std::string() is better than "", here and elsewhere.

[noms (inactive), 2013/04/16 17:15:37]

Done.

[tfarina, 2013/04/16 17:25:28]

It isn't just better:
https://codereview.chromium.org/12746010/
https://codereview.chromium.org/13145003/

       email_(email),
-      web_content_(web_content),
-      learn_more_link_(NULL),
-      close_button_(NULL) {
+      close_button_(NULL),
+      start_sync_callback_(start_sync_callback) {
+  DCHECK(!start_sync_callback_.is_null());
   set_arrow(views::BubbleBorder::NONE);
   set_anchor_view_insets(gfx::Insets(0, 0, anchor_view->height() / 2, 0));
   set_close_on_deactivate(false);
@@ -122,6 +137,7 @@ void OneClickSigninDialogView::InitContent(views::GridLayout* layout) {
                             rb.GetImageNamed(IDR_CLOSE_2_H).ToImageSkia());
     close_button_->SetImage(views::ImageButton::STATE_PRESSED,
                             rb.GetImageNamed(IDR_CLOSE_2_P).ToImageSkia());
+
     layout->AddView(close_button_);
   }
 
@@ -141,10 +157,7 @@ void OneClickSigninDialogView::InitContent(views::GridLayout* layout) {
 
     layout->StartRow(0, COLUMN_SET_FILL_ALIGN);
 
-    learn_more_link_ = new views::Link(
-        l10n_util::GetStringUTF16(IDS_LEARN_MORE));
-    learn_more_link_->set_listener(this);
-    learn_more_link_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+    learn_more_link_ = GetLearnMoreLink();
     layout->AddView(learn_more_link_, 1, 1, views::GridLayout::TRAILING,
                     views::GridLayout::CENTER);
   }
@@ -152,6 +165,12 @@ void OneClickSigninDialogView::InitContent(views::GridLayout* layout) {
   layout->AddPaddingRow(0, views::kUnrelatedControlVerticalSpacing);
 }
 
+void OneClickSigninDialogView::InitButtons(views::GridLayout* layout) {
+  GetButtons(&ok_button_, &undo_button_);
+  layout->AddView(ok_button_);
+  layout->AddView(undo_button_);
+}
+
 void OneClickSigninDialogView::GetButtons(views::LabelButton** ok_button,
                                           views::LabelButton** undo_button) {
   *ok_button = new views::LabelButton(this, string16());
@@ -175,12 +194,13 @@ void OneClickSigninDialogView::GetButtons(views::LabelButton** ok_button,
   (*undo_button)->SetText(undo_label);
 }
 
-views::Link* OneClickSigninDialogView::GetAdvancedLink() {
-  views::Link* advanced_link= new views::Link(
-      l10n_util::GetStringUTF16(IDS_ONE_CLICK_SIGNIN_DIALOG_ADVANCED));
-  advanced_link->set_listener(this);
-  advanced_link->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  return advanced_link;
+void OneClickSigninDialogView::ButtonPressed(views::Button* sender,
+                                             const ui::Event& event) {
+  OneClickSigninBubbleView::ButtonPressed(sender, event);
+
+  base::ResetAndReturn(&start_sync_callback_).Run((sender == ok_button_) ?
+      OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS :
+      OneClickSigninSyncStarter::UNDO_SYNC);
 }
 
 void OneClickSigninDialogView::LinkClicked(views::Link* source,
@@ -193,9 +213,35 @@ void OneClickSigninDialogView::LinkClicked(views::Link* source,
     return;
   }
 
-  OneClickSigninBubbleView::LinkClicked(source, event_flags);
+  base::ResetAndReturn(&start_sync_callback_).Run(
+      OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST);
+
+  OneClickSigninBubbleView::Hide();
 }
 
+bool OneClickSigninDialogView::AcceleratorPressed(
+  const ui::Accelerator& accelerator) {
+  bool isAcceleratorPressed =
+          OneClickSigninBubbleView::AcceleratorPressed(accelerator);
+
+  if (accelerator.key_code() == ui::VKEY_RETURN) {
+      base::ResetAndReturn(&start_sync_callback_).Run(
+          OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS);
+  } else {
+    start_sync_callback_.Reset();
+  }
+
+  return isAcceleratorPressed;
+}
+
+void OneClickSigninDialogView::WindowClosing() {
+  OneClickSigninBubbleView::WindowClosing();
+
+  if (!start_sync_callback_.is_null()) {
+    base::ResetAndReturn(&start_sync_callback_).Run(
+        OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS);
+  }
+}
 }  // namespace
 
 // OneClickSigninBubbleView ----------------------------------------------------
@@ -208,14 +254,16 @@ void OneClickSigninBubbleView::ShowBubble(
     BrowserWindow::OneClickSigninBubbleType type,
     const string16& email,
     ToolbarView* toolbar_view,
-    const BrowserWindow::StartSyncCallback& start_sync) {
+    const BrowserWindow::StartSyncCallback& start_sync,
+    const std::string& error_message) {
   if (IsShowing())
     return;
 
   switch (type) {
     case BrowserWindow::ONE_CLICK_SIGNIN_BUBBLE_TYPE_BUBBLE:
-      bubble_view_ = new OneClickSigninBubbleView(toolbar_view->app_menu(),
-                                                  start_sync);
+      bubble_view_ = new OneClickSigninBubbleView(
+        toolbar_view->GetWebContents(),
+        toolbar_view->app_menu(), error_message);
       break;
     case BrowserWindow::ONE_CLICK_SIGNIN_BUBBLE_TYPE_MODAL_DIALOG:
       bubble_view_ = new OneClickSigninDialogView(
@@ -244,15 +292,17 @@ void OneClickSigninBubbleView::Hide() {
 }
 
 OneClickSigninBubbleView::OneClickSigninBubbleView(
+  content::WebContents* web_content,
     views::View* anchor_view,
-    const BrowserWindow::StartSyncCallback& start_sync_callback)
+    const std::string& error_message)
     : BubbleDelegateView(anchor_view, views::BubbleBorder::TOP_RIGHT),
+      web_content_(web_content),
+      error_message_(error_message),
       advanced_link_(NULL),
+      learn_more_link_(NULL),
       ok_button_(NULL),
       undo_button_(NULL),
-      start_sync_callback_(start_sync_callback),
       message_loop_for_testing_(NULL) {
-  DCHECK(!start_sync_callback_.is_null());
 }
 
 OneClickSigninBubbleView::~OneClickSigninBubbleView() {
@@ -289,29 +339,41 @@ void OneClickSigninBubbleView::Init() {
 
   // Add controls at the bottom.
   advanced_link_= GetAdvancedLink();
-  GetButtons(&ok_button_, &undo_button_);
-  ok_button_->SetIsDefault(true);
 
   layout->StartRow(0, COLUMN_SET_CONTROLS);
   layout->AddView(advanced_link_);
-  layout->AddView(ok_button_);
-  layout->AddView(undo_button_);
+
+  InitButtons(layout);
+  ok_button_->SetIsDefault(true);
 
   AddAccelerator(ui::Accelerator(ui::VKEY_RETURN, 0));
 }
 
 void OneClickSigninBubbleView::InitContent(views::GridLayout* layout) {
   // Add main text description.
-  views::Label* label = new views::Label(
-      l10n_util::GetStringUTF16(IDS_ONE_CLICK_SIGNIN_BUBBLE_MESSAGE));
+  layout->StartRow(0, COLUMN_SET_FILL_ALIGN);
+
+  views::Label* label = !error_message_.empty() ?
+      new views::Label(UTF8ToUTF16(error_message_)) :
+      new views::Label(
+        l10n_util::GetStringUTF16(IDS_ONE_CLICK_SIGNIN_BUBBLE_MESSAGE));
+
   label->SetMultiLine(true);
   label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   label->SizeToFit(kMinimumLabelWidth);
-
-  layout->StartRow(0, COLUMN_SET_FILL_ALIGN);
   layout->AddView(label);
 
-  layout->AddPaddingRow(0, views::kRelatedControlSmallVerticalSpacing);
+  layout->StartRow(0, COLUMN_SET_CONTROLS);
+
+  // and learn more link
+  learn_more_link_ = GetLearnMoreLink();
+
+  layout->AddView(learn_more_link_);
+}
+
+void OneClickSigninBubbleView::InitButtons(views::GridLayout* layout) {
+  GetButtons(&ok_button_, &undo_button_);
+  layout->AddView(ok_button_);
 }
 
 void OneClickSigninBubbleView::GetButtons(views::LabelButton** ok_button,
@@ -327,47 +389,33 @@ void OneClickSigninBubbleView::GetButtons(views::LabelButton** ok_button,
   // same size, SetText() is called with both strings on both buttons.
   (*ok_button)->set_min_size(gfx::Size());
   (*undo_button)->set_min_size(gfx::Size());
+
   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);
 }
 
 views::Link* OneClickSigninBubbleView::GetAdvancedLink() {
-  views::Link* advanced_link= new views::Link(
-      l10n_util::GetStringUTF16(IDS_SYNC_PROMO_NTP_BUBBLE_ADVANCED));
+  views::Link* advanced_link = new views::Link(
+      l10n_util::GetStringUTF16(IDS_ONE_CLICK_SIGNIN_DIALOG_ADVANCED));
   advanced_link->set_listener(this);
   advanced_link->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   return advanced_link;
 }
 
-void OneClickSigninBubbleView::WindowClosing() {
-  // We have to reset |bubble_view_| here, not in our destructor, because
-  // we'll be destroyed asynchronously and the shown state will be checked
-  // before then.
-  DCHECK_EQ(bubble_view_, this);
-  bubble_view_ = NULL;
+views::Link* OneClickSigninBubbleView::GetLearnMoreLink() {
+  views::Link* learn_more_link_ = new views::Link(
+        l10n_util::GetStringUTF16(IDS_LEARN_MORE));
+  learn_more_link_->set_listener(this);
+  learn_more_link_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
 
-  if (!start_sync_callback_.is_null()) {
-    base::ResetAndReturn(&start_sync_callback_).Run(
-        OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS);
-  }
+  return learn_more_link_;
 }
 
 bool OneClickSigninBubbleView::AcceleratorPressed(
-    const ui::Accelerator& accelerator) {
+  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();
-    }
-
+    OneClickSigninBubbleView::Hide();
     return true;
   }
 
@@ -376,15 +424,28 @@ bool OneClickSigninBubbleView::AcceleratorPressed(
 
 void OneClickSigninBubbleView::LinkClicked(views::Link* source,
                                            int event_flags) {
-  StartFade(false);
-  base::ResetAndReturn(&start_sync_callback_).Run(
-      OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST);
+  if (source == learn_more_link_) {
+    content::OpenURLParams params(
+        GURL(chrome::kChromeSyncLearnMoreURL), content::Referrer(),
+        NEW_FOREGROUND_TAB, content::PAGE_TRANSITION_LINK, false);
+    web_content_->OpenURL(params);
+  } else if (source == advanced_link_) {
+    content::OpenURLParams params(
+        GURL(chrome::kChromeUISettingsURL), content::Referrer(),
+        CURRENT_TAB, content::PAGE_TRANSITION_LINK, false);
+    web_content_->OpenURL(params);
+  }
 }
 
 void OneClickSigninBubbleView::ButtonPressed(views::Button* sender,
                                              const ui::Event& event) {
-  StartFade(false);
-  base::ResetAndReturn(&start_sync_callback_).Run((sender == ok_button_) ?
-      OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS :
-      OneClickSigninSyncStarter::UNDO_SYNC);
+  OneClickSigninBubbleView::Hide();
+}
+
+void OneClickSigninBubbleView::WindowClosing() {
+  // We have to reset |bubble_view_| here, not in our destructor, because
+  // we'll be destroyed asynchronously and the shown state will be checked
+  // before then.
+  DCHECK_EQ(bubble_view_, this);
+  bubble_view_ = NULL;
 }