Fix check for user gesture on password autofill

components/autofill/content/renderer/password_autofill_agent.cc

Index: components/autofill/content/renderer/password_autofill_agent.cc
diff --git a/components/autofill/content/renderer/password_autofill_agent.cc b/components/autofill/content/renderer/password_autofill_agent.cc
index 81a822ad84d6f5525602acdad47cb59c784ed9a1..6dfcb4401707d8fb5fafcb206109c20cbd96e0be 100644
--- a/components/autofill/content/renderer/password_autofill_agent.cc
+++ b/components/autofill/content/renderer/password_autofill_agent.cc
@@ -201,15 +201,45 @@ PasswordAutofillAgent::PasswordAutofillAgent(content::RenderView* render_view)
     : content::RenderViewObserver(render_view),
       usernames_usage_(NOTHING_TO_AUTOFILL),
       web_view_(render_view->GetWebView()),
-      gesture_handler_(new AutofillWebUserGestureHandler(this)),
-      user_gesture_occurred_(false),
       weak_ptr_factory_(this) {
-  blink::WebUserGestureIndicator::setHandler(gesture_handler_.get());
 }
 
-PasswordAutofillAgent::~PasswordAutofillAgent() {
-  DCHECK(gesture_handler_.get());
-  blink::WebUserGestureIndicator::setHandler(NULL);
+PasswordAutofillAgent::~PasswordAutofillAgent() {}
+
+PasswordAutofillAgent::PasswordValueGatekeeper::PasswordValueGatekeeper()
+    : was_user_gesture_seen_(false) {}
+
+PasswordAutofillAgent::PasswordValueGatekeeper::~PasswordValueGatekeeper() {}
+
+void PasswordAutofillAgent::PasswordValueGatekeeper::RegisterElement(
+    blink::WebInputElement& element) {
+  if (was_user_gesture_seen_)
+    ShowValue(element);
+  else
+    elements_.push_back(element);
+}
+
+void PasswordAutofillAgent::PasswordValueGatekeeper::ShowValue(
+    blink::WebInputElement& element) {
+  if (!element.isNull() && !element.suggestedValue().isNull())
+    element.setValue(element.suggestedValue(), true);
+}
+
+void PasswordAutofillAgent::PasswordValueGatekeeper::OnGesture() {
+  was_user_gesture_seen_ = true;
+
+  for (std::vector<blink::WebInputElement>::iterator iter = elements_.begin();

[Ilya Sherman, 2014/02/14 23:07:33]

Optional nit: "iter" -> "it" (for consistency with most of the surrounding code)

[vabr (Chromium), 2014/02/17 14:24:56]

Done.

+       iter != elements_.end();
+       ++iter) {
+    ShowValue(*iter);
+  }
+
+  elements_.clear();
+}
+
+void PasswordAutofillAgent::PasswordValueGatekeeper::Reset() {
+  was_user_gesture_seen_ = false;
+  elements_.clear();
 }

[Ilya Sherman, 2014/02/14 23:07:33]

nit: Please order the implementation code within the .cc file to match the order
of the declarations within the .h file.

[vabr (Chromium), 2014/02/17 14:24:56]

Done for the definitions of the added methods (the existing ones of
PasswordAutofillAgent don't seem to match the declaration order.

 
 bool PasswordAutofillAgent::TextFieldDidEndEditing(
@@ -477,6 +507,10 @@ void PasswordAutofillAgent::WillSubmitForm(blink::WebFrame* frame,
   }
 }
 
+void PasswordAutofillAgent::ProcessingUserGestureEvent() {
+  gatekeeper_.OnGesture();
+}
+
 blink::WebFrame* PasswordAutofillAgent::CurrentOrChildFrameWithSavedForms(
     const blink::WebFrame* current_frame) {
   for (FrameToPasswordFormMap::const_iterator it =
@@ -534,10 +568,9 @@ void PasswordAutofillAgent::DidStartProvisionalLoad(blink::WebFrame* frame) {
     // Clear the whole map during main frame navigation.
     provisionally_saved_forms_.clear();
 
-    // We are navigating, se we need to wait for a new user gesture before
+    // New navigation means the need to wait for a new user gesture before

[Ilya Sherman, 2014/02/14 23:07:33]

nit: Suggested rephrasing: "This is a new navigation, so require a new user
gesture before filling in passwords."

[vabr (Chromium), 2014/02/17 14:24:56]

Done.

     // filling in passwords.
-    user_gesture_occurred_ = false;
-    gesture_handler_->clearElements();
+    gatekeeper_.Reset();
   }
 }
 
@@ -772,16 +805,12 @@ bool PasswordAutofillAgent::FillUserNameAndPassword(
     return false;
   }
 
-  // If a user gesture has not occurred, we setup a handler to listen for the
-  // next user gesture, at which point we then fill in the password. This is to
-  // make sure that we do not fill in the DOM with a password until we believe
-  // the user is intentionally interacting with the page.
-  if (!user_gesture_occurred_) {
-    gesture_handler_->addElement(*password_element);
-    password_element->setSuggestedValue(password);
-  } else {
-    password_element->setValue(password, true);
-  }
+  // We wait with filling in the password until a user gesture occurred. This is

[Ilya Sherman, 2014/02/14 23:07:33]

nit: "We wait with filling in" -> "Wait to fill in"

[Ilya Sherman, 2014/02/14 23:07:33]

nit: "occurred" -> "occurs"

[vabr (Chromium), 2014/02/17 14:24:56]

Done.

[vabr (Chromium), 2014/02/17 14:24:56]

Done.

+  // to make sure that we do not fill in the DOM with a password until we
+  // believe the user is intentionally interacting with the page.
+  password_element->setSuggestedValue(password);
+  gatekeeper_.RegisterElement(*password_element);
+
   // Note: Don't call SetElementAutofilled() here, as that dispatches an
   // onChange event in JavaScript, which is not appropriate for the password
   // element if a user gesture has not yet occured.
@@ -857,23 +886,4 @@ bool PasswordAutofillAgent::FindLoginInfo(const blink::WebNode& node,
   return true;
 }
 
-void PasswordAutofillAgent::AutofillWebUserGestureHandler::onGesture() {
-  agent_->set_user_gesture_occurred(true);
-
-  std::vector<blink::WebInputElement>::iterator iter;
-  for (iter = elements_.begin(); iter != elements_.end(); ++iter) {
-    if (!iter->isNull() && !iter->suggestedValue().isNull())
-      iter->setValue(iter->suggestedValue(), true);
-  }
-
-  elements_.clear();
-}
-
-PasswordAutofillAgent::AutofillWebUserGestureHandler::
-    AutofillWebUserGestureHandler(PasswordAutofillAgent* agent)
-    : agent_(agent) {}
-
-PasswordAutofillAgent::AutofillWebUserGestureHandler::
-    ~AutofillWebUserGestureHandler() {}
-
 }  // namespace autofill