Password is not filed for matching usernames

The |WebViewClient::didAcceptAutocompleteSuggestion| method is no longer dispatched from the WebKit side, and had been replaced by |WebViewClient::didAcceptAutoFillSuggestion|.  The PasswordAutocompleteManager had not been updated to account for this.  Those updates are made in this patch.  Follow up is to remove the |WebViewClient::didAcceptAutocompleteSuggestion| method from WebKit.  Also, the |PasswordAutocompleteManager::FrameClosing| method was not getting properly dispatched to perform cleanup tasks.  This is fixed now.

BUG=68681
TEST=PasswordAutocompleteManagerTest.SuggestionSelect

Review URL: http://codereview.chromium.org/6262008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@71984 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 102 insertions, 83 deletions

diff --git a/chrome/renderer/autofill_helper.cc b/chrome/renderer/autofill_helper.cc
index f81f621..ec63c51 100644
--- a/chrome/renderer/autofill_helper.cc
+++ b/chrome/renderer/autofill_helper.cc
@@ -129,6 +129,8 @@ void AutoFillHelper::didSelectAutoFillSuggestion(const WebKit::WebNode& node,
                                                  const WebKit::WebString& label,
                                                  int unique_id) {
   DCHECK_GE(unique_id, 0);
+  if (password_autocomplete_manager_->DidSelectAutoFillSuggestion(node, value))
+    return;
 
   didClearAutoFillSelection(node);
   FillAutoFillFormData(node, unique_id, AUTOFILL_PREVIEW);
@@ -138,14 +140,6 @@ void AutoFillHelper::didClearAutoFillSelection(const WebKit::WebNode& node) {
   form_manager_.ClearPreviewedFormWithNode(node, was_query_node_autofilled_);
 }
 
-void AutoFillHelper::didAcceptAutocompleteSuggestion(
-    const WebKit::WebInputElement& user_element) {
-  bool result = password_autocomplete_manager_->FillPassword(user_element);
-  // Since this user name was selected from a suggestion list, we should always
-  // have password for it.
-  DCHECK(result);
-}
-
 void AutoFillHelper::removeAutocompleteSuggestion(
     const WebKit::WebString& name,
     const WebKit::WebString& value) {
@@ -186,7 +180,8 @@ void AutoFillHelper::TextFieldDidChangeImpl(
 void AutoFillHelper::textFieldDidReceiveKeyDown(
     const WebKit::WebInputElement& element,
     const WebKit::WebKeyboardEvent& event) {
-  password_autocomplete_manager_->TextFieldHandlingKeyDown(element, event);
+  if (password_autocomplete_manager_->TextFieldHandlingKeyDown(element, event))
+    return;
 
   if (event.windowsKeyCode == ui::VKEY_DOWN ||
       event.windowsKeyCode == ui::VKEY_UP)
diff --git a/chrome/renderer/autofill_helper.h b/chrome/renderer/autofill_helper.h
index 2b0fa9a..442b888 100644
--- a/chrome/renderer/autofill_helper.h
+++ b/chrome/renderer/autofill_helper.h
@@ -45,8 +45,6 @@ class AutoFillHelper : public RenderViewObserver,
                                            const WebKit::WebString& label,
                                            int unique_id);
   virtual void didClearAutoFillSelection(const WebKit::WebNode& node);
-  virtual void didAcceptAutocompleteSuggestion(
-      const WebKit::WebInputElement& element);
   virtual void removeAutocompleteSuggestion(const WebKit::WebString& name,
                                             const WebKit::WebString& value);
   virtual void textFieldDidEndEditing(const WebKit::WebInputElement& element);
diff --git a/chrome/renderer/password_autocomplete_manager.cc b/chrome/renderer/password_autocomplete_manager.cc
index 83e6334..93680eb 100644
--- a/chrome/renderer/password_autocomplete_manager.cc
+++ b/chrome/renderer/password_autocomplete_manager.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -197,16 +197,6 @@ PasswordAutocompleteManager::PasswordAutocompleteManager(
 PasswordAutocompleteManager::~PasswordAutocompleteManager() {
 }
 
-void PasswordAutocompleteManager::FrameClosing(const WebKit::WebFrame* frame) {
-  for (LoginToPasswordInfoMap::iterator iter = login_to_password_info_.begin();
-       iter != login_to_password_info_.end();) {
-    if (iter->first.document().frame() == frame)
-      login_to_password_info_.erase(iter++);
-    else
-      ++iter;
-  }
-}
-
 bool PasswordAutocompleteManager::TextFieldDidEndEditing(
     const WebKit::WebInputElement& element) {
   LoginToPasswordInfoMap::const_iterator iter =
@@ -261,8 +251,12 @@ bool PasswordAutocompleteManager::TextDidChangeInTextField(
   }
 
   // Don't inline autocomplete if the user is deleting, that would be confusing.
-  if (iter->second.backspace_pressed_last)
-    return false;
+  // But refresh the popup.  Note, since this is ours, return true to signal
+  // no further processing is required.
+  if (iter->second.backspace_pressed_last) {
+    ShowSuggestionPopup(iter->second.fill_data, username);
+    return true;
+  }
 
   WebKit::WebString name = element.nameForAutofill();
   if (name.isEmpty())
@@ -281,55 +275,41 @@ bool PasswordAutocompleteManager::TextDidChangeInTextField(
   return true;
 }
 
-void PasswordAutocompleteManager::TextFieldHandlingKeyDown(
+bool PasswordAutocompleteManager::TextFieldHandlingKeyDown(
     const WebKit::WebInputElement& element,
     const WebKit::WebKeyboardEvent& event) {
-
   LoginToPasswordInfoMap::iterator iter = login_to_password_info_.find(element);
   if (iter == login_to_password_info_.end())
-    return;
+    return false;
 
   int win_key_code = event.windowsKeyCode;
   iter->second.backspace_pressed_last =
       (win_key_code == ui::VKEY_BACK || win_key_code == ui::VKEY_DELETE);
+  return true;
 }
 
-bool PasswordAutocompleteManager::FillPassword(
-    const WebKit::WebInputElement& user_input) {
+bool PasswordAutocompleteManager::DidSelectAutoFillSuggestion(
+    const WebKit::WebNode& node,
+    const WebKit::WebString& value) {
+  if (!node.isElementNode())
+    return false;
+
+  WebKit::WebElement element(static_cast<const WebKit::WebElement&>(node));
+  if (!element.hasTagName("input"))
+    return false;
+
+  WebKit::WebInputElement user_input = element.to<WebKit::WebInputElement>();
   LoginToPasswordInfoMap::iterator iter =
       login_to_password_info_.find(user_input);
   if (iter == login_to_password_info_.end())
     return false;
-  const webkit_glue::PasswordFormFillData& fill_data =
-      iter->second.fill_data;
-  WebKit::WebInputElement password = iter->second.password_field;
-  WebKit::WebInputElement non_const_user_input(user_input);
-  return FillUserNameAndPassword(&non_const_user_input, &password,
-                                 fill_data, true, true);
-}
-
-void PasswordAutocompleteManager::PerformInlineAutocomplete(
-    const WebKit::WebInputElement& username_input,
-    const WebKit::WebInputElement& password_input,
-    const webkit_glue::PasswordFormFillData& fill_data) {
-  DCHECK(!fill_data.wait_for_username);
-
-  // We need non-const versions of the username and password inputs.
-  WebKit::WebInputElement username = username_input;
-  WebKit::WebInputElement password = password_input;
 
-  // Don't inline autocomplete if the caret is not at the end.
-  // TODO(jcivelli): is there a better way to test the caret location?
-  if (username.selectionStart() != username.selectionEnd() ||
-      username.selectionEnd() != static_cast<int>(username.value().length())) {
-    return;
-  }
-
-  // Show the popup with the list of available usernames.
-  ShowSuggestionPopup(fill_data, username);
-
-  // Fill the user and password field with the most relevant match.
-  FillUserNameAndPassword(&username, &password, fill_data, false, true);
+  // Set the incoming |value| in the text field and |FillUserNameAndPassword|
+  // will do the rest.
+  user_input.setValue(value);
+  const webkit_glue::PasswordFormFillData& fill_data = iter->second.fill_data;
+  WebKit::WebInputElement password = iter->second.password_field;
+  return FillUserNameAndPassword(&user_input, &password, fill_data, true, true);
 }
 
 void PasswordAutocompleteManager::SendPasswordForms(WebKit::WebFrame* frame,
@@ -386,6 +366,15 @@ void PasswordAutocompleteManager::DidFinishLoad(WebKit::WebFrame* frame) {
   SendPasswordForms(frame, true);
 }
 
+void PasswordAutocompleteManager::FrameDetached(WebKit::WebFrame* frame) {
+  FrameClosing(frame);
+}
+
+void PasswordAutocompleteManager::FrameWillClose(WebKit::WebFrame* frame) {
+  FrameClosing(frame);
+}
+
+
 ////////////////////////////////////////////////////////////////////////////////
 // PageClickListener implementation:
 
@@ -516,3 +505,37 @@ bool PasswordAutocompleteManager::FillUserNameAndPassword(
   SetElementAutofilled(password_element, true);
   return true;
 }
+
+void PasswordAutocompleteManager::PerformInlineAutocomplete(
+    const WebKit::WebInputElement& username_input,
+    const WebKit::WebInputElement& password_input,
+    const webkit_glue::PasswordFormFillData& fill_data) {
+  DCHECK(!fill_data.wait_for_username);
+
+  // We need non-const versions of the username and password inputs.
+  WebKit::WebInputElement username = username_input;
+  WebKit::WebInputElement password = password_input;
+
+  // Don't inline autocomplete if the caret is not at the end.
+  // TODO(jcivelli): is there a better way to test the caret location?
+  if (username.selectionStart() != username.selectionEnd() ||
+      username.selectionEnd() != static_cast<int>(username.value().length())) {
+    return;
+  }
+
+  // Show the popup with the list of available usernames.
+  ShowSuggestionPopup(fill_data, username);
+
+  // Fill the user and password field with the most relevant match.
+  FillUserNameAndPassword(&username, &password, fill_data, false, true);
+}
+
+void PasswordAutocompleteManager::FrameClosing(const WebKit::WebFrame* frame) {
+  for (LoginToPasswordInfoMap::iterator iter = login_to_password_info_.begin();
+       iter != login_to_password_info_.end();) {
+    if (iter->first.document().frame() == frame)
+      login_to_password_info_.erase(iter++);
+    else
+      ++iter;
+  }
+}
diff --git a/chrome/renderer/password_autocomplete_manager.h b/chrome/renderer/password_autocomplete_manager.h
index f2963d1..1ecfd35 100644
--- a/chrome/renderer/password_autocomplete_manager.h
+++ b/chrome/renderer/password_autocomplete_manager.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -28,30 +28,17 @@ class PasswordAutocompleteManager : public RenderViewObserver,
   explicit PasswordAutocompleteManager(RenderView* render_view);
   virtual ~PasswordAutocompleteManager();
 
-  // Invoked when the passed frame is closing.  Gives us a chance to clear any
-  // reference we may have to elements in that frame.
-  void FrameClosing(const WebKit::WebFrame* frame);
-
-  // Fills the password associated with |user_input|, using its current value
-  // as the actual user name.  Returns true if the password field was filled,
-  // false otherwise, typically if there was no matching suggestions for the
-  // currently typed username.
-  bool FillPassword(const WebKit::WebInputElement& user_input);
-
-  // Fills |login_input| and |password| with the most relevant suggestion from
-  // |fill_data| and shows a popup with other suggestions.
-  void PerformInlineAutocomplete(
-      const WebKit::WebInputElement& username,
-      const WebKit::WebInputElement& password,
-      const webkit_glue::PasswordFormFillData& fill_data);
-
   // WebViewClient editor related calls forwarded by the RenderView.
   // If they return true, it indicates the event was consumed and should not
   // be used for any other autofill activity.
   bool TextFieldDidEndEditing(const WebKit::WebInputElement& element);
   bool TextDidChangeInTextField(const WebKit::WebInputElement& element);
-  void TextFieldHandlingKeyDown(const WebKit::WebInputElement& element,
+  bool TextFieldHandlingKeyDown(const WebKit::WebInputElement& element,
                                 const WebKit::WebKeyboardEvent& event);
+  // Fills the password associated with user name |value|. Returns true if the
+  // username and password fields were filled, false otherwise.
+  bool DidSelectAutoFillSuggestion(const WebKit::WebNode& node,
+                                   const WebKit::WebString& value);
 
  private:
   friend class PasswordAutocompleteManagerTest;
@@ -64,16 +51,19 @@ class PasswordAutocompleteManager : public RenderViewObserver,
   };
   typedef std::map<WebKit::WebElement, PasswordInfo> LoginToPasswordInfoMap;
 
-  // RenderView::Observer implementation.
+  // RenderViewObserver:
   virtual bool OnMessageReceived(const IPC::Message& message);
   virtual void DidFinishDocumentLoad(WebKit::WebFrame* frame);
   virtual void DidFinishLoad(WebKit::WebFrame* frame);
+  virtual void FrameDetached(WebKit::WebFrame* frame);
+  virtual void FrameWillClose(WebKit::WebFrame* frame);
 
-  // PageClickListener implementation:
+  // PageClickListener:
   virtual bool InputElementClicked(const WebKit::WebInputElement& element,
                                    bool was_focused,
                                    bool is_focused);
 
+  // RenderView IPC handlers:
   void OnFillPasswordForm(
       const webkit_glue::PasswordFormFillData& form_data);
 
@@ -97,6 +87,17 @@ class PasswordAutocompleteManager : public RenderViewObserver,
       bool exact_username_match,
       bool set_selection);
 
+  // Fills |login_input| and |password| with the most relevant suggestion from
+  // |fill_data| and shows a popup with other suggestions.
+  void PerformInlineAutocomplete(
+      const WebKit::WebInputElement& username,
+      const WebKit::WebInputElement& password,
+      const webkit_glue::PasswordFormFillData& fill_data);
+
+  // Invoked when the passed frame is closing.  Gives us a chance to clear any
+  // reference we may have to elements in that frame.
+  void FrameClosing(const WebKit::WebFrame* frame);
+
   // The logins we have filled so far with their associated info.
   LoginToPasswordInfoMap login_to_password_info_;
 
diff --git a/chrome/renderer/password_autocomplete_manager_unittest.cc b/chrome/renderer/password_autocomplete_manager_unittest.cc
index 0072870..d4609a7 100644
--- a/chrome/renderer/password_autocomplete_manager_unittest.cc
+++ b/chrome/renderer/password_autocomplete_manager_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -325,9 +325,11 @@ TEST_F(PasswordAutocompleteManagerTest, SuggestionSelect) {
 
   // To simulate a selection in the suggestion drop-down we just mimick what the
   // WebView does: it sets the element value then calls
-  // didAcceptAutocompleteSuggestion on the renderer.
-  username_element_.setValue(ASCIIToUTF16(kAliceUsername));
-  autofill_helper_->didAcceptAutocompleteSuggestion(username_element_);
+  // didSelectAutoFillSuggestion on the renderer.
+  autofill_helper_->didSelectAutoFillSuggestion(username_element_,
+                                                ASCIIToUTF16(kAliceUsername),
+                                                WebKit::WebString(),
+                                                0);
 
   // Autocomplete should have kicked in.
   CheckTextFieldsState(kAliceUsername, true, kAlicePassword, true);