Revert "[Autofill] Added browser tests for local card migration dialog."

This reverts commit 656c20b3.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 607881 as the
culprit for flakes in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vNjU2YzIwYjNhYWE0OGRiOTM4ZDYxNmMxZjQ2YjQ1ZGQ1NGMyMDZjMAw

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-rel/15973

Sample Failed Step: single_process_mash_browser_tests

Sample Flaky Test: LocalCardMigrationBrowserTest.ClickingSaveClosesDialog

Original change's description:
> [Autofill] Added browser tests for local card migration dialog.
> 
> Relanding browser tests for local migration flow. Commented out flaky tests that were causing issues earlier.
> 
> Bug: 897998
> Change-Id: I7dd0fdb5f71587bddc236732c6d3725dc6411818
> Reviewed-on: https://chromium-review.googlesource.com/c/1332704
> Reviewed-by: Evan Stade <estade@chromium.org>
> Commit-Queue: Manas Verma <manasverma@google.com>
> Cr-Commit-Position: refs/heads/master@{#607881}

Change-Id: I128c446c52bd22b7c1957b69307d4768b5b39166
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 897998
Reviewed-on: https://chromium-review.googlesource.com/c/1335277
Cr-Commit-Position: refs/heads/master@{#607917}

chrome/browser/ui/autofill/local_card_migration_dialog_controller_impl.h

@@ -6,7 +6,6 @@
 #define CHROME_BROWSER_UI_AUTOFILL_LOCAL_CARD_MIGRATION_DIALOG_CONTROLLER_IMPL_H_
 
 #include <memory>
-#include <vector>
 
 #include "base/macros.h"
 #include "base/timer/elapsed_timer.h"
@@ -54,11 +53,6 @@ class LocalCardMigrationDialogControllerImpl
   void OnLegalMessageLinkClicked(const GURL& url) override;
   void OnDialogClosed() override;
 
-  // Returns nullptr if no dialog is currently shown.
-  LocalCardMigrationDialog* local_card_migration_dialog_view() const {
-    return local_card_migration_dialog_;
-  }
-
  protected:
   explicit LocalCardMigrationDialogControllerImpl(
       content::WebContents* web_contents);

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

@@ -8,21 +8,17 @@
 #include "components/autofill/core/browser/field_types.h"
 
 // This defines an enumeration of IDs that can uniquely identify a view within
-// the scope of the local and upload credit card save bubbles as well as the
-// local card migration bubble and dialogs.
+// the scope of the local and upload credit card save bubbles.
 
 namespace autofill {
 
 enum DialogViewId : int {
   VIEW_ID_NONE = 0,
 
-  // The following views are contained in SaveCardBubbleViews.
-  MAIN_CONTENT_VIEW_LOCAL,   // The main content view for a local
-                             // save bubble
-  MAIN_CONTENT_VIEW_UPLOAD,  // The main content view for an upload
-                             // save bubble
-  FOOTNOTE_VIEW,             // The footnote view of either an upload
-                             // save bubble or a manage cards view.
+  // The following are the important containing views of the bubble.
+  MAIN_CONTENT_VIEW_LOCAL,   // The main content view, for a local save bubble
+  MAIN_CONTENT_VIEW_UPLOAD,  // The main content view, for an upload save bubble
+  FOOTNOTE_VIEW,             // Contains the legal messages for upload save
   SIGN_IN_PROMO_VIEW,        // Contains the sign-in promo view
   MANAGE_CARDS_VIEW,         // The manage cards view
   EXPIRATION_DATE_VIEW,      // Contains the dropdowns for expiration date
@@ -30,12 +26,6 @@ enum DialogViewId : int {
   // The sub-view that contains the sign-in button in the promo.
   SIGN_IN_VIEW,
 
-  // The main content view for a migration offer bubble.
-  MAIN_CONTENT_VIEW_MIGRATION_BUBBLE,
-
-  // The main content view for the main migration dialog.
-  MAIN_CONTENT_VIEW_MIGRATION_OFFER_DIALOG,
-
   // The following are views::LabelButton objects (clickable).
   OK_BUTTON,            // Can say [Save], [Next], [Confirm],
                         // or [Done] depending on context

chrome/browser/ui/views/autofill/local_card_migration_browsertest_base.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_BROWSERTEST_BASE_H_
-#define CHROME_BROWSER_UI_VIEWS_AUTOFILL_LOCAL_CARD_MIGRATION_BROWSERTEST_BASE_H_
-
-#include <list>
-#include <memory>
-#include <string>
-
-#include "base/callback_list.h"
-#include "base/command_line.h"
-#include "base/macros.h"
-#include "base/test/scoped_feature_list.h"
-#include "chrome/browser/ui/views/autofill/dialog_view_ids.h"
-#include "chrome/browser/ui/views/autofill/local_card_migration_bubble_views.h"
-#include "chrome/browser/ui/views/autofill/local_card_migration_dialog_view.h"
-#include "chrome/browser/ui/views/autofill/local_card_migration_icon_view.h"
-#include "chrome/test/base/in_process_browser_test.h"
-#include "components/autofill/core/browser/autofill_test_utils.h"
-#include "components/autofill/core/browser/credit_card_save_manager.h"
-#include "components/autofill/core/browser/local_card_migration_manager.h"
-#include "components/autofill/core/browser/personal_data_manager.h"
-#include "components/autofill/core/browser/test_event_waiter.h"
-#include "content/public/browser/web_contents_observer.h"
-#include "net/test/embedded_test_server/embedded_test_server.h"
-#include "net/url_request/test_url_fetcher_factory.h"
-#include "services/network/public/cpp/shared_url_loader_factory.h"
-#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
-#include "services/network/test/test_url_loader_factory.h"
-#include "testing/gmock/include/gmock/gmock.h"
-
-namespace device {
-class ScopedGeolocationOverrider;
-}
-
-namespace autofill {
-
-// Base class for any interactive LocalCardMigrationBubbleViews and
-// LocalCardMigrationDialogView browser tests that will need to show and
-// interact with the local card migration flow.
-class LocalCardMigrationBrowserTestBase
-    : public InProcessBrowserTest,
-      public LocalCardMigrationManager::ObserverForTest {
- protected:
-  // Various events that can be waited on by the DialogEventWaiter.
-  enum class DialogEvent : int {
-    REQUESTED_LOCAL_CARD_MIGRATION,
-    RECEIVED_GET_UPLOAD_DETAILS_RESPONSE,
-    SENT_MIGRATE_LOCAL_CARDS_REQUEST,
-    RECEIVED_MIGRATE_CARDS_RESPONSE
-  };
-
-  LocalCardMigrationBrowserTestBase();
-  ~LocalCardMigrationBrowserTestBase() override;
-
-  void SetUpOnMainThread() override;
-
-  void NavigateTo(const std::string& file_path);
-
-  // LocalCardMigrationManager::ObserverForTest:
-  void OnDecideToRequestLocalCardMigration() override;
-  void OnReceivedGetUploadDetailsResponse() override;
-  void OnSentMigrateLocalCardsRequest() override;
-  void OnRecievedMigrateCardsResponse() override;
-
-  // BrowserTestBase:
-  void SetUpInProcessBrowserTestFixture() override;
-
-  // Sets up the ability to sign in the user.
-  void OnWillCreateBrowserContextServices(content::BrowserContext* context);
-
-  // Saves a local card.
-  void SaveLocalCard(std::string card_number);
-
-  // Saves a full server card.
-  void SaveServerCard(std::string card_number);
-
-  // Submits a checkout form with given card and waits for a migration offer.
-  void UseCardAndWaitForMigrationOffer(std::string card_number);
-
-  // Signs in the user with the provided |full_name|.
-  void SignInWithFullName(const std::string& full_name);
-
-  // Will call JavaScript to fill and submit the form.
-  void FillAndSubmitFormWithCard(std::string card_number);
-
-  // For setting up Payments RPCs.
-  void SetUploadDetailsRpcPaymentsAccepts();
-  void SetUploadDetailsRpcPaymentsDeclines();
-
-  // Clicks on the given views::View*.
-  void ClickOnView(views::View* view);
-
-  // Clicks on the given views::View* and resets the associated
-  // DialogDelegateView.
-  void ClickOnDialogViewAndWait(
-      views::View* view,
-      views::DialogDelegateView* local_card_migration_view);
-
-  // Returns the views::View* that was previously assigned the id |view_id|.
-  views::View* FindViewInDialogById(
-      DialogViewId view_id,
-      views::DialogDelegateView* local_card_migration_view);
-
-  // Clicks on dialog button in |local_card_migration_view|.
-  void ClickOnOkButton(views::DialogDelegateView* local_card_migration_view);
-  void ClickOnCancelButton(
-      views::DialogDelegateView* local_card_migration_view);
-
-  // Gets the views::DialogDelegateView* instance of the intermediate migration
-  // offer bubble.
-  views::DialogDelegateView* GetLocalCardMigrationOfferBubbleViews();
-
-  // Gets the views::DialogDelegateView* instance of the main migration dialog
-  // view.
-  views::DialogDelegateView* GetLocalCardMigrationMainDialogView();
-
-  // Gets the credit card icon in the omnibox.
-  LocalCardMigrationIconView* GetLocalCardMigrationIconView();
-
-  views::View* GetCloseButton();
-
-  views::View* GetCardListView();
-
-  content::WebContents* GetActiveWebContents();
-
-  // Resets the event waiter for a given |event_sequence|.
-  void ResetEventWaiterForSequence(std::list<DialogEvent> event_sequence);
-
-  // Wait for any events passed through ResetEventWaiterForSequence() to occur.
-  void WaitForObservedEvent();
-
-  network::TestURLLoaderFactory* test_url_loader_factory();
-
-  PersonalDataManager* personal_data_ = nullptr;
-
-  std::unique_ptr<
-      base::CallbackList<void(content::BrowserContext*)>::Subscription>
-      will_create_browser_context_services_subscription_;
-
-  base::test::ScopedFeatureList scoped_feature_list_;
-
- private:
-  std::unique_ptr<autofill::EventWaiter<DialogEvent>> event_waiter_;
-  std::unique_ptr<net::FakeURLFetcherFactory> url_fetcher_factory_;
-  network::TestURLLoaderFactory test_url_loader_factory_;
-  scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_;
-  std::unique_ptr<device::ScopedGeolocationOverrider> geolocation_overrider_;
-
-  DISALLOW_COPY_AND_ASSIGN(LocalCardMigrationBrowserTestBase);
-};
-
-}  // namespace autofill
-
-#endif  // CHROME_BROWSER_UI_VIEWS_AUTOFILL_LOCAL_CARD_MIGRATION_BROWSERTEST_BASE_H_

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

@@ -6,7 +6,6 @@
 
 #include <stddef.h>
 #include <memory>
-#include <utility>
 
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
@@ -30,7 +29,6 @@
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/style/typography.h"
-#include "ui/views/window/dialog_client_view.h"
 
 namespace autofill {
 
@@ -156,7 +154,6 @@ void LocalCardMigrationBubbleViews::Init() {
   explanatory_message->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   explanatory_message->SetMultiLine(true);
   AddChildView(explanatory_message);
-  set_id(DialogViewId::MAIN_CONTENT_VIEW_MIGRATION_BUBBLE);
 }
 
 }  // namespace autofill

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

@@ -47,8 +47,6 @@ class LocalCardMigrationBubbleViews : public LocalCardMigrationBubble,
   void WindowClosing() override;
 
  private:
-  friend class LocalCardMigrationBrowserTestBase;
-
   ~LocalCardMigrationBubbleViews() override;
 
   // views::BubbleDialogDelegateView:

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

@@ -4,10 +4,6 @@
 
 #include "chrome/browser/ui/views/autofill/local_card_migration_dialog_view.h"
 
-#include <memory>
-#include <string>
-#include <vector>
-
 #include "base/location.h"
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
@@ -283,8 +279,6 @@ class LocalCardMigrationOfferView : public views::View,
   }
 
  private:
-  friend class LocalCardMigrationDialogView;
-
   LocalCardMigrationDialogController* controller_;
 
   views::View* card_list_view_ = nullptr;
@@ -412,8 +406,6 @@ void LocalCardMigrationDialogView::Init() {
 
   if (controller_->GetViewState() == LocalCardMigrationDialogState::kOffered) {
     offer_view_ = new LocalCardMigrationOfferView(controller_, this);
-    offer_view_->set_id(DialogViewId::MAIN_CONTENT_VIEW_MIGRATION_OFFER_DIALOG);
-    card_list_view_ = offer_view_->card_list_view_;
     AddChildView(offer_view_);
   } else {
     AddChildView(CreateFeedbackContentView(controller_, this).release());

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

@@ -5,9 +5,6 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_AUTOFILL_LOCAL_CARD_MIGRATION_DIALOG_VIEW_H_
 #define CHROME_BROWSER_UI_VIEWS_AUTOFILL_LOCAL_CARD_MIGRATION_DIALOG_VIEW_H_
 
-#include <string>
-#include <vector>
-
 #include "base/macros.h"
 #include "chrome/browser/ui/autofill/local_card_migration_dialog.h"
 #include "chrome/browser/ui/views/autofill/dialog_view_ids.h"
@@ -53,8 +50,6 @@ class LocalCardMigrationDialogView : public LocalCardMigrationDialog,
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
  private:
-  friend class LocalCardMigrationBrowserTestBase;
-
   base::string16 GetOkButtonLabel() const;
   base::string16 GetCancelButtonLabel() const;
 
@@ -66,8 +61,6 @@ class LocalCardMigrationDialogView : public LocalCardMigrationDialog,
   // dialog is not in the 'offer' state.
   LocalCardMigrationOfferView* offer_view_ = nullptr;
 
-  views::View* card_list_view_;
-
   DISALLOW_COPY_AND_ASSIGN(LocalCardMigrationDialogView);
 };
 

chrome/test/BUILD.gn

@@ -1499,9 +1499,6 @@ test("browser_tests") {
         "../browser/ui/views/autofill/autofill_popup_view_views_browsertest.cc",
         "../browser/ui/views/autofill/card_unmask_prompt_view_tester_views.cc",
         "../browser/ui/views/autofill/card_unmask_prompt_view_tester_views.h",
-        "../browser/ui/views/autofill/local_card_migration_browsertest.cc",
-        "../browser/ui/views/autofill/local_card_migration_browsertest_base.cc",
-        "../browser/ui/views/autofill/local_card_migration_browsertest_base.h",
         "../browser/ui/views/autofill/save_card_bubble_views_browsertest.cc",
         "../browser/ui/views/autofill/save_card_bubble_views_browsertest_base.cc",
         "../browser/ui/views/autofill/save_card_bubble_views_browsertest_base.h",

components/autofill/core/browser/credit_card_save_manager.h

@@ -123,7 +123,6 @@ class CreditCardSaveManager {
  private:
   friend class CreditCardSaveManagerTest;
   friend class CreditCardSaveManagerTestObserverBridge;
-  friend class LocalCardMigrationBrowserTestBase;
   friend class SaveCardBubbleViewsBrowserTestBase;
 
   // Sets |show_save_prompt| and moves forward with offering credit card local

components/autofill/core/browser/form_data_importer.h

@@ -145,7 +145,6 @@ class FormDataImporter {
   friend class AutofillMergeTest;
   friend class FormDataImporterTest;
   friend class FormDataImporterTestBase;
-  friend class LocalCardMigrationBrowserTestBase;
   friend class SaveCardBubbleViewsBrowserTestBase;
   friend class SaveCardInfobarEGTestHelper;
   FRIEND_TEST_ALL_PREFIXES(AutofillMergeTest, MergeProfiles);

components/autofill/core/browser/local_card_migration_manager.cc

@@ -7,8 +7,6 @@
 #include <stddef.h>
 
 #include <algorithm>
-#include <unordered_map>
-#include <utility>
 #include <vector>
 
 #include "base/bind.h"
@@ -90,8 +88,6 @@ void LocalCardMigrationManager::AttemptToOfferLocalCardMigration(
     return;
   migration_request_ = payments::PaymentsClient::MigrationRequestDetails();
 
-  if (observer_for_testing_)
-    observer_for_testing_->OnDecideToRequestLocalCardMigration();
   payments_client_->GetUploadDetails(
       std::vector<AutofillProfile>(), GetDetectedValues(),
       /*active_experiments=*/std::vector<const char*>(), app_locale_,
@@ -143,19 +139,12 @@ bool LocalCardMigrationManager::IsCreditCardMigrationEnabled() {
   bool migration_experiment_enabled =
       features::GetLocalCardMigrationExperimentalFlag() !=
       features::LocalCardMigrationExperimentalFlag::kMigrationDisabled;
-  // If |observer_for_testing_| is set, assume we are in a browsertest and
-  // credit card upload should be enabled by default. Cannot get around this as
-  // Chrome OS testing requires an unsupported email domain (i.e.
-  // stub-user@example.com).
-  bool credit_card_upload_enabled =
-      observer_for_testing_ ||
-      ::autofill::IsCreditCardUploadEnabled(
-          client_->GetPrefs(), client_->GetSyncService(),
-          client_->GetIdentityManager()->GetPrimaryAccountInfo().email);
+  bool credit_card_upload_enabled = ::autofill::IsCreditCardUploadEnabled(
+      client_->GetPrefs(), client_->GetSyncService(),
+      client_->GetIdentityManager()->GetPrimaryAccountInfo().email);
   bool has_google_payments_account =
       (payments::GetBillingCustomerId(personal_data_manager_,
                                       payments_client_->GetPrefService()) != 0);
-
   return migration_experiment_enabled && credit_card_upload_enabled &&
          has_google_payments_account;
 }
@@ -165,8 +154,6 @@ void LocalCardMigrationManager::OnDidGetUploadDetails(
     AutofillClient::PaymentsRpcResult result,
     const base::string16& context_token,
     std::unique_ptr<base::DictionaryValue> legal_message) {
-  if (observer_for_testing_)
-    observer_for_testing_->OnReceivedGetUploadDetailsResponse();
   if (result == AutofillClient::SUCCESS) {
     migration_request_.context_token = context_token;
     legal_message_ = std::move(legal_message);
@@ -200,8 +187,6 @@ void LocalCardMigrationManager::OnDidMigrateLocalCards(
     AutofillClient::PaymentsRpcResult result,
     std::unique_ptr<std::unordered_map<std::string, std::string>> save_result,
     const std::string& display_text) {
-  if (observer_for_testing_)
-    observer_for_testing_->OnRecievedMigrateCardsResponse();
   if (!save_result)
     return;
 
@@ -253,8 +238,6 @@ void LocalCardMigrationManager::OnDidGetMigrationRiskData(
 // Send the migration request. Will call payments_client to create a new
 // PaymentsRequest. Also create a new callback function OnDidMigrateLocalCards.
 void LocalCardMigrationManager::SendMigrateLocalCardsRequest() {
-  if (observer_for_testing_)
-    observer_for_testing_->OnSentMigrateLocalCardsRequest();
   migration_request_.app_locale = app_locale_;
   migration_request_.billing_customer_number = payments::GetBillingCustomerId(
       personal_data_manager_, payments_client_->GetPrefService());

components/autofill/core/browser/local_card_migration_manager.h

@@ -7,7 +7,6 @@
 
 #include <memory>
 #include <string>
-#include <unordered_map>
 #include <vector>
 
 #include "base/strings/string16.h"
@@ -46,7 +45,7 @@ class MigratableCreditCard {
     FAILURE_ON_UPLOAD,
   };
 
-  explicit MigratableCreditCard(const CreditCard& credit_card);
+  MigratableCreditCard(const CreditCard& credit_card);
   ~MigratableCreditCard();
 
   CreditCard credit_card() const { return credit_card_; }
@@ -69,16 +68,6 @@ class MigratableCreditCard {
 // Owned by FormDataImporter.
 class LocalCardMigrationManager {
  public:
-  // An observer class used by browsertests that gets notified whenever
-  // particular actions occur.
-  class ObserverForTest {
-   public:
-    virtual void OnDecideToRequestLocalCardMigration() = 0;
-    virtual void OnReceivedGetUploadDetailsResponse() = 0;
-    virtual void OnSentMigrateLocalCardsRequest() = 0;
-    virtual void OnRecievedMigrateCardsResponse() = 0;
-  };
-
   // The parameters should outlive the LocalCardMigrationManager.
   LocalCardMigrationManager(AutofillClient* client,
                             payments::PaymentsClient* payments_client,
@@ -110,6 +99,10 @@ class LocalCardMigrationManager {
   virtual void OnUserAcceptedMainMigrationDialog(
       const std::vector<std::string>& selected_card_guids);
 
+  // Check that the user is signed in, syncing, and the proper experiment
+  // flags are enabled. Override in the test class.
+  virtual bool IsCreditCardMigrationEnabled();
+
   // Determines what detected_values metadata to send (generally, cardholder
   // name if it exists on all cards, and existence of Payments customer).
   int GetDetectedValues() const;
@@ -144,7 +137,6 @@ class LocalCardMigrationManager {
   payments::PaymentsClient* payments_client_;
 
  private:
-  friend class LocalCardMigrationBrowserTestBase;
   FRIEND_TEST_ALL_PREFIXES(LocalCardMigrationManagerTest,
                            MigrateCreditCard_MigrationPermanentFailure);
   FRIEND_TEST_ALL_PREFIXES(LocalCardMigrationManagerTest,
@@ -154,10 +146,6 @@ class LocalCardMigrationManager {
   FRIEND_TEST_ALL_PREFIXES(LocalCardMigrationManagerTest,
                            MigrateCreditCard_ToggleIsChosen);
 
-  // Check that the user is signed in, syncing, and the proper experiment
-  // flags are enabled. Override in the test class.
-  virtual bool IsCreditCardMigrationEnabled();
-
   // Pops up a larger, modal dialog showing the local cards to be uploaded.
   void ShowMainMigrationDialog();
 
@@ -169,11 +157,6 @@ class LocalCardMigrationManager {
   // Finalizes the migration request and calls PaymentsClient.
   void SendMigrateLocalCardsRequest();
 
-  // For testing.
-  void SetEventObserverForTesting(ObserverForTest* observer) {
-    observer_for_testing_ = observer;
-  }
-
   std::unique_ptr<base::DictionaryValue> legal_message_;
 
   std::string app_locale_;
@@ -200,9 +183,6 @@ class LocalCardMigrationManager {
   // Record the triggering source of the local card migration.
   AutofillMetrics::LocalCardMigrationOrigin local_card_migration_origin_;
 
-  // Initialized only during tests.
-  ObserverForTest* observer_for_testing_ = nullptr;
-
   base::WeakPtrFactory<LocalCardMigrationManager> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(LocalCardMigrationManager);