Re-enable requirements fetching if browsing history is not synced

The privacy team considered the precondition to have browsing history syncing
enabled in order to fetch password requirements files as arbitrary as Google
cannot learn what sites you visit from the spec fetching (only a very short
hash prefix is sent, currently this has size 0). They asked to remove that
precondition, which is happening with this CL.

This is largely a revert of crrev.com/566756.

Bug: 846694
Change-Id: I65ad40758d81de7ba6ab816be8e874e561b06a8b
Reviewed-on: https://chromium-review.googlesource.com/c/1292051Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Commit-Queue: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601460}

chrome/browser/password_manager/chrome_password_manager_client.cc

@@ -575,13 +575,6 @@ password_manager::SyncState ChromePasswordManagerClient::GetPasswordSyncState()
   return password_manager_util::GetPasswordSyncState(sync_service);
 }
 
-password_manager::SyncState ChromePasswordManagerClient::GetHistorySyncState()
-    const {
-  const browser_sync::ProfileSyncService* sync_service =
-      ProfileSyncServiceFactory::GetForProfile(profile_);
-  return password_manager_util::GetHistorySyncState(sync_service);
-}
-
 bool ChromePasswordManagerClient::WasLastNavigationHTTPError() const {
   DCHECK(web_contents());
 

chrome/browser/password_manager/chrome_password_manager_client.h

@@ -95,7 +95,6 @@ class ChromePasswordManagerClient
   PrefService* GetPrefs() const override;
   password_manager::PasswordStore* GetPasswordStore() const override;
   password_manager::SyncState GetPasswordSyncState() const override;
-  password_manager::SyncState GetHistorySyncState() const override;
   bool WasLastNavigationHTTPError() const override;
   net::CertStatus GetMainFrameCertStatus() const override;
   bool IsIncognito() const override;

components/password_manager/core/browser/password_generation_manager.cc

@@ -52,8 +52,7 @@ void PasswordGenerationManager::PrefetchSpec(const GURL& origin) {
     return;
 
   // Fetch password requirements for the domain.
-  if (IsRequirementsFetchingEnabled())
-    password_requirements_service->PrefetchSpec(origin);
+  password_requirements_service->PrefetchSpec(origin);
 }
 
 void PasswordGenerationManager::ProcessPasswordRequirements(
@@ -140,10 +139,6 @@ bool PasswordGenerationManager::IsGenerationEnabled(bool log_debug_data) const {
   return false;
 }
 
-bool PasswordGenerationManager::IsRequirementsFetchingEnabled() const {
-  return client_->GetHistorySyncState() == SYNCING_NORMAL_ENCRYPTION;
-}
-
 base::string16 PasswordGenerationManager::GeneratePassword(
     const GURL& last_committed_url,
     autofill::FormSignature form_signature,

components/password_manager/core/browser/password_generation_manager.h

@@ -61,10 +61,6 @@ class PasswordGenerationManager {
   // autofill::SavePasswordProgressLogger.
   bool IsGenerationEnabled(bool log_debug_data) const;
 
-  // Determines whether the PasswordGeneraitonManager has the permission to
-  // fetch domain wide password requirements from gstatic.com.
-  bool IsRequirementsFetchingEnabled() const;
-
   // Returns a randomly generated password that should (but is not guaranteed
   // to) match the requirements of the site.
   // |last_committed_url| refers to the main frame URL and may impact the

components/password_manager/core/browser/password_generation_manager_unittest.cc

@@ -128,7 +128,6 @@ class FakePasswordRequirementsSpecFetcher
 class MockPasswordManagerClient : public StubPasswordManagerClient {
  public:
   MOCK_CONST_METHOD0(GetPasswordSyncState, SyncState());
-  MOCK_CONST_METHOD0(GetHistorySyncState, SyncState());
   MOCK_CONST_METHOD0(IsSavingAndFillingEnabledForCurrentPage, bool());
   MOCK_CONST_METHOD0(IsIncognito, bool());
 
@@ -240,7 +239,6 @@ TEST_F(PasswordGenerationManagerTest, ProcessPasswordRequirements) {
     const char* name;
     bool has_domain_wide_requirements = false;
     bool has_field_requirements = false;
-    bool allowed_to_fetch_specs = true;
     autofill::PasswordRequirementsSpec expected_spec;
   } kTests[] = {
       {
@@ -263,14 +261,6 @@ TEST_F(PasswordGenerationManagerTest, ProcessPasswordRequirements) {
           .has_field_requirements = true,
           .expected_spec = GetDomainWideRequirements(),
       },
-      {
-          .name = "Don't fetch spec if not allowed",
-          .has_domain_wide_requirements = true,
-          .allowed_to_fetch_specs = false,
-          // Default value is expected even though .has_domain_wide_requirements
-          // is true.
-          .expected_spec = autofill::PasswordRequirementsSpec(),
-      },
   };
 
   // The purpose of this counter is to generate unique URLs and field signatures
@@ -328,11 +318,6 @@ TEST_F(PasswordGenerationManagerTest, ProcessPasswordRequirements) {
     autofill::FormStructure::ParseQueryResponse(response_string, forms,
                                                 nullptr);
 
-    EXPECT_CALL(*client_, GetHistorySyncState())
-        .WillRepeatedly(testing::Return(test.allowed_to_fetch_specs
-                                            ? SYNCING_NORMAL_ENCRYPTION
-                                            : NOT_SYNCING));
-
     GetGenerationManager()->PrefetchSpec(origin.GetOrigin());
 
     // Processs the password requirements with expected side effects of

components/password_manager/core/browser/password_manager_client.cc

@@ -40,10 +40,6 @@ SyncState PasswordManagerClient::GetPasswordSyncState() const {
   return NOT_SYNCING;
 }
 
-SyncState PasswordManagerClient::GetHistorySyncState() const {
-  return NOT_SYNCING;
-}
-
 bool PasswordManagerClient::WasLastNavigationHTTPError() const {
   return false;
 }

components/password_manager/core/browser/password_manager_client.h

@@ -182,11 +182,6 @@ class PasswordManagerClient {
   // TODO(crbug.com/515108): Factor this out of the client to the sync layer.
   virtual SyncState GetPasswordSyncState() const;
 
-  // Reports whether and how browsing history is synced in the embedder. The
-  // default implementation always returns NOT_SYNCING.
-  // TODO(crbug.com/515108): Factor this out of the client to the sync layer.
-  virtual SyncState GetHistorySyncState() const;
-
   // Returns true if last navigation page had HTTP error i.e 5XX or 4XX
   virtual bool WasLastNavigationHTTPError() const;
 

components/password_manager/core/browser/password_manager_util.cc

@@ -70,20 +70,6 @@ password_manager::SyncState GetPasswordSyncState(
   return password_manager::NOT_SYNCING;
 }
 
-password_manager::SyncState GetHistorySyncState(
-    const syncer::SyncService* sync_service) {
-  if (sync_service && sync_service->IsFirstSetupComplete() &&
-      sync_service->IsSyncFeatureActive() &&
-      (sync_service->GetActiveDataTypes().Has(
-           syncer::HISTORY_DELETE_DIRECTIVES) ||
-       sync_service->GetActiveDataTypes().Has(syncer::PROXY_TABS))) {
-    return sync_service->IsUsingSecondaryPassphrase()
-               ? password_manager::SYNCING_WITH_CUSTOM_PASSPHRASE
-               : password_manager::SYNCING_NORMAL_ENCRYPTION;
-  }
-  return password_manager::NOT_SYNCING;
-}
-
 void FindDuplicates(std::vector<std::unique_ptr<PasswordForm>>* forms,
                     std::vector<std::unique_ptr<PasswordForm>>* duplicates,
                     std::vector<std::vector<PasswordForm*>>* tag_groups) {

components/password_manager/core/browser/password_manager_util.h

@@ -47,11 +47,6 @@ void UpdateMetadataForUsage(autofill::PasswordForm* credential);
 password_manager::SyncState GetPasswordSyncState(
     const syncer::SyncService* sync_service);
 
-// Reports whether and how browsing history is currently synced. In particular,
-// for a null |sync_service| returns NOT_SYNCING.
-password_manager::SyncState GetHistorySyncState(
-    const syncer::SyncService* sync_service);
-
 // Finds the forms with a duplicate sync tags in |forms|. The first one of
 // the duplicated entries stays in |forms|, the others are moved to
 // |duplicates|.

ios/chrome/browser/passwords/ios_chrome_password_manager_client.h

@@ -58,7 +58,6 @@ class IOSChromePasswordManagerClient
 
   // password_manager::PasswordManagerClient implementation.
   password_manager::SyncState GetPasswordSyncState() const override;
-  password_manager::SyncState GetHistorySyncState() const override;
   bool PromptUserToSaveOrUpdatePassword(
       std::unique_ptr<password_manager::PasswordFormManagerForUI> form_to_save,
       bool update_password) override;

ios/chrome/browser/passwords/ios_chrome_password_manager_client.mm

@@ -80,12 +80,6 @@ SyncState IOSChromePasswordManagerClient::GetPasswordSyncState() const {
   return password_manager_util::GetPasswordSyncState(sync_service);
 }
 
-SyncState IOSChromePasswordManagerClient::GetHistorySyncState() const {
-  browser_sync::ProfileSyncService* sync_service =
-      ProfileSyncServiceFactory::GetForBrowserState(delegate_.browserState);
-  return password_manager_util::GetHistorySyncState(sync_service);
-}
-
 bool IOSChromePasswordManagerClient::PromptUserToChooseCredentials(
     std::vector<std::unique_ptr<autofill::PasswordForm>> local_forms,
     const GURL& origin,