Propagate form predictions for uploading on username first flow.

Bug: 959776
Change-Id: I0e2ad7cf484d2fcabafaa8afa819b821cfc9df4a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1864662
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707101}

components/password_manager/core/browser/form_parsing/password_field_prediction.cc

@@ -16,15 +16,6 @@ using autofill::ServerFieldType;
 
 namespace password_manager {
 
-namespace {
-
-// Returns true if the field is password or username prediction.
-bool IsCredentialRelatedPrediction(ServerFieldType type) {
-  return DeriveFromServerFieldType(type) != CredentialFieldType::kNone;
-}
-
-}  // namespace
-
 CredentialFieldType DeriveFromServerFieldType(ServerFieldType type) {
   switch (type) {
     case autofill::USERNAME:
@@ -89,23 +80,20 @@ FormPredictions ConvertToFormPredictions(int driver_id,
       }
     }
 
-    if (IsCredentialRelatedPrediction(server_type)) {
-      bool may_use_prefilled_placeholder = false;
-      for (const auto& predictions : field->server_predictions()) {
-        may_use_prefilled_placeholder |=
-            predictions.may_use_prefilled_placeholder();
-      }
-
-      field_predictions.emplace_back();
+    bool may_use_prefilled_placeholder = false;
+    for (const auto& predictions : field->server_predictions()) {
+      may_use_prefilled_placeholder |=
+          predictions.may_use_prefilled_placeholder();
+    }
 
-      field_predictions.back().renderer_id = field->unique_renderer_id,
-      field_predictions.back().type = server_type,
-      field_predictions.back().may_use_prefilled_placeholder =
-          may_use_prefilled_placeholder;
+    field_predictions.emplace_back();
+    field_predictions.back().renderer_id = field->unique_renderer_id;
+    field_predictions.back().type = server_type;
+    field_predictions.back().may_use_prefilled_placeholder =
+        may_use_prefilled_placeholder;
 #if defined(OS_IOS)
-      field_predictions.back().unique_id = field->unique_id;
+    field_predictions.back().unique_id = field->unique_id;
 #endif
-    }
   }
 
   FormPredictions predictions;

components/password_manager/core/browser/form_parsing/password_field_prediction_unittest.cc

@@ -37,16 +37,6 @@ namespace password_manager {
 
 namespace {
 
-const PasswordFieldPrediction* FindFormPrediction(
-    const FormPredictions& predictions,
-    uint32_t renderer_id) {
-  for (const PasswordFieldPrediction& prediction : predictions.fields) {
-    if (prediction.renderer_id == renderer_id)
-      return &prediction;
-  }
-  return nullptr;
-}
-
 TEST(FormPredictionsTest, ConvertToFormPredictions) {
   struct TestField {
     std::string name;
@@ -57,7 +47,7 @@ TEST(FormPredictionsTest, ConvertToFormPredictions) {
   } test_fields[] = {
       {"full_name", "text", UNKNOWN_TYPE, UNKNOWN_TYPE, false},
       // Password Manager is interested only in credential related types.
-      {"Email", "email", EMAIL_ADDRESS, UNKNOWN_TYPE, false},
+      {"Email", "email", EMAIL_ADDRESS, EMAIL_ADDRESS, false},
       {"username", "text", USERNAME, USERNAME, true},
       {"Password", "password", PASSWORD, PASSWORD, false},
       {"confirm_password", "password", CONFIRMATION_PASSWORD,
@@ -73,20 +63,15 @@ TEST(FormPredictionsTest, ConvertToFormPredictions) {
   }
 
   FormStructure form_structure(form_data);
-  size_t expected_predictions = 0;
   // Set server predictions and create expected votes.
   for (size_t i = 0; i < base::size(test_fields); ++i) {
     AutofillField* field = form_structure.field(i);
     field->set_server_type(test_fields[i].input_type);
-    ServerFieldType expected_type = test_fields[i].expected_type;
 
     FieldPrediction prediction;
     prediction.set_may_use_prefilled_placeholder(
         test_fields[i].may_use_prefilled_placeholder);
     field->set_server_predictions({prediction});
-
-    if (expected_type != UNKNOWN_TYPE)
-      ++expected_predictions;
   }
 
   constexpr int driver_id = 1000;
@@ -96,20 +81,14 @@ TEST(FormPredictionsTest, ConvertToFormPredictions) {
   // Check whether actual predictions are equal to expected ones.
   EXPECT_EQ(driver_id, actual_predictions.driver_id);
   EXPECT_EQ(form_structure.form_signature(), actual_predictions.form_signature);
-  EXPECT_EQ(expected_predictions, actual_predictions.fields.size());
+  EXPECT_EQ(base::size(test_fields), actual_predictions.fields.size());
 
   for (size_t i = 0; i < base::size(test_fields); ++i) {
-    uint32_t unique_renderer_id = form_data.fields[i].unique_renderer_id;
-    const PasswordFieldPrediction* actual_prediction =
-        FindFormPrediction(actual_predictions, unique_renderer_id);
-    if (test_fields[i].expected_type == UNKNOWN_TYPE) {
-      EXPECT_FALSE(actual_prediction);
-    } else {
-      ASSERT_TRUE(actual_prediction);
-      EXPECT_EQ(test_fields[i].expected_type, actual_prediction->type);
-      EXPECT_EQ(test_fields[i].may_use_prefilled_placeholder,
-                actual_prediction->may_use_prefilled_placeholder);
-    }
+    const PasswordFieldPrediction& actual_prediction =
+        actual_predictions.fields[i];
+    EXPECT_EQ(test_fields[i].expected_type, actual_prediction.type);
+    EXPECT_EQ(test_fields[i].may_use_prefilled_placeholder,
+              actual_prediction.may_use_prefilled_placeholder);
   }
 }
 
@@ -171,10 +150,7 @@ TEST(FormPredictionsTest, ConvertToFormPredictions_SynthesiseConfirmation) {
                    << ", input type=" << test_form[i].input_type
                    << ", expected type=" << test_form[i].expected_type
                    << ", synthesised FormFieldData=" << form_data.fields[i]);
-      const PasswordFieldPrediction* actual_prediction = FindFormPrediction(
-          actual_predictions, form_data.fields[i].unique_renderer_id);
-      ASSERT_TRUE(actual_prediction);
-      EXPECT_EQ(test_form[i].expected_type, actual_prediction->type);
+      EXPECT_EQ(test_form[i].expected_type, actual_predictions.fields[i].type);
     }
   }
 }

components/password_manager/core/browser/password_form_manager.cc

@@ -700,8 +700,10 @@ bool PasswordFormManager::ProvisionallySave(
                               base::Time::Now())) {
     parsed_submitted_form_->username_value = possible_username->value;
     metrics_recorder_->set_possible_username_used(true);
-    votes_uploader_.set_single_username_vote_data(
-        possible_username->renderer_id, possible_username->form_predictions);
+    if (possible_username->form_predictions) {
+      votes_uploader_.set_single_username_vote_data(
+          possible_username->renderer_id, *possible_username->form_predictions);
+    }
   }
   CreatePendingCredentials();
   return true;

components/password_manager/core/browser/password_form_manager_unittest.cc

@@ -2055,7 +2055,7 @@ TEST_F(PasswordFormManagerTest, UsernameFirstFlow) {
   const base::string16 possible_username = ASCIIToUTF16("possible_username");
   PossibleUsernameData possible_username_data(
       saved_match_.signon_realm, 1u /* renderer_id */, possible_username,
-      base::Time::Now());
+      base::Time::Now(), 0 /* driver_id */);
 
   FormData submitted_form = observed_form_only_password_fields_;
   submitted_form.fields[0].value = ASCIIToUTF16("strongpassword");
@@ -2079,7 +2079,7 @@ TEST_F(PasswordFormManagerTest, UsernameFirstFlowDifferentDomains) {
   base::string16 possible_username = ASCIIToUTF16("possible_username");
   PossibleUsernameData possible_username_data(
       "https://another.domain.com", 1u /* renderer_id */, possible_username,
-      base::Time::Now());
+      base::Time::Now(), 0 /* driver_id */);
 
   FormData submitted_form = observed_form_only_password_fields_;
   submitted_form.fields[0].value = ASCIIToUTF16("strongpassword");
@@ -2092,6 +2092,53 @@ TEST_F(PasswordFormManagerTest, UsernameFirstFlowDifferentDomains) {
   EXPECT_TRUE(form_manager_->GetPendingCredentials().username_value.empty());
 }
 
+// Tests that username is taken during username first flow.
+TEST_F(PasswordFormManagerTest, UsernameFirstFlowVotes) {
+  TestMockTimeTaskRunner::ScopedContext scoped_context(task_runner_.get());
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(features::kUsernameFirstFlow);
+
+  CreateFormManager(observed_form_only_password_fields_);
+  fetcher_->NotifyFetchCompleted();
+  const base::string16 possible_username = ASCIIToUTF16("possible_username");
+  constexpr uint64_t kUsernameFieldRendererId = 100;
+  PossibleUsernameData possible_username_data(
+      saved_match_.signon_realm, kUsernameFieldRendererId, possible_username,
+      base::Time::Now(), 0 /* driver_id */);
+
+  // Create form predictions and set them to |possible_username_data|.
+  FormPredictions predictions;
+  constexpr uint64_t kUsernameFormSignature = 1000;
+  predictions.form_signature = kUsernameFormSignature;
+  PasswordFieldPrediction field_prediction;
+  field_prediction.renderer_id = kUsernameFieldRendererId;
+  field_prediction.signature = 123;
+  field_prediction.type = autofill::SINGLE_USERNAME;
+  predictions.fields.push_back(field_prediction);
+  possible_username_data.form_predictions = predictions;
+
+  // Simulate submission a form without username. Data from
+  // |possible_username_data| will be taken for setting username.
+  FormData submitted_form = observed_form_only_password_fields_;
+  submitted_form.fields[0].value = ASCIIToUTF16("strongpassword");
+
+  ASSERT_TRUE(form_manager_->ProvisionallySave(submitted_form, &driver_,
+                                               &possible_username_data));
+
+  // Check that uploads for both username and password form happen.
+  testing::InSequence in_sequence;
+  // Upload for the password form.
+  EXPECT_CALL(mock_autofill_download_manager_,
+              StartUploadRequest(_, false, _, _, true, nullptr));
+
+  // Upload for the username form.
+  EXPECT_CALL(mock_autofill_download_manager_,
+              StartUploadRequest(SignatureIs(kUsernameFormSignature), false, _,
+                                 _, true, nullptr));
+
+  form_manager_->Save();
+}
+
 }  // namespace
 
 }  // namespace password_manager

components/password_manager/core/browser/password_manager.cc

@@ -279,6 +279,8 @@ void PasswordManager::DidNavigateMainFrame(bool form_may_be_submitted) {
     }
   }
   form_managers_.clear();
