Fix flaky AutofillPrivateApiTests

This is an attempt to fix flakes in a number of AutofillPrivateApiTests.
The hypothesis is that the tests fail because the PersonalDataManager
did not have a chance to start up. This means that data may be
unavailable or that change notifications are pushed because the PDM
finally starts but these may then confuse observers that look for
specific signals.

This also fixed AutofillPrivateApiTest.RemoveEntry which became stale
as some code changes happened while the test was disabled. The parameter
list of the callback handler was incorrect.

If there are still fakes, please don't revert this CL but rather disable
the tests again with a new bug report.

Bug: 1162474,643097,1143312,1154856,988146
Change-Id: I06b9dc3db02daa2b5eb74aa354719e323a7d4ae9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2628287
Commit-Queue: Dominic Battré <battre@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843493}

chrome/browser/autofill/autofill_uitest_util.cc

@@ -114,4 +114,11 @@ void WaitForPersonalDataChange(Profile* base_profile) {
   observer.Wait();
 }
 
+void WaitForPersonalDataManagerToBeLoaded(Profile* base_profile) {
+  PersonalDataManager* pdm =
+      autofill::PersonalDataManagerFactory::GetForProfile(base_profile);
+  while (!pdm->IsDataLoaded())
+    WaitForPersonalDataChange(base_profile);
+}
+
 }  // namespace autofill

chrome/browser/autofill/autofill_uitest_util.h

@@ -24,6 +24,7 @@ void AddTestAutofillData(Profile* base_profile,
                          const AutofillProfile& profile,
                          const CreditCard& card);
 void WaitForPersonalDataChange(Profile* base_profile);
+void WaitForPersonalDataManagerToBeLoaded(Profile* base_profile);
 
 }  // namespace autofill
 

chrome/browser/extensions/api/autofill_private/autofill_private_apitest.cc

@@ -5,8 +5,11 @@
 #include "base/command_line.h"
 #include "base/values.h"
 #include "build/build_config.h"
+#include "chrome/browser/autofill/autofill_uitest_util.h"
+#include "chrome/browser/autofill/personal_data_manager_factory.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/common/extensions/api/autofill_private.h"
+#include "components/autofill/core/browser/personal_data_manager.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/test_utils.h"
@@ -34,6 +37,8 @@ class AutofillPrivateApiTest : public ExtensionApiTest {
 
  protected:
   bool RunAutofillSubtest(const std::string& subtest) {
+    autofill::WaitForPersonalDataManagerToBeLoaded(profile());
+
     return RunExtensionSubtest("autofill_private", "main.html?" + subtest,
                                kFlagNone, kFlagLoadAsComponent);
   }
@@ -43,13 +48,7 @@ class AutofillPrivateApiTest : public ExtensionApiTest {
 
 // TODO(hcarmona): Investigate converting these tests to unittests.
 
-// TODO(crbug.com/1162474): Disabled on Mac for flakiness.
-#if defined(OS_MAC)
-#define MAYBE_GetCountryList DISABLED_GetCountryList
-#else
-#define MAYBE_GetCountryList GetCountryList
-#endif
-IN_PROC_BROWSER_TEST_F(AutofillPrivateApiTest, MAYBE_GetCountryList) {
+IN_PROC_BROWSER_TEST_F(AutofillPrivateApiTest, GetCountryList) {
   EXPECT_TRUE(RunAutofillSubtest("getCountryList")) << message_;
 }
 
@@ -57,8 +56,7 @@ IN_PROC_BROWSER_TEST_F(AutofillPrivateApiTest, GetAddressComponents) {
   EXPECT_TRUE(RunAutofillSubtest("getAddressComponents")) << message_;
 }
 
-// TODO(crbug.com/643097) Disabled for flakiness.
-IN_PROC_BROWSER_TEST_F(AutofillPrivateApiTest, DISABLED_RemoveEntry) {
+IN_PROC_BROWSER_TEST_F(AutofillPrivateApiTest, RemoveEntry) {
   EXPECT_TRUE(RunAutofillSubtest("removeEntry")) << message_;
 }
 
@@ -66,23 +64,11 @@ IN_PROC_BROWSER_TEST_F(AutofillPrivateApiTest, ValidatePhoneNumbers) {
   EXPECT_TRUE(RunAutofillSubtest("ValidatePhoneNumbers")) << message_;
 }
 
-// TODO(crbug.com/1143312): Disabled on Mac for flakiness.
-#if defined(OS_MAC)
-#define MAYBE_AddAndUpdateAddress DISABLED_AddAndUpdateAddress
-#else
-#define MAYBE_AddAndUpdateAddress AddAndUpdateAddress
-#endif
-IN_PROC_BROWSER_TEST_F(AutofillPrivateApiTest, MAYBE_AddAndUpdateAddress) {
+IN_PROC_BROWSER_TEST_F(AutofillPrivateApiTest, AddAndUpdateAddress) {
   EXPECT_TRUE(RunAutofillSubtest("addAndUpdateAddress")) << message_;
 }
 
-// TODO(crbug.com/1154856): Disabled on Mac for flakiness.
-#if defined(OS_MAC)
-#define MAYBE_AddAndUpdateCreditCard DISABLED_AddAndUpdateCreditCard
-#else
-#define MAYBE_AddAndUpdateCreditCard AddAndUpdateCreditCard
-#endif
-IN_PROC_BROWSER_TEST_F(AutofillPrivateApiTest, MAYBE_AddAndUpdateCreditCard) {
+IN_PROC_BROWSER_TEST_F(AutofillPrivateApiTest, AddAndUpdateCreditCard) {
   EXPECT_TRUE(RunAutofillSubtest("addAndUpdateCreditCard")) << message_;
 }
 

chrome/test/data/extensions/api_test/autofill_private/test.js

@@ -306,26 +306,33 @@ var availableTests = [
     var guid;
 
     var numCalls = 0;
-    var handler = function(creditCardList) {
+    var getCardsHandler = function(creditCardList) {
       numCalls++;
+      chrome.test.assertEq(1, numCalls);
+    }
 
-      if (numCalls == 1) {
-        chrome.test.assertEq(creditCardList.length, 0);
-      } else if (numCalls == 2) {
+    var personalDataChangedHandler = function(addressList, creditCardList) {
+      numCalls++;
+
+      if (numCalls == 2) {
         chrome.test.assertEq(creditCardList.length, 1);
         var creditCard = creditCardList[0];
         chrome.test.assertEq(creditCard.name, NAME);
 
         guid = creditCard.guid;
         chrome.autofillPrivate.removeEntry(guid);
-      } else {
+      } else if (numCalls == 3) {
         chrome.test.assertEq(creditCardList.length, 0);
         chrome.test.succeed();
+      } else {
+        // We should never receive such a call.
+        chrome.test.fail();
       }
     }
 
-    chrome.autofillPrivate.onPersonalDataChanged.addListener(handler);
-    chrome.autofillPrivate.getCreditCardList(handler);
+    chrome.autofillPrivate.onPersonalDataChanged.addListener(
+        personalDataChangedHandler);
+    chrome.autofillPrivate.getCreditCardList(getCardsHandler);
     chrome.autofillPrivate.saveCreditCard({name: NAME});
   },
 

testing/buildbot/filters/code_coverage.browser_tests.filter

 # TODO(crbug.com/988146): Enable this.
--AutofillPrivateApiTest.AddAndUpdateAddress
 -PasswordManagerBrowserTest.*
 -ContentScriptApiTest.ContentScriptBlobFetch
 -CredentialManagerBrowserTest.*