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..9bc139a9c0b6d2ecad85bc5e4b50f609a6bf1261 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::OnUserGesture() {
+  was_user_gesture_seen_ = true;
+
+  for (std::vector<blink::WebInputElement>::iterator it = elements_.begin();
+       it != elements_.end();
+       ++it) {
+    ShowValue(&(*it));
+  }
+
+  elements_.clear();
+}
+
+void PasswordAutofillAgent::PasswordValueGatekeeper::Reset() {
+  was_user_gesture_seen_ = false;
+  elements_.clear();
+}
+
+void PasswordAutofillAgent::PasswordValueGatekeeper::ShowValue(
+    blink::WebInputElement* element) {
+  if (!element->isNull() && !element->suggestedValue().isNull())
+    element->setValue(element->suggestedValue(), true);
 }
 
 bool PasswordAutofillAgent::TextFieldDidEndEditing(
@@ -477,6 +507,10 @@ void PasswordAutofillAgent::WillSubmitForm(blink::WebFrame* frame,
   }
 }
 
+void PasswordAutofillAgent::ProcessingUserGestureEvent() {
+  gatekeeper_.OnUserGesture();
+}
+
 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
-    // filling in passwords.
-    user_gesture_occurred_ = false;
-    gesture_handler_->clearElements();
+    // This is a new navigation, so require a new user gesture before filling in
+    // passwords.
+    gatekeeper_.Reset();

[jww, 2014/02/18 20:19:56]

Perhaps it's too late to ask this question anyway, but in the old WebUserGesture
system, couldn't we have just allocated a new handler at this point and fixed
the problem?

That is, without this new refactor, if on every navigation we created a new
gesture_handler_, and set the old one to be ref counted, couldn't we have solved
this issue without the new implementation?

There's probably good reason to do the refactor anyway, so feel free to blow
this question off if you think it's not worth thinking about.

[vabr (Chromium), 2014/02/19 07:56:39]

Joel, I don't think what you suggest would help. Consider the following
scenario:
1. User loads a.com in tab 1.
2. User middle-clicks a link to b.com, which opens in tab 2 in background.
3. User re-loads tab 1.

Now, remember that tab 1 and tab 2 shared the handler.
After reloading in step 3, the handler set by tab 2 would be replaced by the new
handler from tab 1. After that, tab 2 would never get notified about user
interaction any more. And because there was no user interaction with tab 2 yet,
no password fields would become accessible to JavaScript in tab 2.

   }
 }
 
@@ -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);
-  }
+  // Wait to fill in the password until a user gesture occurs. 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.
+  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