Obsolete HTTP Cleaner

chrome/browser/password_manager/chrome_password_manager_client.cc

Index: chrome/browser/password_manager/chrome_password_manager_client.cc
diff --git a/chrome/browser/password_manager/chrome_password_manager_client.cc b/chrome/browser/password_manager/chrome_password_manager_client.cc
index 62bd164680d795bea9e57ea43cb983781c968add..090d2519f56b71aa018bb503da40dc606b616da4 100644
--- a/chrome/browser/password_manager/chrome_password_manager_client.cc
+++ b/chrome/browser/password_manager/chrome_password_manager_client.cc
@@ -60,6 +60,8 @@
 #include "extensions/features/features.h"
 #include "google_apis/gaia/gaia_urls.h"
 #include "net/base/url_util.h"
+#include "net/http/transport_security_state.h"
+#include "net/url_request/url_request_context.h"
 #include "third_party/re2/src/re2/re2.h"
 
 #if defined(OS_ANDROID)
@@ -140,9 +142,8 @@ void ChromePasswordManagerClient::CreateForWebContentsWithAutofillClient(
   if (FromWebContents(contents))
     return;
 
-  contents->SetUserData(
-      UserDataKey(),
-      new ChromePasswordManagerClient(contents, autofill_client));
+  contents->SetUserData(UserDataKey(), new ChromePasswordManagerClient(
+                                           contents, autofill_client));

[jdoerrie, 2017/02/03 16:49:15]

I'm just seeing these now, most likely they are caused by |git cl format|. Is it
fine to keep them or should I remove these changes from the CL?

[vasilii, 2017/02/06 16:43:24]

Better remove. The previous formatting is nicer.

 }
 
 ChromePasswordManagerClient::ChromePasswordManagerClient(
@@ -223,6 +224,27 @@ bool ChromePasswordManagerClient::IsFillingEnabledForCurrentPage() const {
          IsPasswordManagementEnabledForCurrentPage();
 }
 
+bool ChromePasswordManagerClient::IsHSTSActiveForOrigin(

[jdoerrie, 2017/02/03 16:49:15]

I'm not quite sure if naming is correct here. Maybe it should be
|IsHSTSActiveForHost| instead?

[vasilii, 2017/02/06 16:43:24]

Agree with host.

[jdoerrie, 2017/02/07 13:15:00]

Done.

+    const GURL& origin) const {
+  if (!origin.is_valid())
+    return false;
+
+  net::TransportSecurityState* security_state =
+      profile_->GetRequestContext()
+          ->GetURLRequestContext()
+          ->transport_security_state();
+
+  if (!security_state)
+    return false;
+
+  const std::string host = origin.host();
+  net::TransportSecurityState::STSState sts_state;
+  net::TransportSecurityState::PKPState pkp_state;
+  return security_state->GetStaticDomainState(host, &sts_state, &pkp_state) ||
+         security_state->GetDynamicSTSState(host, &sts_state) ||
+         security_state->GetDynamicPKPState(host, &pkp_state);

[jdoerrie, 2017/02/03 16:49:15]

This check is very similar to what is done in
|TransportSecurityState::ShouldSSLErrorsBeFatal|
(https://codesearch.chromium.org/chromium/src/net/http/transport_security_stat...)
and |NetInternalsMessageHandler::IOThreadImpl::OnHSTSQuery|
(https://codesearch.chromium.org/chromium/src/chrome/browser/ui/webui/net_inte...).
Thus it leads to some code duplication, but I feel it is good to have this logic
directly in PasswordManagerClient as well. Thoughts?

[vasilii, 2017/02/06 16:43:24]

The code should be here. How is HPKP relevant here?

[jdoerrie, 2017/02/07 13:15:00]

Not sure, I reached out to security folks regarding this.

+}
+
 bool ChromePasswordManagerClient::OnCredentialManagerUsed() {
   prerender::PrerenderContents* prerender_contents =
       prerender::PrerenderContents::FromWebContents(web_contents());
@@ -261,8 +283,7 @@ bool ChromePasswordManagerClient::PromptUserToSaveOrUpdatePassword(
                                           std::move(form_to_save));
     return true;
   }
-  SavePasswordInfoBarDelegate::Create(web_contents(),
-                                      std::move(form_to_save));
+  SavePasswordInfoBarDelegate::Create(web_contents(), std::move(form_to_save));
 #endif  // !defined(OS_ANDROID)
   return true;
 }
@@ -411,14 +432,15 @@ PrefService* ChromePasswordManagerClient::GetPrefs() {
   return profile_->GetPrefs();
 }
 
-password_manager::PasswordStore*
-ChromePasswordManagerClient::GetPasswordStore() const {
+password_manager::PasswordStore* ChromePasswordManagerClient::GetPasswordStore()
+    const {
   // Always use EXPLICIT_ACCESS as the password manager checks IsOffTheRecord
   // itself when it shouldn't access the PasswordStore.
   // TODO(gcasto): Is is safe to change this to
   // ServiceAccessType::IMPLICIT_ACCESS?
-  return PasswordStoreFactory::GetForProfile(
-             profile_, ServiceAccessType::EXPLICIT_ACCESS).get();
+  return PasswordStoreFactory::GetForProfile(profile_,
+                                             ServiceAccessType::EXPLICIT_ACCESS)
+      .get();
 }
 
 password_manager::PasswordSyncState
@@ -435,8 +457,7 @@ bool ChromePasswordManagerClient::WasLastNavigationHTTPError() const {
   if (log_manager_->IsLoggingActive()) {
     logger.reset(new password_manager::BrowserSavePasswordProgressLogger(
         log_manager_.get()));
-    logger->LogMessage(
-        Logger::STRING_WAS_LAST_NAVIGATION_HTTP_ERROR_METHOD);
+    logger->LogMessage(Logger::STRING_WAS_LAST_NAVIGATION_HTTP_ERROR_METHOD);
   }
 
   content::NavigationEntry* entry =