Commit 535e290f authored by sunil.ratnu's avatar sunil.ratnu Committed by Commit bot

Improve logging in password manager internals page

This CL changes the return value of
PasswordManagerClient::PromptUserToSavePassword to bool, indicating if
the prompt was indeed displayed, and logs that appropriately through |logger|

BUG=393138

Review URL: https://codereview.chromium.org/604293002

Cr-Commit-Position: refs/heads/master@{#296985}
parent 37755be4
......@@ -150,13 +150,13 @@ void ChromePasswordManagerClient::AutofillResultsComputed() {
sync_credential_was_filtered_ = false;
}
void ChromePasswordManagerClient::PromptUserToSavePassword(
bool ChromePasswordManagerClient::PromptUserToSavePassword(
scoped_ptr<password_manager::PasswordFormManager> form_to_save) {
// Save password infobar and the password bubble prompts in case of
// "webby" URLs and do not prompt in case of "non-webby" URLS (e.g. file://).
if (!BrowsingDataHelper::IsWebScheme(
web_contents()->GetLastCommittedURL().scheme())) {
return;
return false;
}
if (IsTheHotNewBubbleUIEnabled()) {
......@@ -171,6 +171,7 @@ void ChromePasswordManagerClient::PromptUserToSavePassword(
SavePasswordInfoBarDelegate::Create(
web_contents(), form_to_save.Pass(), uma_histogram_suffix);
}
return true;
}
void ChromePasswordManagerClient::AutomaticPasswordSave(
......
......@@ -46,7 +46,7 @@ class ChromePasswordManagerClient
virtual bool IsSyncAccountCredential(
const std::string& username, const std::string& origin) const OVERRIDE;
virtual void AutofillResultsComputed() OVERRIDE;
virtual void PromptUserToSavePassword(
virtual bool PromptUserToSavePassword(
scoped_ptr<password_manager::PasswordFormManager> form_to_save) OVERRIDE;
virtual void AutomaticPasswordSave(
scoped_ptr<password_manager::PasswordFormManager> saved_form_manager)
......
......@@ -159,6 +159,8 @@ std::string GetStringFromID(SavePasswordProgressLogger::StringID id) {
return "SSL errors present";
case SavePasswordProgressLogger::STRING_ONLY_VISIBLE:
return "only_visible";
case SavePasswordProgressLogger::STRING_SHOW_PASSWORD_PROMPT:
return "Show password prompt";
case SavePasswordProgressLogger::STRING_INVALID:
return "INVALID";
// Intentionally no default: clause here -- all IDs need to get covered.
......
......@@ -98,6 +98,7 @@ class SavePasswordProgressLogger {
STRING_NO_MATCHING_FORM,
STRING_SSL_ERRORS_PRESENT,
STRING_ONLY_VISIBLE,
STRING_SHOW_PASSWORD_PROMPT,
STRING_INVALID, // Represents a string returned in a case of an error.
STRING_MAX = STRING_INVALID
};
......
......@@ -67,8 +67,6 @@ class TestPasswordManagerClient : public StubPasswordManagerClient {
return false;
}
virtual void PromptUserToSavePassword(
scoped_ptr<PasswordFormManager> form_to_save) OVERRIDE {}
virtual PrefService* GetPrefs() OVERRIDE { return &prefs_; }
virtual PasswordStore* GetPasswordStore() OVERRIDE { return password_store_; }
virtual PasswordManagerDriver* GetDriver() OVERRIDE { return &driver_; }
......
......@@ -487,7 +487,10 @@ void PasswordManager::OnPasswordFormsRendered(
if (ShouldPromptUserToSavePassword()) {
if (logger)
logger->LogMessage(Logger::STRING_DECISION_ASK);
client_->PromptUserToSavePassword(provisional_save_manager_.Pass());
if (client_->PromptUserToSavePassword(provisional_save_manager_.Pass())) {
if (logger)
logger->LogMessage(Logger::STRING_SHOW_PASSWORD_PROMPT);
}
} else {
if (logger)
logger->LogMessage(Logger::STRING_DECISION_SAVE);
......
......@@ -52,7 +52,8 @@ class PasswordManagerClient {
// Informs the embedder of a password form that can be saved if the user
// allows it. The embedder is not required to prompt the user if it decides
// that this form doesn't need to be saved.
virtual void PromptUserToSavePassword(
// Returns true if the prompt was indeed displayed.
virtual bool PromptUserToSavePassword(
scoped_ptr<PasswordFormManager> form_to_save) = 0;
// Called when a password is saved in an automated fashion. Embedder may
......
......@@ -54,9 +54,10 @@ class MockPasswordManagerClient : public StubPasswordManagerClient {
MOCK_METHOD0(GetDriver, PasswordManagerDriver*());
// Workaround for scoped_ptr<> lacking a copy constructor.
virtual void PromptUserToSavePassword(
virtual bool PromptUserToSavePassword(
scoped_ptr<PasswordFormManager> manager) OVERRIDE {
PromptUserToSavePasswordPtr(manager.release());
return false;
}
virtual void AutomaticPasswordSave(
scoped_ptr<PasswordFormManager> manager) OVERRIDE {
......
......@@ -22,8 +22,10 @@ bool StubPasswordManagerClient::ShouldFilterAutofillResult(
return false;
}
void StubPasswordManagerClient::PromptUserToSavePassword(
scoped_ptr<PasswordFormManager> form_to_save) {}
bool StubPasswordManagerClient::PromptUserToSavePassword(
scoped_ptr<PasswordFormManager> form_to_save) {
return false;
}
void StubPasswordManagerClient::AutomaticPasswordSave(
scoped_ptr<PasswordFormManager> saved_manager) {}
......
......@@ -22,7 +22,7 @@ class StubPasswordManagerClient : public PasswordManagerClient {
const std::string& username, const std::string& origin) const OVERRIDE;
virtual bool ShouldFilterAutofillResult(
const autofill::PasswordForm& form) OVERRIDE;
virtual void PromptUserToSavePassword(
virtual bool PromptUserToSavePassword(
scoped_ptr<PasswordFormManager> form_to_save) OVERRIDE;
virtual void AutomaticPasswordSave(
scoped_ptr<PasswordFormManager> saved_manager) OVERRIDE;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment