Allow only a user gesture to trigger autofill popup

content/public/test/render_view_test.cc

Index: content/public/test/render_view_test.cc
diff --git a/content/public/test/render_view_test.cc b/content/public/test/render_view_test.cc
index ca67587df8b334707b223f0019aed624b7ebd983..2af6146c1b19af42ea71b18ad0148e1abec2099a 100644
--- a/content/public/test/render_view_test.cc
+++ b/content/public/test/render_view_test.cc
@@ -4,6 +4,8 @@
 
 #include "content/public/test/render_view_test.h"
 
+#include <cctype>
+
 #include "base/run_loop.h"
 #include "content/common/dom_storage/dom_storage_types.h"
 #include "content/common/frame_messages.h"
@@ -27,13 +29,16 @@
 #include "content/test/test_content_client.h"
 #include "third_party/WebKit/public/platform/WebScreenInfo.h"
 #include "third_party/WebKit/public/platform/WebURLRequest.h"
+#include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebHistoryItem.h"
+#include "third_party/WebKit/public/web/WebInputElement.h"
 #include "third_party/WebKit/public/web/WebInputEvent.h"
 #include "third_party/WebKit/public/web/WebKit.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
 #include "third_party/WebKit/public/web/WebScriptSource.h"
 #include "third_party/WebKit/public/web/WebView.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/events/keycodes/keyboard_codes.h"
 #include "v8/include/v8.h"
 
 #if defined(OS_MACOSX)
@@ -49,6 +54,7 @@ using blink::WebString;
 using blink::WebURLRequest;
 
 namespace {
+
 const int32 kOpenerId = -2;
 const int32 kRouteId = 5;
 const int32 kMainFrameRouteId = 6;
@@ -56,6 +62,32 @@ const int32 kNewWindowRouteId = 7;
 const int32 kNewFrameRouteId = 10;
 const int32 kSurfaceId = 42;
 
+// Converts |ascii_character| into |key_code| and returns true on success.
+// Handles only the characters needed by tests.
+bool GetWindowsKeyCode(char ascii_character, int* key_code) {
+  if (isalnum(ascii_character)) {
+    *key_code = base::ToUpperASCII(ascii_character);
+    return true;
+  }
+
+  switch (ascii_character) {
+    case '@':
+      *key_code = '2';
+      return true;
+    case '_':
+      *key_code = ui::VKEY_OEM_MINUS;
+      return true;
+    case '.':
+      *key_code = ui::VKEY_OEM_PERIOD;
+      return true;
+    case ui::VKEY_BACK:
+      *key_code = ui::VKEY_BACK;
+      return true;
+    default:
+      return false;
+  }
+}
+
 }  // namespace
 
 namespace content {
@@ -394,6 +426,62 @@ void RenderViewTest::Resize(gfx::Size new_size,
   OnMessageReceived(*resize_message);
 }
 
+void RenderViewTest::SimulateUserTypingASCIICharacter(char ascii_character) {
+  blink::WebKeyboardEvent event;
+  event.text[0] = ascii_character;
+  ASSERT_TRUE(GetWindowsKeyCode(ascii_character, &event.windowsKeyCode));
+  if (isupper(ascii_character) || ascii_character == '@' ||
+      ascii_character == '_') {
+    event.modifiers = blink::WebKeyboardEvent::ShiftKey;
+  }
+
+  event.type = blink::WebKeyboardEvent::RawKeyDown;
+  SendWebKeyboardEvent(event);
+
+  event.type = blink::WebKeyboardEvent::Char;
+  SendWebKeyboardEvent(event);
+
+  event.type = blink::WebKeyboardEvent::KeyUp;
+  SendWebKeyboardEvent(event);
+}
+
+void RenderViewTest::ProcessInputForAutofill() {

[Jay Civelli, 2015/04/23 21:57:43]

I am confused on whether this actually does something that triggers processing
of autofill or if it makes us wait for a state when we know auotfill has been
applied.

[please use gerrit instead, 2015/04/27 21:20:34]

Running the message loop will trigger autofill.

When a text changes in an input field,  AutofillAgent::textFieldDidChange will
post the function that will trigger the autofill field. The actual autofill
happens in AutofillAgent::TextFieldDidChangeImpl.

[Jay Civelli, 2015/04/27 22:17:19]

OK, why do we need to re-layout though? Wouldn't the
AutofillAgent::textFieldDidChange be called without it?
I also wonder if we could merge this method with
SimulateUserTypingASCIICharacter.
It could take a bool parameter to specify whether to flush the message loop.
WDYT?

[please use gerrit instead, 2015/04/27 23:49:54]

It seems we don't need re-layout. The original comment must
have been outdated. After removing re-layout from tests, all tests are still
passing.

I've added |flush_message_loop| parameter to SimulateUserTypingASCIICharacter
and removed ProcessInputForAutofill entirely.

+  // Processing is delayed because of a Blink bug:
+  // https://bugs.webkit.org/show_bug.cgi?id=16976 See
+  // PasswordAutofillAgent::TextDidChangeInTextField() for details.
+
+  // Autocomplete will trigger a style recalculation when we put up the next
+  // frame, but we don't want to wait that long. Instead, trigger a style
+  // recalcuation manually after TextFieldDidChangeImpl runs.
+  base::MessageLoop::current()->PostTask(
+      FROM_HERE,
+      base::Bind(&RenderViewTest::LayoutMainFrame, base::Unretained(this)));
+
+  base::MessageLoop::current()->RunUntilIdle();
+}
+
+void RenderViewTest::SimulateUserInputChangeForElement(
+    blink::WebInputElement* input,
+    const std::string& new_value) {
+  while (!input->focused())
+    input->document().frame()->view()->advanceFocus(false);

[Jay Civelli, 2015/04/23 21:57:43]

Wouldn't simulating a click on the input element achieve the same result in a
more straight-forward way?

[please use gerrit instead, 2015/04/27 21:20:34]

I tried that and for some reason focus does not change in Blink. I'm not sure
what's at play here, but I'm leaning toward keeping that code as is, because it
remains after copying it from
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/au....
What do you think?

[Jay Civelli, 2015/04/27 22:17:19]

Sounds good.

+
+  size_t previous_length = input->value().length();
+  for (size_t i = 0; i < previous_length; ++i)
+    SimulateUserTypingASCIICharacter(ui::VKEY_BACK);
+
+  EXPECT_TRUE(input->value().utf8().empty());
+  ASSERT_TRUE(base::IsStringASCII(new_value));

[Jay Civelli, 2015/04/23 21:57:43]

Nit: this assert should probably be moved to the top of the method.

[please use gerrit instead, 2015/04/27 21:20:34]

Done.

+  for (size_t i = 0; i < new_value.size(); ++i)
+    SimulateUserTypingASCIICharacter(new_value[i]);
+
+  // Compare only beginning, because autocomplete may have filled out the
+  // form.
+  EXPECT_EQ(new_value, input->value().utf8().substr(0, new_value.length()));
+
+  ProcessInputForAutofill();
+}
+
 bool RenderViewTest::OnMessageReceived(const IPC::Message& msg) {
   RenderViewImpl* impl = static_cast<RenderViewImpl*>(view_);
   return impl->OnMessageReceived(msg);
@@ -464,4 +552,8 @@ void RenderViewTest::GoToOffset(int offset, const PageState& state) {
   FrameLoadWaiter(view_->GetMainRenderFrame()).Wait();
 }
 
+void RenderViewTest::LayoutMainFrame() {
+  GetMainFrame()->view()->layout();
+}
+
 }  // namespace content