Update behavior of one-click infobar to remove modal dialog, add "undo".

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 0c2281d6419510444472a1f5bf8bd00f4e1dc85a..072b53d7c2d261d8aba219d3aad88bc0e718f510 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"
@@ -27,6 +28,11 @@
 // of the field's left edge.
 const int kMinimumFieldSize = 240;
 
+// Button width.  The default width of NativeTextButtons is too large for this
+// bubble.
+const int kButtonWidth = 64;

[Peter Kasting, 2012/05/24 22:25:18]

Hardcoding this makes me really nervous for both l10n and accessibility/theming
reasons (what if someone has large fonts?).

I'm confused why we need this anyway.  You can just call
set_ignore_minimum_size(false) on the button after you create it to allow
narrower buttons.  Then you can make both buttons the same size by calling
SetText() with both strings on each (just in opposite orders).

[Roger Tawa OOO till Jul 10th, 2012/05/25 16:04:08]

set_ignore_minimum_size() seems to be ignored, but in the interest of better
support for l10n and such I think this is better.  I also used spacing constants
instead of hard coding values.  You can see the before and after screenshots of
the bubble here:

www/~rogerta/no_crawl/signin/bubble-before.gif
www/~rogerta/no_crawl/signin/bubble-after.gif

+
+
 // BookmarkBubbleView ---------------------------------------------------------
 
 // static
@@ -35,14 +41,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 +64,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() {
@@ -102,12 +103,14 @@ void OneClickSigninBubbleView::Init() {
                 views::GridLayout::USE_PREF, 0, 0);
   cs->AddPaddingColumn(1, 0);
   cs->AddColumn(views::GridLayout::TRAILING, views::GridLayout::CENTER, 0,
-                views::GridLayout::USE_PREF, 0, 0);
+                views::GridLayout::FIXED, kButtonWidth, 0);
+  cs->AddPaddingColumn(0, 5);
+  cs->AddColumn(views::GridLayout::TRAILING, views::GridLayout::CENTER, 0,
+                views::GridLayout::FIXED, kButtonWidth, 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);
@@ -115,16 +118,7 @@ void OneClickSigninBubbleView::Init() {
   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);
+  layout->AddPaddingRow(0, views::kLabelToControlVerticalSpacing);

[Peter Kasting, 2012/05/24 22:25:18]

This doesn't seem like the right value.  This is for when you're putting a
control below a label that describes it.

[Roger Tawa OOO till Jul 10th, 2012/05/25 16:04:08]

Seems like the right one to me.  Which value do you think is better?

[Peter Kasting, 2012/05/25 16:58:06]

This is for a case like:

  BLAH_FOO option:
    [ ] Yes
    [ ] No

It's the spacing between the descriptive header and the objects it describes.
Based on other bubbles, the existing kRelatedControlSmallVerticalSpacing was
correct.

[Roger Tawa OOO till Jul 10th, 2012/05/25 18:49:51]

Done.

 
   // Add link for user to do advanced config of sync.
   advanced_link_= new views::Link(
@@ -133,13 +127,17 @@ void OneClickSigninBubbleView::Init() {
   advanced_link_->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
 
   // Add controls at the bottom.
-  close_button_ = new views::NativeTextButton(
+  ok_button_ = new views::NativeTextButton(
       this, l10n_util::GetStringUTF16(IDS_OK));
-  close_button_->SetIsDefault(true);
+  ok_button_->SetIsDefault(true);
+
+  undo_button_ = new views::NativeTextButton(
+      this, l10n_util::GetStringUTF16(IDS_ONE_CLICK_BUBBLE_UNDO));
 
   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 +148,24 @@ void OneClickSigninBubbleView::WindowClosing() {
   // before then.
   DCHECK(bubble_view_ == this);
   bubble_view_ = NULL;
- }
+
+  if (!start_sync_callback_.is_null())

[Peter Kasting, 2012/05/24 22:25:18]

Nit: Need {}

[Roger Tawa OOO till Jul 10th, 2012/05/25 16:04:08]

Done.

+    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 +175,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();
+  }
 }