Keep generated password unmasked until complete deletion

Deleting characters from a generated password until only 3 or fewer
characters are left, results in the password not counting as generated
anymore.

Until now, the leftover characters were masked immediately after the
password length was shorter than 4 chars. This CL changes the behavior
to keep the password unmasked until the user completely deletes it or
it loses focus.

Bug: 874822,899798
Change-Id: I19a4116374388d51a5d5bafcba6d45076dde2800
Reviewed-on: https://chromium-review.googlesource.com/c/1305555Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605373}

chrome/renderer/autofill/password_generation_agent_browsertest.cc

@@ -86,6 +86,10 @@ class PasswordGenerationAgentTest : public ChromeRenderViewTest {
             base::BindRepeating([](mojo::ScopedInterfaceEndpointHandle handle) {
               handle.reset();
             }));
+
+    // Necessary for focus changes to work correctly and dispatch blur events
+    // when a field was previously focused.
+    GetWebWidget()->SetFocus(true);
   }
 
   void TearDown() override {
@@ -1088,4 +1092,105 @@ TEST_F(PasswordGenerationAgentTestForHtmlAnnotation, AnnotateForm) {
   }
 }
 
+TEST_F(PasswordGenerationAgentTest, PasswordUnmaskedUntilCompleteDeletion) {
+  LoadHTMLWithUserGesture(kAccountCreationFormHTML);
+  SetNotBlacklistedMessage(password_generation_, kAccountCreationFormHTML);
+  SetAccountCreationFormsDetectedMessage(password_generation_,
+                                         GetMainFrame()->GetDocument(), 0, 1);
+
+  constexpr char kGenerationElementId[] = "first_password";
+
+  // Generate a new password.
+  FocusField(kGenerationElementId);
+  base::string16 password = base::ASCIIToUTF16("random_password");
+  EXPECT_CALL(fake_pw_client_,
+              PresaveGeneratedPassword(testing::Field(
+                  &autofill::PasswordForm::password_value, password)));
+  password_generation_->GeneratedPasswordAccepted(password);
+  fake_pw_client_.Flush();
+  testing::Mock::VerifyAndClearExpectations(&fake_pw_client_);
+
+  // Delete characters of the generated password until only
+  // |kMinimumLengthForEditedPassword| - 1 chars remain.
+  fake_pw_client_.reset_called_automatic_generation_status_changed_true();
+  FocusField(kGenerationElementId);
+  EXPECT_CALL(fake_pw_client_, PasswordNoLongerGenerated(testing::_));
+  size_t max_chars_to_delete =
+      password.length() -
+      PasswordGenerationAgent::kMinimumLengthForEditedPassword + 1;
+  for (size_t i = 0; i < max_chars_to_delete; ++i)
+    SimulateUserTypingASCIICharacter(ui::VKEY_BACK, false);
+  base::RunLoop().RunUntilIdle();
+  // The remaining characters no longer count as a generated password, so
+  // generation should be offered again.
+  EXPECT_TRUE(GetCalledAutomaticGenerationStatusChangedTrue());
+
+  // Check that the characters remain unmasked.
+  WebDocument document = GetMainFrame()->GetDocument();
+  blink::WebElement element =
+      document.GetElementById(blink::WebString::FromUTF8(kGenerationElementId));
+  ASSERT_FALSE(element.IsNull());
+  blink::WebInputElement input = element.To<WebInputElement>();
+  EXPECT_TRUE(input.ShouldRevealPassword());
+
+  // Delete the rest of the characters. The field should now mask new
+  // characters.
+  for (size_t i = 0;
+       i < PasswordGenerationAgent::kMinimumLengthForEditedPassword; ++i)
+    SimulateUserTypingASCIICharacter(ui::VKEY_BACK, false);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(input.ShouldRevealPassword());
+}
+
+TEST_F(PasswordGenerationAgentTest, ShortPasswordMaskedAfterChangingFocus) {
+  LoadHTMLWithUserGesture(kPasswordFormAndSpanHTML);
+  SetNotBlacklistedMessage(password_generation_, kPasswordFormAndSpanHTML);
+  SetAccountCreationFormsDetectedMessage(password_generation_,
+                                         GetMainFrame()->GetDocument(), 0, 1);
+
+  constexpr char kGenerationElementId[] = "password";
+
+  // Generate a new password.
+  FocusField(kGenerationElementId);
+  base::string16 password = base::ASCIIToUTF16("random_password");
+  EXPECT_CALL(fake_pw_client_,
+              PresaveGeneratedPassword(testing::Field(
+                  &autofill::PasswordForm::password_value, password)));
+  password_generation_->GeneratedPasswordAccepted(password);
+  fake_pw_client_.Flush();
+  testing::Mock::VerifyAndClearExpectations(&fake_pw_client_);
+
+  // Delete characters of the generated password until only
+  // |kMinimumLengthForEditedPassword| - 1 chars remain.
+  fake_pw_client_.reset_called_automatic_generation_status_changed_true();
+  FocusField(kGenerationElementId);
+  EXPECT_CALL(fake_pw_client_, PasswordNoLongerGenerated(testing::_));
+  size_t max_chars_to_delete =
+      password.length() -
+      PasswordGenerationAgent::kMinimumLengthForEditedPassword + 1;
+  for (size_t i = 0; i < max_chars_to_delete; ++i)
+    SimulateUserTypingASCIICharacter(ui::VKEY_BACK, false);
+  // The remaining characters no longer count as a generated password, so
+  // generation should be offered again.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(GetCalledAutomaticGenerationStatusChangedTrue());
+
+  // Check that the characters remain unmasked.
+  WebDocument document = GetMainFrame()->GetDocument();
+  blink::WebElement element =
+      document.GetElementById(blink::WebString::FromUTF8(kGenerationElementId));
+  ASSERT_FALSE(element.IsNull());
+  blink::WebInputElement input = element.To<WebInputElement>();
+  EXPECT_TRUE(input.ShouldRevealPassword());
+
+  // Focus another element on the page. The password should be masked.
+  ASSERT_TRUE(SimulateElementClick("span"));
+  EXPECT_FALSE(input.ShouldRevealPassword());
+
+  // Focus the password field again. As the remaining characters are not
+  // a generated password, they should remain masked.
+  FocusField(kGenerationElementId);
+  EXPECT_FALSE(input.ShouldRevealPassword());
+}
+
 }  // namespace autofill

