Add the error dialog for migration flow

It is shown when the response contained api error.
This prevents user from deleting those cards when migration
failure is probably due to server error.

Uploaded screenshot in bug comment#18

Bug: 897998
Change-Id: I828c6560f39e5383c25822631ed15fbc47e05222
Reviewed-on: https://chromium-review.googlesource.com/c/1332410
Commit-Queue: Siyu An <siyua@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Jared Saul <jsaul@google.com>
Cr-Commit-Position: refs/heads/master@{#611365}

chrome/browser/ui/BUILD.gn

@@ -2347,6 +2347,8 @@ jumbo_split_static_library("ui") {
       "views/autofill/local_card_migration_bubble_views.h",
       "views/autofill/local_card_migration_dialog_view.cc",
       "views/autofill/local_card_migration_dialog_view.h",
+      "views/autofill/local_card_migration_error_dialog_view.cc",
+      "views/autofill/local_card_migration_error_dialog_view.h",
       "views/autofill/local_card_migration_icon_view.cc",
       "views/autofill/local_card_migration_icon_view.h",
       "views/autofill/migratable_card_view.cc",

chrome/browser/ui/autofill/local_card_migration_dialog_controller_impl.cc

@@ -100,6 +100,13 @@ void LocalCardMigrationDialogControllerImpl::ShowFeedbackDialog() {
   local_card_migration_dialog_->ShowDialog();
 }
 
+void LocalCardMigrationDialogControllerImpl::ShowErrorDialog() {
+  local_card_migration_dialog_ =
+      CreateLocalCardMigrationErrorDialogView(this, web_contents());
+  UpdateIcon();
+  local_card_migration_dialog_->ShowDialog();
+}
+
 LocalCardMigrationDialogState
 LocalCardMigrationDialogControllerImpl::GetViewState() const {
   return view_state_;

chrome/browser/ui/autofill/local_card_migration_dialog_controller_impl.h

@@ -35,16 +35,22 @@ class LocalCardMigrationDialogControllerImpl
       AutofillClient::LocalCardMigrationCallback
           start_migrating_cards_callback);
 
-  // When migration is finished, show a credit card icon in the omnibox.
+  // When migration is finished, show a credit card icon in the omnibox. Also
+  // passes |tip_message|, and |migratable_credit_cards| to controller.
   void ShowCreditCardIcon(
       const base::string16& tip_message,
       const std::vector<MigratableCreditCard>& migratable_credit_cards);
 
-  // If the user clicks on the credit card icon in the omnibox, we show
-  // the feedback dialog containing the uploading results of the cards that
-  // the user selected to upload.
+  // If the user clicks on the credit card icon in the omnibox, we show the
+  // feedback dialog containing the uploading results of the cards that the
+  // user selected to upload.
   void ShowFeedbackDialog();
 
+  // If the user clicks on the credit card icon in the omnibox after the
+  // migration request failed due to some internal server errors, we show the
+  // error dialog containing an error message.
+  void ShowErrorDialog();
+
   // LocalCardMigrationDialogController:
   LocalCardMigrationDialogState GetViewState() const override;
   const std::vector<MigratableCreditCard>& GetCardList() const override;

chrome/browser/ui/autofill/local_card_migration_dialog_factory.h

@@ -18,6 +18,10 @@ LocalCardMigrationDialog* CreateLocalCardMigrationDialogView(
     LocalCardMigrationDialogController* controller,
     content::WebContents* web_contents);
 
+LocalCardMigrationDialog* CreateLocalCardMigrationErrorDialogView(
+    LocalCardMigrationDialogController* controller,
+    content::WebContents* web_contents);
+
 }  // namespace autofill
 
 #endif  // CHROME_BROWSER_UI_AUTOFILL_LOCAL_CARD_MIGRATION_DIALOG_FACTORY_H_

chrome/browser/ui/autofill/manage_migration_ui_controller.cc

@@ -6,6 +6,7 @@
 
 #include "chrome/browser/ui/autofill/local_card_migration_bubble.h"
 #include "chrome/browser/ui/autofill/local_card_migration_dialog.h"
+#include "components/autofill/core/browser/local_card_migration_manager.h"
 
 namespace autofill {
 
@@ -50,6 +51,9 @@ void ManageMigrationUiController::ShowCreditCardIcon(
 
   DCHECK_EQ(flow_step_, LocalCardMigrationFlowStep::OFFER_DIALOG);
   flow_step_ = LocalCardMigrationFlowStep::CREDIT_CARD_ICON;
+  // Show error dialog when the vector is an empty vector, which indicates
+  // Payments Rpc failure.
+  show_error_dialog_ = migratable_credit_cards.empty();
   dialog_controller_->ShowCreditCardIcon(tip_message, migratable_credit_cards);
 }
 
@@ -60,7 +64,7 @@ void ManageMigrationUiController::OnUserClickingCreditCardIcon() {
       break;
     }
     case LocalCardMigrationFlowStep::CREDIT_CARD_ICON: {
-      ShowFeedbackDialog();
+      show_error_dialog_ ? ShowErrorDialog() : ShowFeedbackDialog();
       break;
     }
     default: {
@@ -101,6 +105,15 @@ void ManageMigrationUiController::ReshowBubble() {
   bubble_controller_->ReshowBubble();
 }
 
+void ManageMigrationUiController::ShowErrorDialog() {
+  if (!dialog_controller_)
+    return;
+
+  DCHECK_EQ(flow_step_, LocalCardMigrationFlowStep::CREDIT_CARD_ICON);
+  flow_step_ = LocalCardMigrationFlowStep::ERROR_DIALOG;
+  dialog_controller_->ShowErrorDialog();
+}
+
 void ManageMigrationUiController::ShowFeedbackDialog() {
   if (!dialog_controller_)
     return;

chrome/browser/ui/autofill/manage_migration_ui_controller.h

@@ -28,9 +28,13 @@ enum class LocalCardMigrationFlowStep {
   // Should show the credit card icon when migration is finished and the
   // feedback dialog is ready.
   CREDIT_CARD_ICON,
-  // Should show the feedback dialog dialog after the user clicking the
-  // credit card.
+  // Should show the feedback dialog containing the migration results of cards
+  // that the user selected to upload after the user clicking the credit card
+  // icon.
   FEEDBACK_DIALOG,
+  // Should show the error dialog if the Payments Rpc request failed after the
+  // user clicks the credit card icon.
+  ERROR_DIALOG,
 };
 
 // Controller controls the step of migration flow and is responsible
@@ -71,6 +75,8 @@ class ManageMigrationUiController
 
   void ReshowBubble();
 
+  void ShowErrorDialog();
+
   void ShowFeedbackDialog();
 
   LocalCardMigrationBubbleControllerImpl* bubble_controller_ = nullptr;
@@ -80,6 +86,10 @@ class ManageMigrationUiController
   // what should be shown next.
   LocalCardMigrationFlowStep flow_step_ = LocalCardMigrationFlowStep::NOT_SHOWN;
 
+  // This indicates if we should show error dialog or normal feedback dialog
+  // after users click the credit card icon.
+  bool show_error_dialog_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(ManageMigrationUiController);
 };
 

chrome/browser/ui/autofill/popup_constants.h

@@ -7,6 +7,7 @@
 
 #include "base/time/time.h"
 #include "build/build_config.h"
+#include "ui/gfx/geometry/insets.h"
 
 namespace autofill {
 
@@ -18,6 +19,9 @@ const int kPopupBorderThickness = 1;
 const int kPopupBorderThickness = 0;
 #endif
 
+constexpr int kMigrationDialogMainContainerChildSpacing = 24;
+constexpr gfx::Insets kMigrationDialogInsets = gfx::Insets(0, 24, 48, 24);
+
 // The time span a card bubble should be visible even if the document
 // navigates away meanwhile. This is to ensure that the user can see
 // the bubble.

chrome/browser/ui/views/autofill/local_card_migration_dialog_view.cc

@@ -7,8 +7,10 @@
 #include "base/location.h"
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/app/vector_icons/vector_icons.h"
 #include "chrome/browser/ui/autofill/local_card_migration_dialog_factory.h"
 #include "chrome/browser/ui/autofill/local_card_migration_dialog_state.h"
+#include "chrome/browser/ui/autofill/popup_constants.h"
 #include "chrome/browser/ui/views/autofill/migratable_card_view.h"
 #include "chrome/browser/ui/views/autofill/view_util.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
@@ -46,9 +48,6 @@ namespace autofill {
 
 namespace {
 
-constexpr int kMainContainerChildSpacing = 24;
-constexpr gfx::Insets kMigrationDialogInsets = gfx::Insets(0, 24, 48, 24);
-
 // Create the title label container for the migration dialogs. The title
 // text depends on the |view_state| of the dialog.
 std::unique_ptr<views::Label> CreateTitle(
@@ -220,7 +219,7 @@ class LocalCardMigrationOfferView : public views::View,
     ChromeLayoutProvider* provider = ChromeLayoutProvider::Get();
     SetLayoutManager(std::make_unique<views::BoxLayout>(
         views::BoxLayout::kVertical, gfx::Insets(),
-        kMainContainerChildSpacing));
+        kMigrationDialogMainContainerChildSpacing));
 
     auto* contents_container = new views::View();
     contents_container->SetLayoutManager(std::make_unique<views::BoxLayout>(
@@ -391,7 +390,8 @@ void LocalCardMigrationDialogView::Init() {
     return;
 
   SetLayoutManager(std::make_unique<views::BoxLayout>(
-      views::BoxLayout::kVertical, gfx::Insets(), kMainContainerChildSpacing));
+      views::BoxLayout::kVertical, gfx::Insets(),
+      kMigrationDialogMainContainerChildSpacing));
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
 
   auto* image = new views::ImageView();
@@ -402,9 +402,10 @@ void LocalCardMigrationDialogView::Init() {
       l10n_util::GetStringUTF16(IDS_AUTOFILL_GOOGLE_PAY_LOGO_ACCESSIBLE_NAME));
   AddChildView(image);
 
-  AddChildView(CreateTitle(controller_->GetViewState()).release());
+  LocalCardMigrationDialogState view_state = controller_->GetViewState();
+  AddChildView(CreateTitle(view_state).release());
 
-  if (controller_->GetViewState() == LocalCardMigrationDialogState::kOffered) {
+  if (view_state == LocalCardMigrationDialogState::kOffered) {
     offer_view_ = new LocalCardMigrationOfferView(controller_, this);
     AddChildView(offer_view_);
   } else {

chrome/browser/ui/views/autofill/local_card_migration_error_dialog_view.cc

+// Copyright 2018 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/autofill/local_card_migration_error_dialog_view.h"
+
+#include "base/macros.h"
+#include "chrome/app/vector_icons/vector_icons.h"
+#include "chrome/browser/ui/autofill/local_card_migration_dialog_factory.h"
+#include "chrome/browser/ui/autofill/popup_constants.h"
+#include "chrome/browser/ui/views/chrome_layout_provider.h"
+#include "chrome/browser/ui/views/chrome_typography.h"
+#include "components/autofill/core/browser/local_card_migration_manager.h"
+#include "components/constrained_window/constrained_window_views.h"
+#include "components/grit/components_scaled_resources.h"
+#include "components/strings/grit/components_strings.h"
+#include "components/web_modal/web_contents_modal_dialog_host.h"
+#include "components/web_modal/web_contents_modal_dialog_manager.h"
+#include "components/web_modal/web_contents_modal_dialog_manager_delegate.h"
+#include "third_party/skia/include/core/SkColor.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/geometry/insets.h"
+#include "ui/gfx/paint_vector_icon.h"
+#include "ui/views/controls/image_view.h"
+#include "ui/views/controls/label.h"
+#include "ui/views/layout/box_layout.h"
+#include "ui/views/style/typography.h"
+#include "ui/views/widget/widget.h"
+#include "ui/views/window/dialog_client_view.h"
+
+namespace autofill {
+
+LocalCardMigrationErrorDialogView::LocalCardMigrationErrorDialogView(
+    LocalCardMigrationDialogController* controller,
+    content::WebContents* web_contents)
+    : controller_(controller), web_contents_(web_contents) {
+  set_close_on_deactivate(false);
+  set_margins(gfx::Insets());
+}
+
+LocalCardMigrationErrorDialogView::~LocalCardMigrationErrorDialogView() {}
+
+void LocalCardMigrationErrorDialogView::ShowDialog() {
+  Init();
+  constrained_window::ShowWebModalDialogViews(this, web_contents_);
+}
+
+void LocalCardMigrationErrorDialogView::CloseDialog() {
+  GetWidget()->Close();
+}
+
+gfx::Size LocalCardMigrationErrorDialogView::CalculatePreferredSize() const {
+  const int width = ChromeLayoutProvider::Get()->GetDistanceMetric(
+                        DISTANCE_LARGE_MODAL_DIALOG_PREFERRED_WIDTH) -
+                    margins().width();
+  return gfx::Size(width, GetHeightForWidth(width));
+}
+
+ui::ModalType LocalCardMigrationErrorDialogView::GetModalType() const {
+  // The error dialog should be a modal dialog which is consistent with other
+  // dialogs. It should make sure that the user can see the error message.
+  return ui::MODAL_TYPE_CHILD;
+}
+
+bool LocalCardMigrationErrorDialogView::ShouldShowCloseButton() const {
+  return false;
+}
+
+int LocalCardMigrationErrorDialogView::GetDialogButtons() const {
+  return ui::DIALOG_BUTTON_CANCEL;
+}
+
+void LocalCardMigrationErrorDialogView::WindowClosing() {
+  if (controller_) {
+    controller_->OnDialogClosed();
+    controller_ = nullptr;
+  }
+}
+
+void LocalCardMigrationErrorDialogView::Init() {
+  if (has_children())
+    return;
+
+  ChromeLayoutProvider* provider = ChromeLayoutProvider::Get();
+  SetLayoutManager(std::make_unique<views::BoxLayout>(
+      views::BoxLayout::kVertical, gfx::Insets(),
+      kMigrationDialogMainContainerChildSpacing));
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+
+  auto* image = new views::ImageView();
+  image->SetImage(rb.GetImageSkiaNamed(IDR_AUTOFILL_MIGRATION_DIALOG_HEADER));
+  image->SetAccessibleName(
+      l10n_util::GetStringUTF16(IDS_AUTOFILL_GOOGLE_PAY_LOGO_ACCESSIBLE_NAME));
+  AddChildView(image);
+
+  auto* error_view = new views::View();
+  auto* horizontal_layout =
+      error_view->SetLayoutManager(std::make_unique<views::BoxLayout>(
+          views::BoxLayout::kHorizontal, gfx::Insets(),
+          provider->GetDistanceMetric(
+              views::DISTANCE_UNRELATED_CONTROL_VERTICAL)));
+  horizontal_layout->set_main_axis_alignment(
+      views::BoxLayout::MAIN_AXIS_ALIGNMENT_CENTER);
+  error_view->SetBorder(views::CreateEmptyBorder(kMigrationDialogInsets));
+  auto* error_image = new views::ImageView();
+  error_image->SetImage(
+      gfx::CreateVectorIcon(kBrowserToolsErrorIcon, gfx::kGoogleRed700));
+  error_view->AddChildView(error_image);
+
+  auto* error_message = new views::Label(
+      l10n_util::GetPluralStringFUTF16(
+          IDS_AUTOFILL_LOCAL_CARD_MIGRATION_DIALOG_MESSAGE_ERROR,
+          controller_->GetCardList().size()),
+      CONTEXT_BODY_TEXT_LARGE, ChromeTextStyle::STYLE_RED);
+  error_view->AddChildView(error_message);
+
+  AddChildView(error_view);
+}
+
+LocalCardMigrationDialog* CreateLocalCardMigrationErrorDialogView(
+    LocalCardMigrationDialogController* controller,
+    content::WebContents* web_contents) {
+  return new LocalCardMigrationErrorDialogView(controller, web_contents);
+}
+
+}  // namespace autofill

chrome/browser/ui/views/autofill/local_card_migration_error_dialog_view.h

+// Copyright 2018 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.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_AUTOFILL_LOCAL_CARD_MIGRATION_ERROR_DIALOG_VIEW_H_
+#define CHROME_BROWSER_UI_VIEWS_AUTOFILL_LOCAL_CARD_MIGRATION_ERROR_DIALOG_VIEW_H_
+
+#include "base/macros.h"
+#include "chrome/browser/ui/autofill/local_card_migration_dialog.h"
+#include "chrome/browser/ui/views/autofill/dialog_view_ids.h"
+#include "components/autofill/core/browser/ui/local_card_migration_dialog_controller.h"
+#include "ui/views/bubble/bubble_dialog_delegate_view.h"
+#include "ui/views/controls/button/button.h"
+#include "ui/views/view.h"
+#include "ui/views/window/dialog_delegate.h"
+
+namespace content {
+class WebContents;
+}  // namespace content
+
+namespace autofill {
+
+class LocalCardMigrationErrorDialogView
+    : public LocalCardMigrationDialog,
+      public views::BubbleDialogDelegateView {
+ public:
+  LocalCardMigrationErrorDialogView(
+      LocalCardMigrationDialogController* controller,
+      content::WebContents* web_contents);
+  ~LocalCardMigrationErrorDialogView() override;
+
+  // LocalCardMigrationDialog
+  void ShowDialog() override;
+  void CloseDialog() override;
+
+  // views::BubbleDialogDelegateView
+  gfx::Size CalculatePreferredSize() const override;
+  ui::ModalType GetModalType() const override;
+  bool ShouldShowCloseButton() const override;
+  int GetDialogButtons() const override;
+  void Init() override;
+  void WindowClosing() override;
+
+ private:
+  LocalCardMigrationDialogController* controller_;
+
+  content::WebContents* web_contents_;
+
+  DISALLOW_COPY_AND_ASSIGN(LocalCardMigrationErrorDialogView);
+};
+
+}  // namespace autofill
+
+#endif  // CHROME_BROWSER_UI_VIEWS_AUTOFILL_LOCAL_CARD_MIGRATION_ERROR_DIALOG_VIEW_H_

components/autofill/core/browser/autofill_client.h

@@ -191,8 +191,11 @@ class AutofillClient : public RiskDataLoader {
       const std::vector<MigratableCreditCard>& migratable_credit_cards,
       LocalCardMigrationCallback start_migrating_cards_callback) = 0;
 
-  // Will show a dialog with all the selected card migration results. Invoked
-  // when the migration process is finished.
+  // Will show a dialog containing a error message if |migratable_credit_cards|
+  // is an empty vector, or the migration results for cards in
+  // |migratable_credit_cards| otherwise. If migration succeeds the dialog will
+  // contain a |tip_message|. The dialog is invoked when the migration process
+  // is finished.
   virtual void ShowLocalCardMigrationResults(
       const base::string16& tip_message,
       const std::vector<MigratableCreditCard>& migratable_credit_cards) = 0;

components/autofill/core/browser/local_card_migration_manager.cc

@@ -190,37 +190,43 @@ void LocalCardMigrationManager::OnDidMigrateLocalCards(
   if (!save_result)
     return;
 
-  std::vector<CreditCard> migrated_cards;
-  // Traverse the migratable credit cards to update each migrated card status.
-  for (MigratableCreditCard& card : migratable_credit_cards_) {
-    // Not every card exists in the |save_result| since some cards are unchecked
-    // by the user and not migrated.
-    auto it = save_result->find(card.credit_card().guid());
-    // If current card exists in the |save_result|, update its migration status.
-    if (it != save_result->end()) {
-      // Server-side response can return SUCCESS, TEMPORARY_FAILURE, or
-      // PERMANENT_FAILURE (see SaveResult enum). Branch here depending on which
-      // is received.
-      if (it->second == kMigrationResultPermanentFailure ||
-          it->second == kMigrationResultTemporaryFailure) {
-        card.set_migration_status(
-            autofill::MigratableCreditCard::MigrationStatus::FAILURE_ON_UPLOAD);
-      } else if (it->second == kMigrationResultSuccess) {
-        card.set_migration_status(
-            autofill::MigratableCreditCard::MigrationStatus::SUCCESS_ON_UPLOAD);
-        migrated_cards.push_back(card.credit_card());
-      } else {
-        NOTREACHED();
+  if (result == AutofillClient::PaymentsRpcResult::SUCCESS) {
+    std::vector<CreditCard> migrated_cards;
+    // Traverse the migratable credit cards to update each migrated card status.
+    for (MigratableCreditCard& card : migratable_credit_cards_) {
+      // Not every card exists in the |save_result| since some cards are
+      // unchecked by the user and not migrated.
+      auto it = save_result->find(card.credit_card().guid());
+      // If current card exists in the |save_result|, update its migration
+      // status.
+      if (it != save_result->end()) {
+        // Server-side response can return SUCCESS, TEMPORARY_FAILURE, or
+        // PERMANENT_FAILURE (see SaveResult enum). Branch here depending on
+        // which is received.
+        if (it->second == kMigrationResultPermanentFailure ||
+            it->second == kMigrationResultTemporaryFailure) {
+          card.set_migration_status(autofill::MigratableCreditCard::
+                                        MigrationStatus::FAILURE_ON_UPLOAD);
+        } else if (it->second == kMigrationResultSuccess) {
+          card.set_migration_status(autofill::MigratableCreditCard::
+                                        MigrationStatus::SUCCESS_ON_UPLOAD);
+          migrated_cards.push_back(card.credit_card());
+        } else {
+          NOTREACHED();
+        }
       }
     }
+    // Remove cards that were successfully migrated from local storage.
+    personal_data_manager_->DeleteLocalCreditCards(migrated_cards);
   }
-  // Remove cards that were successfully migrated from local storage.
-  personal_data_manager_->DeleteLocalCreditCards(migrated_cards);
 
   if (base::FeatureList::IsEnabled(
           features::kAutofillLocalCardMigrationShowFeedback)) {
-    client_->ShowLocalCardMigrationResults(base::UTF8ToUTF16(display_text),
-                                           migratable_credit_cards_);
+    client_->ShowLocalCardMigrationResults(
+        base::UTF8ToUTF16(display_text),
+        result == AutofillClient::PaymentsRpcResult::SUCCESS
+            ? migratable_credit_cards_
+            : std::vector<MigratableCreditCard>());
   }
 }
 

components/autofill_strings.grdp

@@ -290,6 +290,11 @@
       =1 {This card has been added to your Google Account. You can now use it from any signed-in device.}
       other {These cards have been added to your Google Account. You can now use them from any signed-in device.}}
     </message>
+    <message name="IDS_AUTOFILL_LOCAL_CARD_MIGRATION_DIALOG_MESSAGE_ERROR" desc="The body text shown on a dialog when the user attempts to migrate their cards to Google Payments, but all migrations fail. [ICU Syntax]">
+      {NUM_CARDS, plural,
+      =1 {This card can't be added right now}
+      other {These cards can't be added right now}}
+    </message>
     <message name="IDS_AUTOFILL_LOCAL_CARD_MIGRATION_DIALOG_MESSAGE_FIX" desc="The body text shown on a dialog after the user accepts another dialog that offers to migrate local cards into the cloud but there are errors with the cards that they need to address.">
       Check the info below and delete any invalid cards
     </message>

components/autofill_strings_grdp/IDS_AUTOFILL_LOCAL_CARD_MIGRATION_DIALOG_MESSAGE_ERROR.png.sha1

+f38f9fd1383bd9d1864b5fbc1b5202c126b0b316
\ No newline at end of file