[Passwords] Guard test only PassswordForm methods with UNIT_TEST

This change guards PassswordForm methods that are only supposed to be
used in tests by `#if defined(UNIT_TEST)`.

Bug: None
Change-Id: I8e3d9381434a90041504a91ccb953e53f9e548be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2517692Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Viktor Semeniuk <vsemeniuk@google.com>
Auto-Submit: Jan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826214}

components/password_manager/core/browser/BUILD.gn

@@ -184,7 +184,6 @@ static_library("browser") {
     "password_session_durations_metrics_recorder.h",
     "password_store.cc",
     "password_store.h",
-    "password_store_change.cc",
     "password_store_change.h",
     "password_store_consumer.cc",
     "password_store_consumer.h",

components/password_manager/core/browser/multi_store_password_save_manager.cc

@@ -5,6 +5,7 @@
 #include "components/password_manager/core/browser/multi_store_password_save_manager.h"
 
 #include "base/metrics/histogram_functions.h"
+#include "base/ranges/algorithm.h"
 #include "components/autofill/core/common/gaia_id_hash.h"
 #include "components/password_manager/core/browser/form_fetcher.h"
 #include "components/password_manager/core/browser/form_saver.h"
@@ -43,13 +44,11 @@ std::vector<const PasswordForm*> ProfileStoreMatches(
 bool AccountStoreMatchesContainForm(
     const std::vector<const PasswordForm*>& matches,
     const PasswordForm& form) {
-  PasswordForm form_in_account_store(form);
-  form_in_account_store.in_store = PasswordForm::Store::kAccountStore;
-  for (const PasswordForm* match : matches) {
-    if (form_in_account_store == *match)
-      return true;
-  }
-  return false;
+  DCHECK(base::ranges::all_of(matches, &PasswordForm::IsUsingAccountStore));
+  return base::ranges::find_if(matches, [&form](const PasswordForm* match) {
+           return ArePasswordFormUniqueKeysEqual(*match, form) &&
+                  match->password_value == form.password_value;
+         }) != matches.end();
 }
 
 PendingCredentialsState ResolvePendingCredentialsStates(

components/password_manager/core/browser/password_form.cc

@@ -209,55 +209,6 @@ bool PasswordForm::HasNonEmptyPasswordValue() const {
   return !password_value.empty() || !new_password_value.empty();
 }
 
-bool PasswordForm::operator==(const PasswordForm& form) const {
-  return scheme == form.scheme && signon_realm == form.signon_realm &&
-         url == form.url && action == form.action &&
-         submit_element == form.submit_element &&
-         username_element == form.username_element &&
-         username_element_renderer_id == form.username_element_renderer_id &&
-         username_value == form.username_value &&
-         all_possible_usernames == form.all_possible_usernames &&
-         all_possible_passwords == form.all_possible_passwords &&
-         form_has_autofilled_value == form.form_has_autofilled_value &&
-         password_element == form.password_element &&
-         password_element_renderer_id == form.password_element_renderer_id &&
-         password_value == form.password_value &&
-         new_password_element == form.new_password_element &&
-         confirmation_password_element_renderer_id ==
-             form.confirmation_password_element_renderer_id &&
-         confirmation_password_element == form.confirmation_password_element &&
-         confirmation_password_element_renderer_id ==
-             form.confirmation_password_element_renderer_id &&
-         new_password_value == form.new_password_value &&
-         date_created == form.date_created && date_synced == form.date_synced &&
-         date_last_used == form.date_last_used &&
-         blocked_by_user == form.blocked_by_user && type == form.type &&
-         times_used == form.times_used &&
-         form_data.SameFormAs(form.form_data) &&
-         generation_upload_status == form.generation_upload_status &&
-         display_name == form.display_name && icon_url == form.icon_url &&
-         // We compare the serialization of the origins here, as we want unique
-         // origins to compare as '=='.
-         federation_origin.Serialize() == form.federation_origin.Serialize() &&
-         skip_zero_click == form.skip_zero_click &&
-         was_parsed_using_autofill_predictions ==
-             form.was_parsed_using_autofill_predictions &&
-         is_public_suffix_match == form.is_public_suffix_match &&
-         is_affiliation_based_match == form.is_affiliation_based_match &&
-         affiliated_web_realm == form.affiliated_web_realm &&
-         app_display_name == form.app_display_name &&
-         app_icon_url == form.app_icon_url &&
-         submission_event == form.submission_event &&
-         only_for_fallback == form.only_for_fallback &&
-         is_new_password_reliable == form.is_new_password_reliable &&
-         in_store == form.in_store &&
-         moving_blocked_for_list == form.moving_blocked_for_list;
-}
-
-bool PasswordForm::operator!=(const PasswordForm& form) const {
-  return !operator==(form);
-}
-
 bool ArePasswordFormUniqueKeysEqual(const PasswordForm& left,
                                     const PasswordForm& right) {
   return (left.signon_realm == right.signon_realm && left.url == right.url &&
@@ -266,6 +217,58 @@ bool ArePasswordFormUniqueKeysEqual(const PasswordForm& left,
           left.password_element == right.password_element);
 }
 
+bool operator==(const PasswordForm& lhs, const PasswordForm& rhs) {
+  return lhs.scheme == rhs.scheme && lhs.signon_realm == rhs.signon_realm &&
+         lhs.url == rhs.url && lhs.action == rhs.action &&
+         lhs.submit_element == rhs.submit_element &&
+         lhs.username_element == rhs.username_element &&
+         lhs.username_element_renderer_id == rhs.username_element_renderer_id &&
+         lhs.username_value == rhs.username_value &&
+         lhs.all_possible_usernames == rhs.all_possible_usernames &&
+         lhs.all_possible_passwords == rhs.all_possible_passwords &&
+         lhs.form_has_autofilled_value == rhs.form_has_autofilled_value &&
+         lhs.password_element == rhs.password_element &&
+         lhs.password_element_renderer_id == rhs.password_element_renderer_id &&
+         lhs.password_value == rhs.password_value &&
+         lhs.new_password_element == rhs.new_password_element &&
+         lhs.confirmation_password_element_renderer_id ==
+             rhs.confirmation_password_element_renderer_id &&
+         lhs.confirmation_password_element ==
+             rhs.confirmation_password_element &&
+         lhs.confirmation_password_element_renderer_id ==
+             rhs.confirmation_password_element_renderer_id &&
+         lhs.new_password_value == rhs.new_password_value &&
+         lhs.date_created == rhs.date_created &&
+         lhs.date_synced == rhs.date_synced &&
+         lhs.date_last_used == rhs.date_last_used &&
+         lhs.blocked_by_user == rhs.blocked_by_user && lhs.type == rhs.type &&
+         lhs.times_used == rhs.times_used &&
+         lhs.form_data.SameFormAs(rhs.form_data) &&
+         lhs.generation_upload_status == rhs.generation_upload_status &&
+         lhs.display_name == rhs.display_name && lhs.icon_url == rhs.icon_url &&
+         // We compare the serialization of the origins here, as we want unique
+         // origins to compare as '=='.
+         lhs.federation_origin.Serialize() ==
+             rhs.federation_origin.Serialize() &&
+         lhs.skip_zero_click == rhs.skip_zero_click &&
+         lhs.was_parsed_using_autofill_predictions ==
+             rhs.was_parsed_using_autofill_predictions &&
+         lhs.is_public_suffix_match == rhs.is_public_suffix_match &&
+         lhs.is_affiliation_based_match == rhs.is_affiliation_based_match &&
+         lhs.affiliated_web_realm == rhs.affiliated_web_realm &&
+         lhs.app_display_name == rhs.app_display_name &&
+         lhs.app_icon_url == rhs.app_icon_url &&
+         lhs.submission_event == rhs.submission_event &&
+         lhs.only_for_fallback == rhs.only_for_fallback &&
+         lhs.is_new_password_reliable == rhs.is_new_password_reliable &&
+         lhs.in_store == rhs.in_store &&
+         lhs.moving_blocked_for_list == rhs.moving_blocked_for_list;
+}
+
+bool operator!=(const PasswordForm& lhs, const PasswordForm& rhs) {
+  return !(lhs == rhs);
+}
+
 std::ostream& operator<<(std::ostream& os, PasswordForm::Scheme scheme) {
   return os << ToString(scheme);
 }

components/password_manager/core/browser/password_form.h

@@ -359,10 +359,6 @@ struct PasswordForm {
   // Returns true when |password_value| or |new_password_value| are non-empty.
   bool HasNonEmptyPasswordValue() const;
 
-  // Equality operators for testing.
-  bool operator==(const PasswordForm& form) const;
-  bool operator!=(const PasswordForm& form) const;
-
   PasswordForm();
   PasswordForm(const PasswordForm& other);
   PasswordForm(PasswordForm&& other);
@@ -378,9 +374,16 @@ bool ArePasswordFormUniqueKeysEqual(const PasswordForm& left,
                                     const PasswordForm& right);
 
 // For testing.
+#if defined(UNIT_TEST)
+// An exact equality comparison of all the fields is only useful for tests.
+// Production code should be using `ArePasswordFormUniqueKeysEqual` instead.
+bool operator==(const PasswordForm& lhs, const PasswordForm& rhs);
+bool operator!=(const PasswordForm& lhs, const PasswordForm& rhs);
+
 std::ostream& operator<<(std::ostream& os, PasswordForm::Scheme scheme);
 std::ostream& operator<<(std::ostream& os, const PasswordForm& form);
 std::ostream& operator<<(std::ostream& os, PasswordForm* form);
+#endif
 
 }  // namespace password_manager
 

components/password_manager/core/browser/password_store.cc

@@ -1389,11 +1389,4 @@ void PasswordStore::DestroyOnBackgroundSequence() {
 #endif
 }
 
-std::ostream& operator<<(std::ostream& os,
-                         const PasswordStore::FormDigest& digest) {
-  return os << "FormDigest(scheme: " << digest.scheme
-            << ", signon_realm: " << digest.signon_realm
-            << ", url: " << digest.url << ")";
-}
-
 }  // namespace password_manager

components/password_manager/core/browser/password_store.h

@@ -923,9 +923,15 @@ class PasswordStore : protected PasswordStoreSync,
   DISALLOW_COPY_AND_ASSIGN(PasswordStore);
 };
 
-// For logging only.
-std::ostream& operator<<(std::ostream& os,
-                         const PasswordStore::FormDigest& digest);
+// For testing only.
+#if defined(UNIT_TEST)
+inline std::ostream& operator<<(std::ostream& os,
+                                const PasswordStore::FormDigest& digest) {
+  return os << "FormDigest(scheme: " << digest.scheme
+            << ", signon_realm: " << digest.signon_realm
+            << ", url: " << digest.url << ")";
+}
+#endif
 
 }  // namespace password_manager
 

components/password_manager/core/browser/password_store_change.cc

-// Copyright 2015 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 "components/password_manager/core/browser/password_store_change.h"
-
-namespace password_manager {
-
-std::ostream& operator<<(std::ostream& os,
-                         const PasswordStoreChange& password_store_change) {
-  return os << "type: " << password_store_change.type()
-            << ", primary key: " << password_store_change.primary_key()
-            << ", password change: " << password_store_change.password_changed()
-            << ", password form: " << password_store_change.form();
-}
-
-}  // namespace password_manager

components/password_manager/core/browser/password_store_change.h

@@ -72,8 +72,16 @@ class PasswordStoreChange {
 typedef std::vector<PasswordStoreChange> PasswordStoreChangeList;
 
 // For testing.
-std::ostream& operator<<(std::ostream& os,
-                         const PasswordStoreChange& password_store_change);
+#if defined(UNIT_TEST)
+inline std::ostream& operator<<(
+    std::ostream& os,
+    const PasswordStoreChange& password_store_change) {
+  return os << "type: " << password_store_change.type()
+            << ", primary key: " << password_store_change.primary_key()
+            << ", password change: " << password_store_change.password_changed()
+            << ", password form: " << password_store_change.form();
+}
+#endif
 
 }  // namespace password_manager
 

ios/chrome/browser/ui/settings/password/passwords_table_view_controller.mm

@@ -1255,7 +1255,8 @@ std::vector<std::unique_ptr<password_manager::PasswordForm>> CopyOf(
         begin, end,
         [&deletedForm](
             const std::unique_ptr<password_manager::PasswordForm>& value) {
-          return *value == deletedForm;
+          return password_manager::ArePasswordFormUniqueKeysEqual(*value,
+                                                                  deletedForm);
         });
     DCHECK(formIterator != end);
 
@@ -1636,7 +1637,7 @@ std::vector<std::unique_ptr<password_manager::PasswordForm>> CopyOf(
   auto iterator = std::find_if(
       forms.begin(), forms.end(),
       [&form](const std::unique_ptr<password_manager::PasswordForm>& value) {
-        return *value == form;
+        return password_manager::ArePasswordFormUniqueKeysEqual(*value, form);
       });
   // If |form| not found, pop password details view controller.
   if (iterator == forms.end()) {