PasswordFormManager clean-ups.

As part as implementation of saving in NewPasswordFormManager some parts
of PasswordFormManager will be copied to NewPasswordFormManager. Let's
make clean-up of the old code first.

Bug: 831123
Change-Id: Idb6acf4e0308d6193e3dbf4da07f73ee63859e99
Reviewed-on: https://chromium-review.googlesource.com/1073231Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577523}

components/password_manager/core/browser/password_form_manager.cc

@@ -150,8 +150,7 @@ PasswordFormManager::PasswordFormManager(
                              true /* should_migrate_http_passwords */,
                              true /* should_query_suppressed_https_forms */)),
       form_fetcher_(form_fetcher ? form_fetcher : owned_form_fetcher_.get()),
-      votes_uploader_(client, observed_form.IsPossibleChangePasswordForm()),
-      is_main_frame_secure_(client->IsMainFrameSecure()) {
+      votes_uploader_(client, observed_form.IsPossibleChangePasswordForm()) {
   // Non-HTML forms should not need any interaction with the renderer, and hence
   // no driver. Note that cloned PasswordFormManager instances can have HTML
   // forms without drivers as well.
@@ -625,17 +624,14 @@ void PasswordFormManager::CreatePendingCredentials() {
 
   // Look for the actually submitted credentials in the list of previously saved
   // credentials that were available to autofilling.
-  // This first match via FindBestSavedMatch focuses on matches by username and
-  // falls back to password based matches if |submitted_form_| has no username
-  // filled.
   const PasswordForm* saved_form = FindBestSavedMatch(submitted_form_.get());
-  if (saved_form != nullptr) {
+  if (saved_form) {
     // The user signed in with a login we autofilled.
     pending_credentials_ = *saved_form;
     SetPasswordOverridden(pending_credentials_.password_value !=
                           password_to_save.first);
 
-    if (IsPendingCredentialsPublicSuffixMatch()) {
+    if (pending_credentials_.is_public_suffix_match) {
       // If the autofilled credentials were a PSL match or credentials stored
       // from Android apps, store a copy with the current origin and signon
       // realm. This ensures that on the next visit, a precise match is found.
@@ -647,10 +643,6 @@ void PasswordFormManager::CreatePendingCredentials() {
       // If this isn't updated, then password generation uploads are off for
       // sites where PSL matching is required to fill the login form, as two
       // PASSWORD votes are uploaded per saved password instead of one.
-      //
-      // TODO(gcasto): It would be nice if other state were shared such that if
-      // say a password was updated on one match it would update on all related
-      // passwords. This is a much larger change.
       UpdateMetadataForUsage(&pending_credentials_);
 
       // Update |pending_credentials_| in order to be able correctly save it.
@@ -667,8 +659,7 @@ void PasswordFormManager::CreatePendingCredentials() {
       // actually correspond to two different accounts (see
       // http://crbug.com/385619). In that case the user should be asked again
       // before saving the password. This is ensured by setting
-      // |password_overriden_| on |pending_credentials_| to false and setting
-      // |origin| and |signon_realm| to correct values.
+      // |password_overriden_| on |pending_credentials_| to false.
       //
       // There is still the edge case when the autofilled credentials represent
       // the same account as |submitted_form_| but the stored password
@@ -746,14 +737,8 @@ void PasswordFormManager::CreatePendingCredentials() {
     }
   }
 
-  if (!IsValidAndroidFacetURI(pending_credentials_.signon_realm)) {
+  if (!IsValidAndroidFacetURI(pending_credentials_.signon_realm))
     pending_credentials_.action = submitted_form_->action;
-    // If the user selected credentials we autofilled from a PasswordForm
-    // that contained no action URL (IE6/7 imported passwords, for example),
-    // bless it with the action URL from the observed form. See b/1107719.
-    if (pending_credentials_.action.is_empty())
-      pending_credentials_.action = observed_form_.action;
-  }
 
   pending_credentials_.password_value = password_to_save.first;
   pending_credentials_.preferred = submitted_form_->preferred;
@@ -771,7 +756,9 @@ void PasswordFormManager::CreatePendingCredentials() {
     pending_credentials_.display_name = submitted_form_->display_name;
     pending_credentials_.federation_origin = submitted_form_->federation_origin;
     pending_credentials_.icon_url = submitted_form_->icon_url;
-    // Take the correct signon_realm for federated credentials.
+    // It's important to override |signon_realm| for federated credentials
+    // because it has format "federation://" + origin_host + "/" +
+    // federation_host
     pending_credentials_.signon_realm = submitted_form_->signon_realm;
   }
 

components/password_manager/core/browser/password_form_manager.h

@@ -437,10 +437,6 @@ class PasswordFormManager : public PasswordFormManagerForUI,
 
   VotesUploader votes_uploader_;
 
-  // True if the main frame's visible URL, at the time this PasswordFormManager
-  // was created, is secure.
-  bool is_main_frame_secure_ = false;
-
   // Takes care of recording metrics and events for this PasswordFormManager.
   // Make sure to call Init before using |*this|, to ensure it is not null.
   scoped_refptr<PasswordFormMetricsRecorder> metrics_recorder_;