Do not trust local NOT_USERNAME prediction if server has data.

Local NOT_USERNAME prediction is based on a weak signal - that the user
ignored the save prompt. So it makes sense to ignore if the server
has data.

Bug: 959776
Change-Id: If2f186ee47e59b8eef4271c980b6837ffeea6da0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2023836
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736341}

components/password_manager/core/browser/password_manager.cc

@@ -51,6 +51,7 @@ using autofill::NEW_PASSWORD;
 using autofill::NOT_USERNAME;
 using autofill::PasswordForm;
 using autofill::SINGLE_USERNAME;
+using autofill::USERNAME;
 using autofill::mojom::PasswordFormFieldPredictionType;
 #if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)
 using password_manager::metrics_util::GaiaPasswordHashChange;
@@ -166,9 +167,15 @@ void AddLocallySavedPredictions(FieldInfoManager* field_info_manager,
   for (PasswordFieldPrediction& field : predictions->fields) {
     auto local_prediction = field_info_manager->GetFieldType(
         predictions->form_signature, field.signature);
-    if (local_prediction != SINGLE_USERNAME && local_prediction != NOT_USERNAME)
-      continue;
-    field.type = local_prediction;
+    if (local_prediction == SINGLE_USERNAME) {
+      field.type = SINGLE_USERNAME;
+    } else if (local_prediction == NOT_USERNAME) {
+      // Now local prediction NOT_USERNAME is based on the weak signal (the user
+      // ignored or rejected the prompt) so use it only if the server does not
+      // have data.
+      if (field.type != SINGLE_USERNAME && field.type != USERNAME)
+        field.type = NOT_USERNAME;
+    }
   }
 }
 

components/password_manager/core/browser/password_manager_unittest.cc

@@ -59,9 +59,12 @@
 using autofill::FormData;
 using autofill::FormFieldData;
 using autofill::FormStructure;
+using autofill::NO_SERVER_DATA;
+using autofill::NOT_USERNAME;
 using autofill::PasswordForm;
 using autofill::PasswordFormFillData;
 using autofill::ServerFieldType;
+using autofill::SINGLE_USERNAME;
 using autofill::mojom::PasswordFormFieldPredictionType;
 using base::ASCIIToUTF16;
 using base::Feature;
@@ -3372,15 +3375,14 @@ TEST_F(PasswordManagerTest, UsernameFirstFlow) {
   EXPECT_EQ(password, saved_form.password_value);
 }
 
-//  Checks that username is saved on username first flow.
-TEST_F(PasswordManagerTest, UsernameFirstFlowFillingLocalPredictions) {
+#if !defined(OS_IOS)
+//  Checks that username is filled on username first flow based on server and
+//  local predictions.
+TEST_F(PasswordManagerTest, UsernameFirstFlowFillingServerAndLocalPredictions) {
   base::test::ScopedFeatureList feature_list;
   feature_list.InitAndEnableFeature(features::kUsernameFirstFlow);
   EXPECT_CALL(*store_, GetLogins(_, _))
       .WillRepeatedly(WithArg<1>(InvokeConsumer(MakeSavedForm())));
-
-  manager()->OnPasswordFormsParsed(&driver_, {} /* observed */);
-
   EXPECT_CALL(client_, IsSavingAndFillingEnabled).WillRepeatedly(Return(true));
 
   constexpr uint32_t kFieldRendererId = 1;
@@ -3391,17 +3393,37 @@ TEST_F(PasswordManagerTest, UsernameFirstFlowFillingLocalPredictions) {
   field.unique_renderer_id = kFieldRendererId;
   non_password_form.fields.push_back(field);
 
-  FormStructure form_structure(non_password_form);
-
   MockFieldInfoManager mock_field_manager;
   ON_CALL(client_, GetFieldInfoManager())
       .WillByDefault(Return(&mock_field_manager));
-  EXPECT_CALL(mock_field_manager, GetFieldType(_, _))
-      .WillOnce(Return(autofill::SINGLE_USERNAME));
 
-  EXPECT_CALL(driver_, FillPasswordForm(_));
-  manager()->ProcessAutofillPredictions(&driver_, {&form_structure});
+  for (auto server_type : {NO_SERVER_DATA, SINGLE_USERNAME, NOT_USERNAME}) {
+    for (auto local_type : {NO_SERVER_DATA, SINGLE_USERNAME, NOT_USERNAME}) {
+      SCOPED_TRACE(testing::Message("server_type=")
+                   << server_type << " "
+                   << "local_type=" << local_type);
+      manager()->OnPasswordFormsParsed(&driver_, {} /* observed */);
+      EXPECT_CALL(mock_field_manager, GetFieldType(_, _))
+          .WillOnce(Return(local_type));
+
+      // Simulate filling of different forms on each iteration.
+      ++non_password_form.unique_renderer_id;
+      non_password_form.name += ASCIIToUTF16("1");  // for iOS.
+
+      FormStructure form_structure(non_password_form);
+      form_structure.field(0)->set_server_type(server_type);
+
+      bool should_be_filled =
+          server_type == SINGLE_USERNAME || local_type == SINGLE_USERNAME;
+      EXPECT_CALL(driver_, FillPasswordForm(_)).Times(should_be_filled ? 1 : 0);
+      manager()->ProcessAutofillPredictions(&driver_, {&form_structure});
+
+      Mock::VerifyAndClearExpectations(&client_);
+      Mock::VerifyAndClearExpectations(&mock_field_manager);
+    }
+  }
 }
+#endif
 
 TEST_F(PasswordManagerTest, FormSubmittedOnMainFrame) {
   EXPECT_CALL(client_, IsSavingAndFillingEnabled).WillRepeatedly(Return(true));