Fix check for user gesture on password autofill

components/autofill/content/renderer/password_autofill_agent.cc

 // Copyright 2013 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.
 
 #include "components/autofill/content/renderer/password_autofill_agent.h"
 
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
@@ skipping 183 matching lines @@
 
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 // PasswordAutofillAgent, public:
 
 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(
     const blink::WebInputElement& element) {
   LoginToPasswordInfoMap::const_iterator iter =
       login_to_password_info_.find(element);
   if (iter == login_to_password_info_.end())
     return false;
 
   const PasswordFormFillData& fill_data =
       iter->second.fill_data;
@@ skipping 246 matching lines @@
     // the frame starts loading. If there are redirects that cause a new
     // RenderView to be instantiated (such as redirects to the WebStore)
     // we will never get to finish the load.
     Send(new AutofillHostMsg_PasswordFormSubmitted(routing_id(),
                                                    *submitted_form));
     // Remove reference since we have already submitted this form.
     provisionally_saved_forms_.erase(frame);
   }
 }
 
+void PasswordAutofillAgent::ProcessingUserGestureEvent() {
+  gatekeeper_.OnGesture();
+}
+
 blink::WebFrame* PasswordAutofillAgent::CurrentOrChildFrameWithSavedForms(
     const blink::WebFrame* current_frame) {
   for (FrameToPasswordFormMap::const_iterator it =
            provisionally_saved_forms_.begin();
        it != provisionally_saved_forms_.end();
        ++it) {
     blink::WebFrame* form_frame = it->first;
     // The check that the returned frame is related to |current_frame| is mainly
     // for double-checking. There should not be any unrelated frames in
     // |provisionally_saved_forms_|, because the map is cleared after
@@ skipping 37 matching lines @@
               !password_form->password_value.empty()) {
             Send(new AutofillHostMsg_PasswordFormSubmitted(
                 routing_id(), *password_form));
           }
         }
       }
     }
     // 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();
   }
 }
 
 void PasswordAutofillAgent::OnFillPasswordForm(
     const PasswordFormFillData& form_data) {
   if (usernames_usage_ == NOTHING_TO_AUTOFILL) {
     if (form_data.other_possible_usernames.size())
       usernames_usage_ = OTHER_POSSIBLE_USERNAMES_PRESENT;
     else if (usernames_usage_ == NOTHING_TO_AUTOFILL)
       usernames_usage_ = OTHER_POSSIBLE_USERNAMES_ABSENT;
@@ skipping 214 matching lines @@
     if (set_selection) {
       username_element->setSelectionRange(current_username.length(),
                                           username.length());
     }
   } else if (current_username != username) {
     // If the username can't be filled and it doesn't match a saved password
     // as is, don't autofill a password.
     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.
   password_element->setAutofilled(true);
   return true;
 }
 
 void PasswordAutofillAgent::PerformInlineAutocomplete(
     const blink::WebInputElement& username_input,
     const blink::WebInputElement& password_input,
@@ skipping 55 matching lines @@
   blink::WebInputElement input = element.to<blink::WebInputElement>();
   LoginToPasswordInfoMap::iterator iter = login_to_password_info_.find(input);
   if (iter == login_to_password_info_.end())
     return false;
 
   *found_input = input;
   *found_password = iter->second;
   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