Updating PasswordCheckDelegate to work with CredentialUIEntry
This CL replaces use of CredentialWithPassword with CredentialUIEntry
inside PasswordCheckDelegate.
After PasswordsPrivateDelegateImpl switches to SavedPasswordsPresenter
CredentialUIEntry will be a unified model which will allow to simplify
API and clean big chunks of code.
Bug: 1152747
Change-Id: I3dd071a7940373b181d4b921fb83a26a3c9757c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3677297
Commit-Queue: Viktor Semeniuk <vsemeniuk@google.com>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1011484}
diff --git a/chrome/browser/extensions/api/passwords_private/password_check_delegate.cc b/chrome/browser/extensions/api/passwords_private/password_check_delegate.cc
index 5d6b53a0..fcd63f8 100644
--- a/chrome/browser/extensions/api/passwords_private/password_check_delegate.cc
+++ b/chrome/browser/extensions/api/passwords_private/password_check_delegate.cc
@@ -34,7 +34,6 @@
#include "components/password_manager/content/browser/password_change_success_tracker_factory.h"
#include "components/password_manager/core/browser/android_affiliation/affiliation_utils.h"
#include "components/password_manager/core/browser/bulk_leak_check_service.h"
-#include "components/password_manager/core/browser/insecure_credentials_table.h"
#include "components/password_manager/core/browser/leak_detection/bulk_leak_check.h"
#include "components/password_manager/core/browser/leak_detection/encryption_utils.h"
#include "components/password_manager/core/browser/password_change_success_tracker.h"
@@ -43,7 +42,6 @@
#include "components/password_manager/core/browser/ui/insecure_credentials_manager.h"
#include "components/password_manager/core/browser/ui/saved_passwords_presenter.h"
#include "components/password_manager/core/browser/well_known_change_password_util.h"
-#include "components/password_manager/core/common/password_manager_features.h"
#include "components/password_manager/core/common/password_manager_pref_names.h"
#include "components/prefs/pref_service.h"
#include "components/url_formatter/elide_url.h"
@@ -58,8 +56,8 @@
namespace {
using password_manager::CanonicalizeUsername;
-using password_manager::CredentialWithPassword;
-using password_manager::InsecureCredentialTypeFlags;
+using password_manager::CredentialUIEntry;
+using password_manager::InsecureType;
using password_manager::LeakCheckCredential;
using password_manager::PasswordChangeSuccessTracker;
using password_manager::PasswordForm;
@@ -181,46 +179,79 @@
TimeFormat::FORMAT_ELAPSED, TimeFormat::LENGTH_LONG, elapsed_time, true));
}
-// Helper struct that bundles a CredentialWithPassword with a corresponding
-// passwords_private::CompromiseType.
-struct CompromisedCredentialAndType {
- CredentialWithPassword credential;
- api::passwords_private::CompromiseType type;
-};
+base::Time GetTimeWhenWasPhishedOrLeaked(const CredentialUIEntry& entry) {
+ base::Time compromise_time;
+ if (entry.IsLeaked()) {
+ compromise_time =
+ entry.password_issues.at(InsecureType::kLeaked).create_time;
+ }
+ if (entry.IsPhished()) {
+ compromise_time =
+ std::max(compromise_time,
+ entry.password_issues.at(InsecureType::kPhished).create_time);
+ }
+ return compromise_time;
+}
+
+api::passwords_private::CompromiseType GetCompromiseType(
+ const CredentialUIEntry& entry) {
+ if (entry.IsLeaked() && entry.IsPhished()) {
+ return api::passwords_private::COMPROMISE_TYPE_PHISHED_AND_LEAKED;
+ } else if (entry.IsLeaked()) {
+ return api::passwords_private::COMPROMISE_TYPE_LEAKED;
+ } else if (entry.IsPhished()) {
+ return api::passwords_private::COMPROMISE_TYPE_PHISHED;
+ }
+ NOTREACHED();
+ return api::passwords_private::COMPROMISE_TYPE_NONE;
+}
+
+bool IsCredentialMuted(const CredentialUIEntry& entry) {
+ if (!entry.IsLeaked() && !entry.IsPhished())
+ return false;
+
+ bool is_muted = true;
+ if (entry.IsLeaked()) {
+ is_muted &=
+ entry.password_issues.at(InsecureType::kLeaked).is_muted.value();
+ }
+ if (entry.IsPhished()) {
+ is_muted &=
+ entry.password_issues.at(InsecureType::kPhished).is_muted.value();
+ }
+ return is_muted;
+}
// Orders |compromised_credentials| in such a way that phished credentials
// precede leaked credentials, and that credentials of the same compromise type
// are ordered by recency.
-std::vector<CompromisedCredentialAndType> OrderCompromisedCredentials(
- std::vector<CredentialWithPassword> compromised_credentials) {
- // Move all credentials into a single list, associating with the
- // corresponding CompromiseType.
- std::vector<CompromisedCredentialAndType> results;
- results.reserve(compromised_credentials.size());
- for (auto& credential : compromised_credentials) {
- // Since CompromiseType does not contain information about weakness/reuse
- // of credential, we need to unset those bits in the
- // |credential.insecure_type|.
- auto type = static_cast<api::passwords_private::CompromiseType>(
- UnsetWeakAndReusedCredentialTypeFlags(credential.insecure_type));
- results.push_back({std::move(credential), type});
- }
+void OrderInsecureCredentials(std::vector<CredentialUIEntry>& credentials) {
// Reordering phished credential to the beginning.
- auto last_phished = std::partition(
- results.begin(), results.end(), [](const auto& credential) {
- return credential.type !=
- api::passwords_private::COMPROMISE_TYPE_LEAKED;
- });
+ auto non_phished = std::partition(
+ credentials.begin(), credentials.end(),
+ [](const auto& credential) { return credential.IsPhished(); });
// By construction the phished credentials precede the leaked credentials in
// |results|. Now sort both groups by their creation date so that most recent
// compromises appear first in both lists.
- auto create_time_cmp = [](const auto& lhs, const auto& rhs) {
- return lhs.credential.create_time > rhs.credential.create_time;
+ auto create_time_cmp = [](auto& lhs, auto& rhs) {
+ return GetTimeWhenWasPhishedOrLeaked(lhs) >
+ GetTimeWhenWasPhishedOrLeaked(rhs);
};
- std::sort(results.begin(), last_phished, create_time_cmp);
- std::sort(last_phished, results.end(), create_time_cmp);
- return results;
+ std::sort(credentials.begin(), non_phished, create_time_cmp);
+ std::sort(non_phished, credentials.end(), create_time_cmp);
+}
+
+api::passwords_private::CompromisedInfo CreateCompromiseInfo(
+ const CredentialUIEntry& form) {
+ api::passwords_private::CompromisedInfo compromise_info;
+ compromise_info.compromise_time =
+ GetTimeWhenWasPhishedOrLeaked(form).ToJsTimeIgnoringNull();
+ compromise_info.elapsed_time_since_compromise =
+ FormatElapsedTime(GetTimeWhenWasPhishedOrLeaked(form));
+ compromise_info.compromise_type = GetCompromiseType(form);
+ compromise_info.is_muted = IsCredentialMuted(form);
+ return compromise_info;
}
} // namespace
@@ -258,27 +289,19 @@
std::vector<api::passwords_private::InsecureCredential>
PasswordCheckDelegate::GetCompromisedCredentials() {
- std::vector<CompromisedCredentialAndType>
- ordered_compromised_credential_and_types = OrderCompromisedCredentials(
- insecure_credentials_manager_.GetInsecureCredentials());
+ std::vector<CredentialUIEntry> ordered_credentials =
+ insecure_credentials_manager_.GetInsecureCredentialEntries();
+ OrderInsecureCredentials(ordered_credentials);
std::vector<api::passwords_private::InsecureCredential>
compromised_credentials;
- compromised_credentials.reserve(
- ordered_compromised_credential_and_types.size());
- for (const auto& credential_and_type :
- ordered_compromised_credential_and_types) {
- const CredentialWithPassword& credential = credential_and_type.credential;
+ compromised_credentials.reserve(ordered_credentials.size());
+ for (const auto& credential : ordered_credentials) {
api::passwords_private::InsecureCredential api_credential =
ConstructInsecureCredential(credential);
api_credential.compromised_info =
- std::make_unique<api::passwords_private::CompromisedInfo>();
- api_credential.compromised_info->compromise_time =
- credential.create_time.ToJsTimeIgnoringNull();
- api_credential.compromised_info->elapsed_time_since_compromise =
- FormatElapsedTime(credential.create_time);
- api_credential.compromised_info->compromise_type = credential_and_type.type;
- api_credential.compromised_info->is_muted = credential.is_muted.value();
+ std::make_unique<api::passwords_private::CompromisedInfo>(
+ CreateCompromiseInfo(credential));
compromised_credentials.push_back(std::move(api_credential));
}
@@ -287,12 +310,12 @@
std::vector<api::passwords_private::InsecureCredential>
PasswordCheckDelegate::GetWeakCredentials() {
- std::vector<CredentialWithPassword> weak_credentials =
- insecure_credentials_manager_.GetWeakCredentials();
+ std::vector<CredentialUIEntry> weak_credentials =
+ insecure_credentials_manager_.GetWeakCredentialEntries();
std::vector<api::passwords_private::InsecureCredential> api_credentials;
api_credentials.reserve(weak_credentials.size());
- for (const auto& weak_credential : weak_credentials) {
+ for (auto& weak_credential : weak_credentials) {
api_credentials.push_back(ConstructInsecureCredential(weak_credential));
}
@@ -302,60 +325,56 @@
absl::optional<api::passwords_private::InsecureCredential>
PasswordCheckDelegate::GetPlaintextInsecurePassword(
api::passwords_private::InsecureCredential credential) const {
- const CredentialWithPassword* insecure_credential =
- FindMatchingInsecureCredential(credential);
- if (!insecure_credential)
+ const CredentialUIEntry* entry = FindMatchingEntry(credential);
+ if (!entry)
return absl::nullopt;
- credential.password = std::make_unique<std::string>(
- base::UTF16ToUTF8(insecure_credential->password));
+ credential.password =
+ std::make_unique<std::string>(base::UTF16ToUTF8(entry->password));
return credential;
}
bool PasswordCheckDelegate::ChangeInsecureCredential(
const api::passwords_private::InsecureCredential& credential,
base::StringPiece new_password) {
- // Try to obtain the original CredentialWithPassword. Return false if fails.
- const CredentialWithPassword* insecure_credential =
- FindMatchingInsecureCredential(credential);
- if (!insecure_credential)
+ // Try to obtain the original CredentialUIEntry. Return false if fails.
+ const CredentialUIEntry* entry = FindMatchingEntry(credential);
+ if (!entry)
return false;
- return insecure_credentials_manager_.UpdateCredential(*insecure_credential,
- new_password);
+ CredentialUIEntry credential_to_edit = *entry;
+ credential_to_edit.password = base::UTF8ToUTF16(new_password);
+ return saved_passwords_presenter_->EditSavedCredentials(credential_to_edit);
}
bool PasswordCheckDelegate::RemoveInsecureCredential(
const api::passwords_private::InsecureCredential& credential) {
- // Try to obtain the original CredentialWithPassword. Return false if fails.
- const CredentialWithPassword* insecure_credential =
- FindMatchingInsecureCredential(credential);
- if (!insecure_credential)
+ // Try to obtain the original CredentialUIEntry. Return false if fails.
+ const CredentialUIEntry* entry = FindMatchingEntry(credential);
+ if (!entry)
return false;
- return insecure_credentials_manager_.RemoveCredential(*insecure_credential);
+ return saved_passwords_presenter_->RemoveCredential(*entry);
}
bool PasswordCheckDelegate::MuteInsecureCredential(
const api::passwords_private::InsecureCredential& credential) {
- // Try to obtain the original CredentialWithPassword. Return false if fails.
- const CredentialWithPassword* insecure_credential =
- FindMatchingInsecureCredential(credential);
- if (!insecure_credential)
+ // Try to obtain the original CredentialUIEntry. Return false if fails.
+ const CredentialUIEntry* entry = FindMatchingEntry(credential);
+ if (!entry)
return false;
- return insecure_credentials_manager_.MuteCredential(*insecure_credential);
+ return insecure_credentials_manager_.MuteCredential(*entry);
}
bool PasswordCheckDelegate::UnmuteInsecureCredential(
const api::passwords_private::InsecureCredential& credential) {
- // Try to obtain the original CredentialWithPassword. Return false if fails.
- const CredentialWithPassword* insecure_credential =
- FindMatchingInsecureCredential(credential);
- if (!insecure_credential)
+ // Try to obtain the original CredentialUIEntry. Return false if fails.
+ const CredentialUIEntry* entry = FindMatchingEntry(credential);
+ if (!entry)
return false;
- return insecure_credentials_manager_.UnmuteCredential(*insecure_credential);
+ return insecure_credentials_manager_.UnmuteCredential(*entry);
}
// Records that a change password flow was started for |credential| and
@@ -439,8 +458,6 @@
}
State state = bulk_leak_check_service_adapter_.GetBulkLeakCheckState();
- SavedPasswordsView saved_passwords =
- saved_passwords_presenter_->GetSavedPasswords();
// Handle the currently running case first, only then consider errors.
if (state == State::kRunning) {
@@ -459,7 +476,7 @@
return result;
}
- if (saved_passwords.empty()) {
+ if (saved_passwords_presenter_->GetSavedCredentials().empty()) {
result.state = api::passwords_private::PASSWORD_CHECK_STATE_NO_PASSWORDS;
return result;
}
@@ -535,23 +552,21 @@
}
}
-const CredentialWithPassword*
-PasswordCheckDelegate::FindMatchingInsecureCredential(
+const CredentialUIEntry* PasswordCheckDelegate::FindMatchingEntry(
const api::passwords_private::InsecureCredential& credential) const {
- const CredentialWithPassword* insecure_credential =
+ const CredentialUIEntry* entry =
insecure_credential_id_generator_.TryGetKey(credential.id);
- if (!insecure_credential)
+ if (!entry)
return nullptr;
- if (credential.signon_realm != insecure_credential->signon_realm ||
- credential.username != base::UTF16ToUTF8(insecure_credential->username) ||
+ if (credential.signon_realm != entry->signon_realm ||
+ credential.username != base::UTF16ToUTF8(entry->username) ||
(credential.password &&
- *credential.password !=
- base::UTF16ToUTF8(insecure_credential->password))) {
+ *credential.password != base::UTF16ToUTF8(entry->password))) {
return nullptr;
}
- return insecure_credential;
+ return entry;
}
void PasswordCheckDelegate::
@@ -589,22 +604,20 @@
api::passwords_private::InsecureCredential
PasswordCheckDelegate::ConstructInsecureCredential(
- const CredentialWithPassword& credential) {
+ const CredentialUIEntry& entry) {
api::passwords_private::InsecureCredential api_credential;
auto facet = password_manager::FacetURI::FromPotentiallyInvalidSpec(
- credential.signon_realm);
+ entry.signon_realm);
if (facet.IsValidAndroidFacetURI()) {
api_credential.is_android_credential = true;
// |formatted_orgin|, |detailed_origin| and |change_password_url| need
// special handling for Android. Here we use affiliation information
// instead of the origin.
- const PasswordForm& android_form =
- insecure_credentials_manager_.GetSavedPasswordsFor(credential)[0];
- if (!android_form.app_display_name.empty()) {
- api_credential.formatted_origin = android_form.app_display_name;
- api_credential.detailed_origin = android_form.app_display_name;
+ if (!entry.app_display_name.empty()) {
+ api_credential.formatted_origin = entry.app_display_name;
+ api_credential.detailed_origin = entry.app_display_name;
api_credential.change_password_url =
- GetChangePasswordUrl(GURL(android_form.affiliated_web_realm));
+ GetChangePasswordUrl(GURL(entry.affiliated_web_realm));
} else {
// In case no affiliation information could be obtained show the
// formatted package name to the user. An empty change_password_url will
@@ -618,7 +631,7 @@
api_credential.is_android_credential = false;
api_credential.formatted_origin =
base::UTF16ToUTF8(url_formatter::FormatUrl(
- credential.url.DeprecatedGetOriginAsURL(),
+ entry.url.GetWithEmptyPath(),
url_formatter::kFormatUrlOmitDefaults |
url_formatter::kFormatUrlOmitHTTPS |
url_formatter::kFormatUrlOmitTrivialSubdomains |
@@ -626,13 +639,13 @@
base::UnescapeRule::SPACES, nullptr, nullptr, nullptr));
api_credential.detailed_origin =
base::UTF16ToUTF8(url_formatter::FormatUrlForSecurityDisplay(
- credential.url.DeprecatedGetOriginAsURL()));
- api_credential.change_password_url = GetChangePasswordUrl(credential.url);
+ entry.url.GetWithEmptyPath()));
+ api_credential.change_password_url = GetChangePasswordUrl(entry.url);
}
- api_credential.id = insecure_credential_id_generator_.GenerateId(credential);
- api_credential.signon_realm = credential.signon_realm;
- api_credential.username = base::UTF16ToUTF8(credential.username);
+ api_credential.id = insecure_credential_id_generator_.GenerateId(entry);
+ api_credential.signon_realm = entry.signon_realm;
+ api_credential.username = base::UTF16ToUTF8(entry.username);
return api_credential;
}
diff --git a/chrome/browser/extensions/api/passwords_private/password_check_delegate.h b/chrome/browser/extensions/api/passwords_private/password_check_delegate.h
index b7db681..1280671 100644
--- a/chrome/browser/extensions/api/passwords_private/password_check_delegate.h
+++ b/chrome/browser/extensions/api/passwords_private/password_check_delegate.h
@@ -18,8 +18,8 @@
#include "components/password_manager/core/browser/bulk_leak_check_service.h"
#include "components/password_manager/core/browser/leak_detection/bulk_leak_check.h"
#include "components/password_manager/core/browser/leak_detection/leak_detection_delegate_interface.h"
-#include "components/password_manager/core/browser/password_store.h"
#include "components/password_manager/core/browser/ui/bulk_leak_check_service_adapter.h"
+#include "components/password_manager/core/browser/ui/credential_ui_entry.h"
#include "components/password_manager/core/browser/ui/credential_utils.h"
#include "components/password_manager/core/browser/ui/insecure_credentials_manager.h"
#include "components/password_manager/core/browser/ui/saved_passwords_presenter.h"
@@ -134,14 +134,13 @@
void OnCredentialDone(const password_manager::LeakCheckCredential& credential,
password_manager::IsLeaked is_leaked) override;
- // Tries to find the matching CredentialWithPassword for |credential|. It
+ // Tries to find the matching CredentialUIEntry for |credential|. It
// performs a look-up in |insecure_credential_id_generator_| using
// |credential.id|. If a matching value exists it also verifies that signon
// realm, username and when possible password match.
- // Returns a pointer to the matching CredentialWithPassword on success or
+ // Returns a pointer to the matching CredentialUIEntry on success or
// nullptr otherwise.
- const password_manager::CredentialWithPassword*
- FindMatchingInsecureCredential(
+ const password_manager::CredentialUIEntry* FindMatchingEntry(
const api::passwords_private::InsecureCredential& credential) const;
// Invoked when a compromised password check completes. Records the current
@@ -157,9 +156,9 @@
// OnStateChanged.
void NotifyPasswordCheckStatusChanged();
- // Constructs |InsecureCredential| from |CredentialWithPassword|.
+ // Constructs |InsecureCredential| from |CredentialUIEntry|.
api::passwords_private::InsecureCredential ConstructInsecureCredential(
- const password_manager::CredentialWithPassword& credential);
+ const password_manager::CredentialUIEntry& entry);
// Obtain a raw pointer to the |PasswordChangeSuccessTracker| associated
// with |profile_|.
@@ -219,10 +218,10 @@
// An id generator for insecure credentials. Required to match
// api::passwords_private::InsecureCredential instances passed to the UI
- // with the underlying CredentialWithPassword they are based on.
- IdGenerator<password_manager::CredentialWithPassword,
+ // with the underlying CredentialUIEntry they are based on.
+ IdGenerator<password_manager::CredentialUIEntry,
int,
- password_manager::PasswordCredentialLess>
+ password_manager::CredentialUIEntry::Less>
insecure_credential_id_generator_;
base::WeakPtrFactory<PasswordCheckDelegate> weak_ptr_factory_{this};
diff --git a/chrome/browser/extensions/api/passwords_private/password_check_delegate_unittest.cc b/chrome/browser/extensions/api/passwords_private/password_check_delegate_unittest.cc
index 4d3e3ed7..375950c 100644
--- a/chrome/browser/extensions/api/passwords_private/password_check_delegate_unittest.cc
+++ b/chrome/browser/extensions/api/passwords_private/password_check_delegate_unittest.cc
@@ -701,7 +701,9 @@
}
// Test that changing a insecure password removes duplicates from store.
-TEST_F(PasswordCheckDelegateTest, ChangeInsecureCredentialRemovesDupes) {
+// https://crbug.com/1334160
+TEST_F(PasswordCheckDelegateTest,
+ DISABLED_ChangeInsecureCredentialRemovesDupes) {
PasswordForm form = MakeSavedPassword(kExampleCom, kUsername1, kPassword1);
AddIssueToForm(&form, InsecureType::kLeaked);
store().AddLogin(form);
diff --git a/components/password_manager/core/browser/ui/credential_ui_entry.cc b/components/password_manager/core/browser/ui/credential_ui_entry.cc
index 4b08a2ca..c047f15 100644
--- a/components/password_manager/core/browser/ui/credential_ui_entry.cc
+++ b/components/password_manager/core/browser/ui/credential_ui_entry.cc
@@ -34,6 +34,14 @@
CredentialUIEntry& CredentialUIEntry::operator=(CredentialUIEntry&& other) =
default;
+bool CredentialUIEntry::IsLeaked() const {
+ return password_issues.contains(InsecureType::kLeaked);
+}
+
+bool CredentialUIEntry::IsPhished() const {
+ return password_issues.contains(InsecureType::kPhished);
+}
+
bool operator==(const CredentialUIEntry& lhs, const CredentialUIEntry& rhs) {
return lhs.key() == rhs.key();
}
diff --git a/components/password_manager/core/browser/ui/credential_ui_entry.h b/components/password_manager/core/browser/ui/credential_ui_entry.h
index da72546..b55f322 100644
--- a/components/password_manager/core/browser/ui/credential_ui_entry.h
+++ b/components/password_manager/core/browser/ui/credential_ui_entry.h
@@ -20,6 +20,13 @@
// construction from PasswordForm for convenience. A single entry might
// correspond to multiple PasswordForms.
struct CredentialUIEntry {
+ struct Less {
+ bool operator()(const CredentialUIEntry& lhs,
+ const CredentialUIEntry& rhs) const {
+ return lhs.key() < rhs.key();
+ }
+ };
+
explicit CredentialUIEntry(const PasswordForm& form);
CredentialUIEntry(const CredentialUIEntry& other);
CredentialUIEntry(CredentialUIEntry&& other);
@@ -71,6 +78,10 @@
const CredentialKey& key() const { return key_; }
+ // Information about password insecurities.
+ bool IsLeaked() const;
+ bool IsPhished() const;
+
private:
// Key which is constructed from an original PasswordForm.
CredentialKey key_;
diff --git a/components/password_manager/core/browser/ui/insecure_credentials_manager.cc b/components/password_manager/core/browser/ui/insecure_credentials_manager.cc
index d7d4ae02..239aff3 100644
--- a/components/password_manager/core/browser/ui/insecure_credentials_manager.cc
+++ b/components/password_manager/core/browser/ui/insecure_credentials_manager.cc
@@ -280,18 +280,6 @@
}
bool InsecureCredentialsManager::MuteCredential(
- const CredentialView& credential) {
- auto it = credentials_to_forms_.find(credential);
- if (it == credentials_to_forms_.end())
- return false;
-
- const auto& saved_passwords = it->second.forms;
- DCHECK(!saved_passwords.empty());
-
- return MuteCredential(CredentialUIEntry(saved_passwords[0]));
-}
-
-bool InsecureCredentialsManager::MuteCredential(
const CredentialUIEntry& credential) {
CredentialUIEntry updated_credential = credential;
for (auto& password_issue : updated_credential.password_issues) {
@@ -304,18 +292,6 @@
}
bool InsecureCredentialsManager::UnmuteCredential(
- const CredentialView& credential) {
- auto it = credentials_to_forms_.find(credential);
- if (it == credentials_to_forms_.end())
- return false;
-
- const auto& saved_passwords = it->second.forms;
- DCHECK(!saved_passwords.empty());
-
- return UnmuteCredential(CredentialUIEntry(saved_passwords[0]));
-}
-
-bool InsecureCredentialsManager::UnmuteCredential(
const CredentialUIEntry& credential) {
CredentialUIEntry updated_credential = credential;
for (auto& password_issue : updated_credential.password_issues) {
diff --git a/components/password_manager/core/browser/ui/insecure_credentials_manager.h b/components/password_manager/core/browser/ui/insecure_credentials_manager.h
index 20cf5eb..2ccd09cf 100644
--- a/components/password_manager/core/browser/ui/insecure_credentials_manager.h
+++ b/components/password_manager/core/browser/ui/insecure_credentials_manager.h
@@ -189,14 +189,10 @@
// Attempts to mute |credential| from the password store.
// Returns whether the mute succeeded.
- // TODO(crbug.com/1330549): Use CredentialUIEntry only.
- bool MuteCredential(const CredentialView& credential);
bool MuteCredential(const CredentialUIEntry& credential);
// Attempts to unmute |credential| from the password store.
// Returns whether the unmute succeeded.
- // TODO(crbug.com/1330549): Use CredentialUIEntry only.
- bool UnmuteCredential(const CredentialView& credential);
bool UnmuteCredential(const CredentialUIEntry& credential);
// Returns a vector of currently insecure credentials.
diff --git a/components/password_manager/core/browser/ui/saved_passwords_presenter.cc b/components/password_manager/core/browser/ui/saved_passwords_presenter.cc
index ece02a88..24bae7d 100644
--- a/components/password_manager/core/browser/ui/saved_passwords_presenter.cc
+++ b/components/password_manager/core/browser/ui/saved_passwords_presenter.cc
@@ -93,11 +93,12 @@
RemoveCredential(CredentialUIEntry(form));
}
-void SavedPasswordsPresenter::RemoveCredential(
+bool SavedPasswordsPresenter::RemoveCredential(
const CredentialUIEntry& credential) {
const auto range =
sort_key_to_password_forms_.equal_range(credential.key().value());
+ bool removed = false;
std::for_each(range.first, range.second, [&](const auto& pair) {
const auto& current_form = pair.second;
// Make sure |form| and |current_form| share the same store.
@@ -106,8 +107,10 @@
// 'OnGetPasswordStoreResultsFrom'. So it can be present only in one store
// at a time..
GetStoreFor(current_form).RemoveLogin(current_form);
+ removed = true;
}
});
+ return removed;
}
bool SavedPasswordsPresenter::AddPassword(const PasswordForm& form) {
@@ -193,6 +196,7 @@
base::ranges::transform(range.first, range.second,
std::back_inserter(forms_to_change),
[](const auto& pair) { return pair.second; });
+
if (forms_to_change.empty())
return false;
diff --git a/components/password_manager/core/browser/ui/saved_passwords_presenter.h b/components/password_manager/core/browser/ui/saved_passwords_presenter.h
index ee453da4..134906d 100644
--- a/components/password_manager/core/browser/ui/saved_passwords_presenter.h
+++ b/components/password_manager/core/browser/ui/saved_passwords_presenter.h
@@ -69,7 +69,7 @@
// Removes the credential and all its duplicates from the store.
// TODO(crbug.com/1330906): Remove in favor of EditSavedCredentials.
void RemovePassword(const PasswordForm& form);
- void RemoveCredential(const CredentialUIEntry& credential);
+ bool RemoveCredential(const CredentialUIEntry& credential);
// Adds the credential to the store specified in the |form|. Returns true
// if the password was added, false if |form|'s data is not valid (invalid