+
+  TryToFindPredictionsToPossibleUsernameData();
   predictions_.clear();
   store_password_called_ = false;
 }
@@ -322,6 +324,7 @@ void PasswordManager::DropFormManagers() {
   form_managers_.clear();
   owned_submitted_form_manager_.reset();
   all_visible_forms_.clear();
+  TryToFindPredictionsToPossibleUsernameData();
   predictions_.clear();
 }
 
@@ -386,8 +389,10 @@ void PasswordManager::OnUserModifiedNonPasswordField(
     PasswordManagerDriver* driver,
     int32_t renderer_id,
     const base::string16& value) {
+  // |driver| might be empty on iOS or in tests.
+  int driver_id = driver ? driver->GetId() : 0;
   possible_username_.emplace(GetSignonRealm(driver->GetLastCommittedURL()),
-                             renderer_id, value, base::Time::Now());
+                             renderer_id, value, base::Time::Now(), driver_id);
 }
 
 void PasswordManager::ShowManualFallbackForSaving(
@@ -586,6 +591,7 @@ PasswordFormManager* PasswordManager::ProvisionallySaveForm(
     return nullptr;
   }
 
+  TryToFindPredictionsToPossibleUsernameData();
   const PossibleUsernameData* possible_username =
       possible_username_ ? &possible_username_.value() : nullptr;
   if (!matched_manager->ProvisionallySave(submitted_form, driver,
@@ -1050,4 +1056,20 @@ void PasswordManager::ReportSubmittedFormFrameMetric(
   metrics_util::LogSubmittedFormFrame(frame);
 }
 
+void PasswordManager::TryToFindPredictionsToPossibleUsernameData() {
+  if (!possible_username_ || possible_username_->form_predictions)
+    return;
+
+  for (auto it : predictions_) {
+    if (it.second.driver_id != possible_username_->driver_id)
+      continue;
+    for (const PasswordFieldPrediction& field : it.second.fields) {
+      if (field.renderer_id == possible_username_->renderer_id) {
+        possible_username_->form_predictions = it.second;
+        return;
+      }
+    }
+  }
+}
+
 }  // namespace password_manager

components/password_manager/core/browser/password_manager.h

@@ -299,6 +299,11 @@ class PasswordManager : public FormSubmissionObserver {
   void ReportSubmittedFormFrameMetric(const PasswordManagerDriver* driver,
                                       const autofill::PasswordForm& form);
 
+  //  If |possible_username_.form_predictions| is missing, this functions tries
+  //  to find predictions for the form which contains |possible_username_| in
+  //  |predictions_|.
+  void TryToFindPredictionsToPossibleUsernameData();
+
   // PasswordFormManager transition schemes:
   // 1. HTML submission with navigation afterwads.
   // form "seen"

components/password_manager/core/browser/possible_username_data.cc

@@ -12,13 +12,17 @@ using base::TimeDelta;
 namespace password_manager {
 
 PossibleUsernameData::PossibleUsernameData(std::string signon_realm,
-                                           int32_t renderer_id,
+                                           uint32_t renderer_id,
                                            base::string16 value,
-                                           base::Time last_change)
+                                           base::Time last_change,
+                                           int driver_id)
     : signon_realm(std::move(signon_realm)),
       renderer_id(renderer_id),
       value(std::move(value)),
-      last_change(last_change) {}
+      last_change(last_change),
+      driver_id(driver_id) {}
+PossibleUsernameData::PossibleUsernameData(const PossibleUsernameData&) =
+    default;
 PossibleUsernameData::~PossibleUsernameData() = default;
 
 bool IsPossibleUsernameValid(const PossibleUsernameData& possible_username,

components/password_manager/core/browser/possible_username_data.h

@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/optional.h"
 #include "base/strings/string16.h"
 #include "base/time/time.h"
 #include "components/password_manager/core/browser/form_parsing/password_field_prediction.h"
@@ -23,18 +24,24 @@ constexpr base::TimeDelta kMaxDelayBetweenTypingUsernameAndSubmission =
 // username during username first flow.
 struct PossibleUsernameData {
   PossibleUsernameData(std::string signon_realm,
-                       int32_t renderer_id,
+                       uint32_t renderer_id,
                        base::string16 value,
-                       base::Time last_change);
+                       base::Time last_change,
+                       int driver_id);
+  PossibleUsernameData(const PossibleUsernameData&);
   ~PossibleUsernameData();
 
   std::string signon_realm;
-  int32_t renderer_id;
+  uint32_t renderer_id;
   base::string16 value;
   base::Time last_change;
 
+  // Id of PasswordManagerDriver which corresponds to the frame of this field.
+  // Paired with the |renderer_id|, this identifies a field globally.
+  int driver_id;
+
   // Predictions for the form which contains a field with |renderer_id|.
-  FormPredictions* form_predictions = nullptr;
+  base::Optional<FormPredictions> form_predictions;
 };
 
 // Checks that |possible_username| might represent an username:

components/password_manager/core/browser/possible_username_data_unittest.cc

@@ -22,7 +22,8 @@ class IsPossibleUsernameValidTest : public testing::Test {
             "https://example.com/" /* submitted_signon_realm */,
             1u /* renderer_id */,
             ASCIIToUTF16("username") /* value */,
-            base::Time::Now() /* last_change */) {}
+            base::Time::Now() /* last_change */,
+            10 /* driver_id */) {}
 
  protected:
   PossibleUsernameData possible_username_data_;

components/password_manager/core/browser/votes_uploader.h

@@ -163,9 +163,8 @@ class VotesUploader {
   void clear_single_username_vote_data() { single_username_vote_data_.reset(); }
 
   void set_single_username_vote_data(int renderer_id,
-                                     const FormPredictions* form_predictions) {
-    if (form_predictions)
-      single_username_vote_data_.emplace(renderer_id, *form_predictions);
+                                     const FormPredictions& form_predictions) {
+    single_username_vote_data_.emplace(renderer_id, form_predictions);
   }
 
  private:

components/password_manager/core/browser/votes_uploader_unittest.cc

@@ -405,7 +405,7 @@ TEST_F(VotesUploaderTest, UploadSingleUsername) {
     form_predictions.fields.back().signature = kUsernameFieldSignature;
 
     votes_uploader.set_single_username_vote_data(kUsernameRendererId,
-                                                 &form_predictions);
+                                                 form_predictions);
 
     ServerFieldTypeSet expected_types = {credentials_saved ? SINGLE_USERNAME
                                                            : NOT_USERNAME};