Autofill: Replace "save credit card" infobar with a bubble (Views only).

chrome/browser/ui/views/location_bar/location_bar_view.cc

Index: chrome/browser/ui/views/location_bar/location_bar_view.cc
diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.cc b/chrome/browser/ui/views/location_bar/location_bar_view.cc
index 02c1de32f88b077a8ad2ffd5585e8fdcceed6496..a563a16b979a4e6138fe0b212f00cd786660a400 100644
--- a/chrome/browser/ui/views/location_bar/location_bar_view.cc
+++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc
@@ -27,6 +27,7 @@
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/translate/chrome_translate_client.h"
 #include "chrome/browser/translate/translate_service.h"
+#include "chrome/browser/ui/autofill/chrome_autofill_client.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_instant_controller.h"
@@ -36,6 +37,7 @@
 #include "chrome/browser/ui/passwords/manage_passwords_ui_controller.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/view_ids.h"
+#include "chrome/browser/ui/views/autofill/save_card_bubble_view.h"
 #include "chrome/browser/ui/views/browser_dialogs.h"
 #include "chrome/browser/ui/views/layout_constants.h"
 #include "chrome/browser/ui/views/location_bar/content_setting_image_view.h"
@@ -46,6 +48,7 @@
 #include "chrome/browser/ui/views/location_bar/open_pdf_in_reader_view.h"
 #include "chrome/browser/ui/views/location_bar/page_action_image_view.h"
 #include "chrome/browser/ui/views/location_bar/page_action_with_badge_view.h"
+#include "chrome/browser/ui/views/location_bar/save_credit_card_icon_view.h"
 #include "chrome/browser/ui/views/location_bar/selected_keyword_view.h"
 #include "chrome/browser/ui/views/location_bar/star_view.h"
 #include "chrome/browser/ui/views/location_bar/translate_icon_view.h"
@@ -144,6 +147,7 @@ LocationBarView::LocationBarView(Browser* browser,
       zoom_view_(NULL),
       open_pdf_in_reader_view_(NULL),
       manage_passwords_icon_view_(NULL),
+      save_credit_card_icon_view_(NULL),
       translate_icon_view_(NULL),
       star_view_(NULL),
       size_animation_(this),
@@ -301,6 +305,10 @@ void LocationBarView::Init() {
   manage_passwords_icon_view_ = new ManagePasswordsIconView(command_updater());
   AddChildView(manage_passwords_icon_view_);
 
+  save_credit_card_icon_view_ = new SaveCreditCardIconView(command_updater());
+  save_credit_card_icon_view_->SetVisible(false);
+  AddChildView(save_credit_card_icon_view_);
+
   translate_icon_view_ = new TranslateIconView(command_updater());
   translate_icon_view_->SetVisible(false);
   AddChildView(translate_icon_view_);
@@ -561,6 +569,7 @@ gfx::Size LocationBarView::GetPreferredSize() const {
   trailing_width += IncrementalMinimumWidth(star_view_) +
       IncrementalMinimumWidth(translate_icon_view_) +
       IncrementalMinimumWidth(open_pdf_in_reader_view_) +
+      IncrementalMinimumWidth(save_credit_card_icon_view_) +
       IncrementalMinimumWidth(manage_passwords_icon_view_) +
       IncrementalMinimumWidth(zoom_view_) +
       IncrementalMinimumWidth(mic_search_view_);
@@ -659,6 +668,10 @@ void LocationBarView::Layout() {
     trailing_decorations.AddDecoration(vertical_padding, location_height,
                                        open_pdf_in_reader_view_);
   }
+  if (save_credit_card_icon_view_->visible()) {
+    trailing_decorations.AddDecoration(vertical_padding, location_height,
+                                       save_credit_card_icon_view_);
+  }
   if (manage_passwords_icon_view_->visible()) {
     trailing_decorations.AddDecoration(vertical_padding, location_height,
                                        manage_passwords_icon_view_);
@@ -804,6 +817,7 @@ void LocationBarView::Update(const WebContents* contents) {
   RefreshZoomView();
   RefreshPageActionViews();
   RefreshTranslateIcon();
+  RefreshSaveCreditCardIconView();
   RefreshManagePasswordsIconView();
   content::WebContents* web_contents_for_sub_views =
       GetToolbarModel()->input_in_progress() ? NULL : GetWebContents();
@@ -974,6 +988,24 @@ void LocationBarView::OnDefaultZoomLevelChanged() {
   RefreshZoomView();
 }
 
+bool LocationBarView::RefreshSaveCreditCardIconView() {
+  WebContents* web_contents = GetWebContents();
+  if (!web_contents)
+    return false;
+  const bool was_visible = save_credit_card_icon_view_->visible();
+  autofill::ChromeAutofillClient* autofill_client =
+      autofill::ChromeAutofillClient::FromWebContents(web_contents);
+  bool enabled = autofill_client->WantSaveCreditCardIconVisible();

[Justin Donnelly, 2015/10/13 16:25:53]

I'm not in love with the names of these Want methods. It seems to me that it
puts us in a state where neither side of this relationship really knows what
that means. The location bar just knows that there's some undefined desire while
ChromeAutofillClient doesn't have a good grasp on what toggled means. (It's
clear in the code here but when I first read ChromeAutofillClient it seemed like
a poor naming choice.)

I'd prefer the same approach used by the translate icon code below, which deals
in concrete states. In other words, IsBubbleActive() and IsBubbleVisible(). From
ChromeAutofillClient's point of view it makes sense that other views would be
interested in these states. From here, it's clear that we want the icon visible
when the bubble is available and the icon on ("toggled") when the bubble is
visible.

[bondd, 2015/10/15 22:27:08]

Done. Thanks. This is another case where I was going around and around and
figured I could really use someone else's opinion.

+  command_updater()->UpdateCommandEnabled(IDC_SAVE_CREDIT_CARD_FOR_PAGE,
+                                          enabled);
+  save_credit_card_icon_view_->SetVisible(enabled);
+  save_credit_card_icon_view_->SetToggled(
+      autofill_client->WantSaveCreditCardIconToggled());
+  if (!enabled)
+    autofill::SaveCardBubbleView::CloseBubble();

[Justin Donnelly, 2015/10/13 16:25:53]

AutofillClient already has the ShowSaveCreditCardBubble, how about adding a
CloseSaveCreditCardBubble to go with it and then there's one less interface this
class needs to know about.

[bondd, 2015/10/15 22:27:08]

Moved this Close() call to the controller (in DidNavigateMainFrame()), so
doesn't need to be done here anymore.

+  return was_visible != save_credit_card_icon_view_->visible();
+}
+
 void LocationBarView::RefreshTranslateIcon() {
   if (!TranslateService::IsTranslateBubbleEnabled())
     return;
@@ -1082,6 +1114,13 @@ void LocationBarView::UpdateManagePasswordsIconAndBubble() {
   }
 }
 
+void LocationBarView::UpdateSaveCreditCardIcon() {
+  if (RefreshSaveCreditCardIconView()) {
+    Layout();
+    SchedulePaint();
+  }
+}
+
 void LocationBarView::UpdatePageActions() {
   if (RefreshPageActionViews()) {  // Changed.
     Layout();