components/autofill/content/renderer/autofill_agent.cc

@@ -313,6 +313,8 @@ void AutofillAgent::Shutdown() {
 void AutofillAgent::TextFieldDidEndEditing(const WebInputElement& element) {
   GetAutofillDriver()->DidEndTextFieldEditing();
   password_autofill_agent_->DidEndTextFieldEditing();
+  if (password_generation_agent_)
+    password_generation_agent_->DidEndTextFieldEditing(element);
 }
 
 void AutofillAgent::SetUserGestureRequired(bool required) {

components/autofill/content/renderer/password_generation_agent.cc

@@ -290,6 +290,7 @@ void PasswordGenerationAgent::OnFieldAutofilled(
     password_generation::LogPasswordGenerationEvent(
         password_generation::PASSWORD_DELETED_BY_AUTOFILLING);
     PasswordNoLongerGenerated();
+    generation_element_.SetShouldRevealPassword(false);
   }
 }
 
@@ -558,11 +559,7 @@ bool PasswordGenerationAgent::SetUpUserTriggeredGeneration() {
 
 bool PasswordGenerationAgent::FocusedNodeHasChanged(
     const blink::WebNode& node) {
-  if (!generation_element_.IsNull())
-    generation_element_.SetShouldRevealPassword(false);
-
   if (node.IsNull() || !node.IsElementNode()) {
-    AutomaticGenerationStatusChanged(false);
     return false;
   }
 
@@ -576,7 +573,6 @@ bool PasswordGenerationAgent::FocusedNodeHasChanged(
   if (element && element->IsPasswordFieldForAutofill())
     last_focused_password_element_ = *element;
   if (!element || *element != generation_element_) {
-    AutomaticGenerationStatusChanged(false);
     return false;
   }
 
@@ -584,6 +580,8 @@ bool PasswordGenerationAgent::FocusedNodeHasChanged(
     if (generation_element_.Value().length() <
         kMinimumLengthForEditedPassword) {
       PasswordNoLongerGenerated();
+      if (generation_element_.Value().IsEmpty())
+        generation_element_.SetShouldRevealPassword(false);
     } else {
       generation_element_.SetShouldRevealPassword(true);
       ShowEditingPopup();
@@ -604,6 +602,14 @@ bool PasswordGenerationAgent::FocusedNodeHasChanged(
   return false;
 }
 
+void PasswordGenerationAgent::DidEndTextFieldEditing(
+    const blink::WebInputElement& element) {
+  if (!element.IsNull() && element == generation_element_) {
+    AutomaticGenerationStatusChanged(false);
+    generation_element_.SetShouldRevealPassword(false);
+  }
+}
+
 bool PasswordGenerationAgent::TextDidChangeInTextField(
     const WebInputElement& element) {
   if (element != generation_element_) {
@@ -623,6 +629,9 @@ bool PasswordGenerationAgent::TextDidChangeInTextField(
     return false;
   }
 
+  if (element.Value().IsEmpty())
+    generation_element_.SetShouldRevealPassword(false);
+
   if (!password_is_generated_ &&
       element.Value().length() > kMaximumCharsForGenerationOffer) {
     // User has rejected the feature and has started typing a password.
@@ -705,7 +714,6 @@ void PasswordGenerationAgent::PasswordNoLongerGenerated() {
   // Do not treat the password as generated, either here or in the browser.
   password_is_generated_ = false;
   password_edited_ = false;
-  generation_element_.SetShouldRevealPassword(false);
   for (WebInputElement& password : generation_form_data_->password_elements)
     password.SetAutofillState(WebAutofillState::kNotFilled);
   password_generation::LogPasswordGenerationEvent(

components/autofill/content/renderer/password_generation_agent.h

@@ -67,6 +67,13 @@ class PasswordGenerationAgent : public content::RenderFrameObserver,
   // Returns true if the newly focused node caused the generation UI to show.
   bool FocusedNodeHasChanged(const blink::WebNode& node);
 
+  // Event forwarded by AutofillAgent from WebAutofillClient, informing that
+  // the text field editing has ended, which means that the field is not
+  // focused anymore. This is required for Android, where moving focus
+  // to a non-focusable element doesn't result in |FocusedNodeHasChanged|
+  // being called.
+  void DidEndTextFieldEditing(const blink::WebInputElement& element);
+
   // Called when new form controls are inserted.
   void OnDynamicFormsSeen();