[autofill] Adding new API to request an interactive autocomplete UI flow.

chrome/renderer/autofill/autofill_agent.cc

Index: chrome/renderer/autofill/autofill_agent.cc
diff --git a/chrome/renderer/autofill/autofill_agent.cc b/chrome/renderer/autofill/autofill_agent.cc
index 8af890ab029926c37efce9cfb7d83c3699cd94ca..3bea16e293cdb3d5e92798e66b7a77acf3ca8893 100644
--- a/chrome/renderer/autofill/autofill_agent.cc
+++ b/chrome/renderer/autofill/autofill_agent.cc
@@ -24,6 +24,7 @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebRect.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebDocument.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFormControlElement.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebFormElement.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebInputEvent.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebNode.h"
@@ -157,6 +158,8 @@ bool AutofillAgent::OnMessageReceived(const IPC::Message& message) {
                         OnAcceptDataListSuggestion)
     IPC_MESSAGE_HANDLER(AutofillMsg_AcceptPasswordAutofillSuggestion,
                         OnAcceptPasswordAutofillSuggestion)
+    IPC_MESSAGE_HANDLER(AutofillMsg_RequestAutocompleteFinished,
+                        OnRequestAutocompleteFinished)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -211,6 +214,30 @@ void AutofillAgent::DidChangeScrollOffset(WebKit::WebFrame*) {
   Send(new AutofillHostMsg_HideAutofillPopup(routing_id()));
 }
 
+void AutofillAgent::didRequestAutocomplete(WebKit::WebFrame* frame,
+                                           const WebFormElement& form) {
+  FormData form_data;
+  if (!form_.isNull() ||
+      !WebFormElementToFormData(form,
+                                WebFormControlElement(),
+                                REQUIRE_AUTOCOMPLETE,
+                                EXTRACT_NONE,
+                                &form_data,
+                                NULL)) {
+    WebFormElement non_const_form(form);

[Ilya Sherman, 2012/11/02 02:56:57]

nit: Seems like we shouldn't be passing in a const form to this method if we're
just going to cast the constness away.

[Dan Beam, 2012/11/02 03:10:15]

So I removed const but there were issues with this, IIRC.  We're only copying a
pointer here, I think.  Is there a better way?  Just removing const didn't work
very well and passing by value wasn't so hot either, I think.

[Ilya Sherman, 2012/11/02 03:24:45]

What are the issues with removing const?  I agree that the pointer copy is safe,
but it seems rather weird to have "const WebFormElement" in the signature of the
method if we treat the form as non-const.

[Dan Beam, 2012/11/02 22:37:55]

So it looks like this is already being done every time you see:

  element_ = element;

in this file (3 times) as well as someone pointed out to me that even
WebNode::appendChild() is passed a const WebNode& (which seems that it'll
*always* be changed).

I know this is every-so-slightly misleading, but can you find any
counter-examples that'd actually make it worth going against this seemingly
pervasive convention?

[Ilya Sherman, 2012/11/02 22:47:36]

No counter-examples that I'm aware of, no.

+    non_const_form.finishRequestAutocomplete(
+        WebFormElement::AutocompleteResultError);
+    return;
+  }
+
+  // Cancel any pending Autofill requests and hide any currently showing popups.
+  ++autofill_query_id_;
+  HidePopups();
+
+  form_ = form;
+  Send(new AutofillHostMsg_RequestAutocomplete(routing_id(), form_data));
+}
+
 bool AutofillAgent::InputElementClicked(const WebInputElement& element,
                                         bool was_focused,
                                         bool is_focused) {
@@ -574,6 +601,15 @@ void AutofillAgent::OnAcceptPasswordAutofillSuggestion(const string16& value) {
   DCHECK(handled);
 }
 
+void AutofillAgent::OnRequestAutocompleteFinished(
+    WebFormElement::AutocompleteResult result) {
+  if (form_.isNull())
+    return;

[Ilya Sherman, 2012/11/02 02:56:57]

When can this happen?

[Dan Beam, 2012/11/02 03:10:15]

I'd think it's possible that the form *could* be removed from the DOM and set =
null, but maybe this doesn't cause it to be null if it's wrapped in a
WebFormElement?  Also what would happen if the tab's closed during this time -
would this ever fire?  Or what if a tab is refreshed mid-IPC?  We've had issues
with this in the past in WebUI code.

[Ilya Sherman, 2012/11/02 03:24:45]

The form could be removed from the DOM, but I don't think that would cause it to
become null, since we still have a reference to it from the event.
Can either of these actually happen, given that the dialog is modal?  Even if
they can, I think that either the AutofillAgent will be torn down, or the form
will remain non-null, since we're holding a reference to it.

[Dan Beam, 2012/11/02 21:06:08]

True, I definitely think you're right here and rescind that original guess.
The issue in WebUI is that while IPC was in flight the WebUI handlers were
complete destroyed and recreated so it looked like a valid IPC message coming at
a totally weird time. It seems possible if somebody in their script does:

  form.requestAutocomplete();
  location.reload();

(which I just did many times in various ways by slamming my CPU and
form.requestAutocomplete(); and didn't do anything)

I'll make a DCHECK() for now and change if people start hitting it.

+
+  form_.finishRequestAutocomplete(result);
+  form_.reset();
+}
+
 void AutofillAgent::ShowSuggestions(const WebInputElement& element,
                                     bool autofill_on_empty_values,
                                     bool requires_caret_at_end,
@@ -591,10 +627,7 @@ void AutofillAgent::ShowSuggestions(const WebInputElement& element,
        (element.selectionStart() != element.selectionEnd() ||
         element.selectionEnd() != static_cast<int>(value.length())))) {
     // Any popup currently showing is obsolete.
-    WebKit::WebView* web_view = render_view()->GetWebView();
-    if (web_view)
-      web_view->hidePopups();
-
+    HidePopups();
     return;
   }
 
@@ -694,4 +727,10 @@ void AutofillAgent::SetNodeText(const string16& value,
   node->setEditingValue(substring);
 }
 
+void AutofillAgent::HidePopups() {
+  WebKit::WebView* web_view = render_view()->GetWebView();
+  if (web_view)
+    web_view->hidePopups();
+}
+
 }  // namespace autofill