Do not set autofill property for datalist options

chrome/browser/autofill/autofill_interactive_uitest.cc

Index: chrome/browser/autofill/autofill_interactive_uitest.cc
diff --git a/chrome/browser/autofill/autofill_interactive_uitest.cc b/chrome/browser/autofill/autofill_interactive_uitest.cc
index a641fa6bb9ca09f5de82504045c2b5e2d20c840d..3a3c37bd6d7945b87242be9ea7580371e478a196 100644
--- a/chrome/browser/autofill/autofill_interactive_uitest.cc
+++ b/chrome/browser/autofill/autofill_interactive_uitest.cc
@@ -294,6 +294,16 @@ class AutofillInteractiveTest : public InProcessBrowserTest {
     EXPECT_EQ(expected_value, value);
   }
 
+  void GetFieldBackgroundColor(const std::string& field_name,
+                               std::string& color) {

[Ilya Sherman, 2014/08/22 23:49:17]

nit: Please pass a pointer.  We pretty much never pass by non-const-reference in
the Chromium repository.

[ziran.sun, 2014/08/26 13:32:15]

Done.

+    ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+        GetWebContents(),
+        "window.domAutomationController.send("
+        "    document.defaultView.getComputedStyle(document.getElementById('" +
+            field_name + "')).backgroundColor);",

[Ilya Sherman, 2014/08/22 23:49:17]

nit: The current indentation looks really odd.  I'd either de-indent this line,
or indent the line above by four spaces.

[ziran.sun, 2014/08/26 13:32:15]

I tried both suggestions, "git cl format" doesn't seem liking them :(. It
returns the original format we have here.

 

[Ilya Sherman, 2014/08/27 00:37:47]

I understand that this is the format that "git cl format" returns, but "git cl
format" is not entirely canonical -- it's a good starting point, but it
sometimes gets the formatting wrong.  This is such a case.

+        &color));
+  }
+
   void SimulateURLFetch(bool success) {
     net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0);
     ASSERT_TRUE(fetcher);
@@ -423,6 +433,19 @@ class AutofillInteractiveTest : public InProcessBrowserTest {
     GetRenderViewHost()->RemoveKeyPressEventCallback(key_press_event_sink_);
   }
 
+  // There is no need to start message loop for Datalist.

[Ilya Sherman, 2014/08/22 23:49:17]

Hmm, why not?  Doesn't it use the same UI?

Even if it's not *necessary* to start the message loop, does it hurt anything to
do so?  If not, I'd prefer that we reuse the SendKeyToPopupAndWait() method, for
the sake of having less (and therefore simpler) code. 

[ziran.sun, 2014/08/26 13:32:15]

Existing test cases in this test file assume that once select an option in the
suggestion popup box, the selection gets previewed. So after
"DidSelectSuggestion()" is called, "DidPreviewFormData()" should be called.
"DidPreviewFormData()" defined in this file will stop the message loop.

We don't have preview support for datalist. If we starts message loop for
datalist, after "DidSelectSuggestion()" it will wait for autofill preview that
will never happen. This will cause the message loop hang and the test will end
up with timeout.

[Ilya Sherman, 2014/08/27 00:37:47]

I see.  That makes sense; thanks for the explanation.

+  void SendKeyToDataListPopup(ui::KeyboardCode key) {
+    // Route popup-targeted key presses via the render view host.
+    content::NativeWebKeyboardEvent event;
+    event.windowsKeyCode = key;
+    event.type = blink::WebKeyboardEvent::RawKeyDown;
+    // Install the key press event sink to ensure that any events that are not
+    // handled by the installed callbacks do not end up crashing the test.
+    GetRenderViewHost()->AddKeyPressEventCallback(key_press_event_sink_);
+    GetRenderViewHost()->ForwardKeyboardEvent(event);
+    GetRenderViewHost()->RemoveKeyPressEventCallback(key_press_event_sink_);
+  }
+
   void TryBasicFormFill() {
     FocusFirstNameField();
 
@@ -634,6 +657,35 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, OnDeleteValueAfterAutofill) {
   ExpectFieldValue("firstname", "Milton");
 }
 
+// Test that an input field should not be autofilled when choose an option from
+// its datalist option list.

[Ilya Sherman, 2014/08/22 23:49:17]

nit: Perhaps something like "Test that an input field is not rendered with the
yellow autofilled background color when choosing an option from the datalist
suggestion list."? (As always, feel free to refine further :)

[ziran.sun, 2014/08/26 13:32:15]

Done.

+IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, OnSelectOptionFromDatalist) {
+  // Load the test page.
+  ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
+      browser(),
+      GURL(std::string(kDataURIPrefix) +
+           "<form action=\"http://www.example.com/\" method=\"POST\">"
+           " <input list=\"dl\" type=\"search\" id=\"firstname\""

[Ilya Sherman, 2014/08/22 23:49:17]

nit: Please indent this line, and the ones below, by one additional space
(within the quotation marks).  That is, the start of the line should be

           "  <input ...

[ziran.sun, 2014/08/26 13:32:15]

Done.

+           "        onfocus=\"domAutomationController.send(true)\"><br>"
+           " <datalist id=\"dl\">"

[Ilya Sherman, 2014/08/22 23:49:17]

nit: Why does the datalist need an id?  Would the test work fine if you omitted
this?

[ziran.sun, 2014/08/26 13:32:15]

I think the minimum argument for datalist tag is to have an id. And we refer
this id in input "list". 

The test doesn't work without id specified here.

+           " <option value=\"Adam\"></option>"
+           " <option value=\"Bob\"></option>"
+           " <option value=\"Carl\"></option>"
+           " </datalist>"
+           "</form>")));
+  std::string orginalcolor;
+  GetFieldBackgroundColor("firstname", orginalcolor);
+
+  FocusFirstNameField();
+  SendKeyToPageAndWait(ui::VKEY_DOWN);
+  SendKeyToDataListPopup(ui::VKEY_DOWN);
+  SendKeyToDataListPopup(ui::VKEY_RETURN);
+  ExpectFieldValue("firstname", "Adam");
+  std::string color;
+  GetFieldBackgroundColor("firstname", color);
+  EXPECT_EQ(color, orginalcolor);
+}
+
 // Test that a JavaScript oninput event is fired after auto-filling a form.
 IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, OnInputAfterAutofill) {
   CreateTestProfile();