[Password Generation] Notify PasswordGenerationAgent about focus change when...

[Password Generation] Notify PasswordGenerationAgent about focus change when the user triggers the context menu

The generation agent tracks last password field that had focus. The field is used for user-triggered password generation. Before this CL, the generation agent wasn't notified about focus change when the user triggers the context menu (only when the user makes left click on the field). This CL propagates right click events to autofill.

Bug: 784840
Change-Id: I31e1d793350f3bd9a6b8a3d61d9fcff6e61acdd8
Reviewed-on: https://chromium-review.googlesource.com/768811Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Roger McFarlane <rogerm@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517403}

chrome/renderer/autofill/password_generation_agent_browsertest.cc

@@ -121,13 +121,8 @@ class PasswordGenerationAgentTest : public ChromeRenderViewTest {
     return fake_pw_client_.called_show_pw_generation_popup();
   }
 
-  void ShowGenerationPopUpManually(const char* element_id) {
-    FocusField(element_id);
-    password_generation_->UserTriggeredGeneratePassword();
-  }
-
-  void SelectGenerationFallback(const char* element_id) {
-    FocusField(element_id);
+  void SelectGenerationFallbackInContextMenu(const char* element_id) {
+    SimulateElementRightClick(element_id);
     password_generation_->UserSelectedManualGenerationOption();
   }
 
@@ -141,7 +136,7 @@ class PasswordGenerationAgentTest : public ChromeRenderViewTest {
         mojom::PasswordManagerClientAssociatedRequest(std::move(handle)));
   }
 
