[Credential Management] Smart lock save Credentials bubble should not always pop up.

components/password_manager/core/browser/password_manager.cc

Index: components/password_manager/core/browser/password_manager.cc
diff --git a/components/password_manager/core/browser/password_manager.cc b/components/password_manager/core/browser/password_manager.cc
index 38f2df9b836421b535ad10682ad32cded7f998d6..8faeeff4954697bc1aed94ff445821c9557cab49 100644
--- a/components/password_manager/core/browser/password_manager.cc
+++ b/components/password_manager/core/browser/password_manager.cc
@@ -179,38 +179,23 @@ void PasswordManager::SetFormHasGeneratedPassword(
   // TODO(gcasto): Add UMA stats to track this.
 }
 
-bool PasswordManager::IsEnabledForCurrentPage() const {
-  bool ssl_errors = client_->DidLastPageLoadEncounterSSLErrors();
-  bool client_check = client_->IsPasswordManagerEnabledForCurrentPage();
-
-  scoped_ptr<BrowserSavePasswordProgressLogger> logger;
-  if (client_->IsLoggingActive()) {
-    logger.reset(new BrowserSavePasswordProgressLogger(client_));
-    logger->LogMessage(Logger::STRING_ENABLED_FOR_CURRENT_PAGE_METHOD);
-    logger->LogBoolean(Logger::STRING_SSL_ERRORS_PRESENT, ssl_errors);
-    logger->LogBoolean(Logger::STRING_CLIENT_CHECK_PRESENT, client_check);
-  }
-
-  return !ssl_errors && client_check;
-}
-
 bool PasswordManager::IsSavingEnabledForCurrentPage() const {
   return *saving_passwords_enabled_ && !client_->IsOffTheRecord() &&
-         IsEnabledForCurrentPage();
+         !client_->DidLastPageLoadEncounterSSLErrors() &&
+         client_->IsPasswordManagerEnabledForCurrentPage();
 }
 
 void PasswordManager::ProvisionallySavePassword(const PasswordForm& form) {
   bool is_saving_enabled = IsSavingEnabledForCurrentPage();
 
-  scoped_ptr<BrowserSavePasswordProgressLogger> logger;
+  scoped_ptr<password_manager::BrowserSavePasswordProgressLogger> logger;

[vabr (Chromium), 2015/03/30 10:58:43]

You can drop password_manager::, because this code is in this namespace already.

[melandory, 2015/04/07 13:14:13]

Done.

   if (client_->IsLoggingActive()) {
     logger.reset(new BrowserSavePasswordProgressLogger(client_));
     logger->LogMessage(Logger::STRING_PROVISIONALLY_SAVE_PASSWORD_METHOD);
     logger->LogPasswordForm(Logger::STRING_PROVISIONALLY_SAVE_PASSWORD_FORM,
                             form);
-    logger->LogBoolean(Logger::STRING_IS_SAVING_ENABLED, is_saving_enabled);
-    logger->LogBoolean(Logger::STRING_SSL_ERRORS_PRESENT,
-                       client_->DidLastPageLoadEncounterSSLErrors());
+    // This method logs the SSL status as a side-effect.

[vabr (Chromium), 2015/03/30 10:58:43]

Actually, you can drop lines 197-198 completely. I did not notice before, that
DidLastPageLoadEncounterSSLErrors() will get called (and the status logged)
because of line 189.
Sorry for the noise!

[melandory, 2015/04/07 13:14:13]

Done.

+    // client_->DidLastPageLoadEncounterSSLErrors(
   }
 
   if (!is_saving_enabled) {
@@ -444,7 +429,8 @@ void PasswordManager::CreatePendingLoginManagers(
     logger->LogMessage(Logger::STRING_CREATE_LOGIN_MANAGERS_METHOD);
   }
 
-  if (!IsEnabledForCurrentPage())
+  if (client_->DidLastPageLoadEncounterSSLErrors() ||
+      !client_->IsPasswordManagerEnabledForCurrentPage())
     return;
 
   if (logger) {