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

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

 // 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/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"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/url_constants.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "ui/base/keycodes/keyboard_codes.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/views/controls/button/text_button.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/link.h"
 #include "ui/views/events/event.h"
 #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;
 
+// 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
 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();
 }
 
 // static
 bool OneClickSigninBubbleView::IsShowing() {
   return bubble_view_ != NULL;
 }
 
 // static
 void OneClickSigninBubbleView::Hide() {
   if (IsShowing())
     bubble_view_->GetWidget()->Close();
 }
 
 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() {
 }
 
 void OneClickSigninBubbleView::AnimationEnded(const ui::Animation* animation) {
   views::BubbleDelegateView::AnimationEnded(animation);
   if (message_loop_for_testing_)
     message_loop_for_testing_->Quit();
 }
@@ skipping 11 matching lines @@
   // 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);
 
   cs = layout->AddColumnSet(kColumnSetControls);
   cs->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER, 0,
                 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);
 
   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(
       l10n_util::GetStringUTF16(IDS_SYNC_PROMO_NTP_BUBBLE_ADVANCED));
   advanced_link_->set_listener(this);
   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));
 }
 
 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(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;
   }
 
   return BubbleDelegateView::AcceleratorPressed(accelerator);
 }
 
 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();
+  }
 }