-  void SetManualGenerationFallback() {
+  void EnableManualGenerationFallback() {
     scoped_feature_list_.InitAndEnableFeature(
         password_manager::features::kEnableManualFallbacksGeneration);
   }
@@ -666,14 +661,14 @@ TEST_F(PasswordGenerationAgentTest, ChangePasswordFormDetectionTest) {
 
 TEST_F(PasswordGenerationAgentTest, ManualGenerationInFormTest) {
   LoadHTMLWithUserGesture(kAccountCreationFormHTML);
-  ShowGenerationPopUpManually("first_password");
+  SelectGenerationFallbackInContextMenu("first_password");
   ExpectGenerationAvailable("first_password", true);
   ExpectGenerationAvailable("second_password", false);
 }
 
 TEST_F(PasswordGenerationAgentTest, ManualGenerationNoFormTest) {
   LoadHTMLWithUserGesture(kAccountCreationNoForm);
-  ShowGenerationPopUpManually("first_password");
+  SelectGenerationFallbackInContextMenu("first_password");
   ExpectGenerationAvailable("first_password", true);
   ExpectGenerationAvailable("second_password", false);
 }
@@ -684,7 +679,7 @@ TEST_F(PasswordGenerationAgentTest, ManualGenerationChangeFocusTest) {
   // generate password, even if focused element has changed.
   LoadHTMLWithUserGesture(kAccountCreationFormHTML);
   FocusField("first_password");
-  ShowGenerationPopUpManually("username" /* current focus */);
+  SelectGenerationFallbackInContextMenu("username" /* current focus */);
   ExpectGenerationAvailable("first_password", true);
   ExpectGenerationAvailable("second_password", false);
 }
@@ -701,7 +696,7 @@ TEST_F(PasswordGenerationAgentTest, PresavingGeneratedPassword) {
     LoadHTMLWithUserGesture(test_case.form);
     // To be able to work with input elements outside <form>'s, use manual
     // generation.
-    ShowGenerationPopUpManually(test_case.generation_element);
+    SelectGenerationFallbackInContextMenu(test_case.generation_element);
     ExpectGenerationAvailable(test_case.generation_element, true);
 
     base::string16 password = base::ASCIIToUTF16("random_password");
@@ -727,7 +722,7 @@ TEST_F(PasswordGenerationAgentTest, PresavingGeneratedPassword) {
 
 TEST_F(PasswordGenerationAgentTest, FallbackForSaving) {
   LoadHTMLWithUserGesture(kAccountCreationFormHTML);
-  ShowGenerationPopUpManually("first_password");
+  SelectGenerationFallbackInContextMenu("first_password");
   ExpectGenerationAvailable("first_password", true);
   EXPECT_EQ(0, fake_driver_.called_show_manual_fallback_for_saving_count());
   password_generation_->GeneratedPasswordAccepted(
@@ -859,7 +854,6 @@ TEST_F(PasswordGenerationAgentTest, JavascriptClearedTheField) {
 }
 
 TEST_F(PasswordGenerationAgentTest, GenerationFallbackTest) {
-  SetManualGenerationFallback();
   LoadHTMLWithUserGesture(kAccountCreationFormHTML);
   WebDocument document = GetMainFrame()->GetDocument();
   WebElement element =
@@ -867,8 +861,15 @@ TEST_F(PasswordGenerationAgentTest, GenerationFallbackTest) {
   ASSERT_FALSE(element.IsNull());
   WebInputElement first_password_element = element.To<WebInputElement>();
   EXPECT_TRUE(first_password_element.Value().IsNull());
-  SelectGenerationFallback("first_password");
+  SelectGenerationFallbackInContextMenu("first_password");
   EXPECT_TRUE(first_password_element.Value().IsNull());
 }
 
+TEST_F(PasswordGenerationAgentTest, GenerationFallback_NoFocusedElement) {
+  // Checks the fallback doesn't cause a crash just in case no password element
+  // had focus so far.
+  LoadHTMLWithUserGesture(kAccountCreationFormHTML);
+  password_generation_->UserSelectedManualGenerationOption();
+}
+
 }  // namespace autofill

components/autofill/content/renderer/password_generation_agent.cc

@@ -465,9 +465,9 @@ void PasswordGenerationAgent::DetermineGenerationElement() {
   }
 }
 
-void PasswordGenerationAgent::SetUpUserTriggeredGeneration() {
+bool PasswordGenerationAgent::SetUpUserTriggeredGeneration() {
   if (last_focused_password_element_.IsNull() || !render_frame())
-    return;
+    return false;
 
   blink::WebFormElement form = last_focused_password_element_.Form();
   std::unique_ptr<PasswordForm> password_form;
@@ -479,14 +479,14 @@ void PasswordGenerationAgent::SetUpUserTriggeredGeneration() {
     const blink::WebLocalFrame& frame = *render_frame()->GetWebFrame();
     blink::WebDocument doc = frame.GetDocument();
     if (doc.IsNull())
-      return;
+      return false;
     password_form = password_agent_->GetPasswordFormFromUnownedInputElements();
     control_elements =
         form_util::GetUnownedFormFieldElements(doc.All(), nullptr);
   }
 
   if (!password_form)
-    return;
+    return false;
 
   generation_element_ = last_focused_password_element_;
   std::vector<blink::WebInputElement> password_elements;
@@ -501,6 +501,7 @@ void PasswordGenerationAgent::SetUpUserTriggeredGeneration() {
   generation_form_data_.reset(new AccountCreationFormData(
       make_linked_ptr(password_form.release()), password_elements));
   is_manually_triggered_ = true;
+  return true;
 }
 
 bool PasswordGenerationAgent::FocusedNodeHasChanged(
@@ -579,7 +580,7 @@ bool PasswordGenerationAgent::TextDidChangeInTextField(
 }
 
 void PasswordGenerationAgent::ShowGenerationPopup() {
-  if (!render_frame())
+  if (!render_frame() || generation_element_.IsNull())
     return;
   LogMessage(Logger::STRING_GENERATION_RENDERER_SHOW_GENERATION_POPUP);
   GetPasswordManagerClient()->ShowPasswordGenerationPopup(
@@ -623,15 +624,16 @@ void PasswordGenerationAgent::PasswordNoLongerGenerated() {
 }
 
 void PasswordGenerationAgent::UserTriggeredGeneratePassword() {
-  SetUpUserTriggeredGeneration();
-  ShowGenerationPopup();
+  if (SetUpUserTriggeredGeneration())
+    ShowGenerationPopup();
 }
 
 void PasswordGenerationAgent::UserSelectedManualGenerationOption() {
-  SetUpUserTriggeredGeneration();
-  last_focused_password_element_.SetAutofillValue(blink::WebString());
-  last_focused_password_element_.SetAutofilled(false);
-  ShowGenerationPopup();
+  if (SetUpUserTriggeredGeneration()) {
+    last_focused_password_element_.SetAutofillValue(blink::WebString());
+    last_focused_password_element_.SetAutofilled(false);
+    ShowGenerationPopup();
+  }
 }
 
 const mojom::PasswordManagerDriverPtr&

components/autofill/content/renderer/password_generation_agent.h

@@ -112,8 +112,9 @@ class PasswordGenerationAgent : public content::RenderFrameObserver,
   void DetermineGenerationElement();
 
   // Helper function which takes care of the form processing and collecting the
-  // information which is required to show the generation popup.
-  void SetUpUserTriggeredGeneration();
+  // information which is required to show the generation popup. Returns true if
+  // all required information is collected.
+  bool SetUpUserTriggeredGeneration();
 
   // Show password generation UI anchored at |generation_element_|.
   void ShowGenerationPopup();

content/renderer/input/render_widget_input_handler.cc

@@ -394,7 +394,7 @@ void RenderWidgetInputHandler::HandleInputEvent(
   // Virtual keyboard is not supported, so react to focus change immediately.
   if (processed != WebInputEventResult::kNotHandled &&
       (input_event.GetType() == WebInputEvent::kTouchEnd ||
-       input_event.GetType() == WebInputEvent::kMouseUp)) {
+       input_event.GetType() == WebInputEvent::kMouseDown)) {
     delegate_->FocusChangeComplete();
   }
 #endif