diff options
author | rouslan <rouslan@chromium.org> | 2015-01-21 17:54:14 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-01-22 01:55:40 +0000 |
commit | f7ebd883699112506893fc0500a65c5a10e563ec (patch) | |
tree | 9e19faae4ecf97f42adc6d2620b0903ab2837f74 /chrome/renderer/autofill | |
parent | 196692c000558976906e11c7d184301c55aada20 (diff) | |
download | chromium_src-f7ebd883699112506893fc0500a65c5a10e563ec.zip chromium_src-f7ebd883699112506893fc0500a65c5a10e563ec.tar.gz chromium_src-f7ebd883699112506893fc0500a65c5a10e563ec.tar.bz2 |
[Android] Show autofill popup after animation.
The renderer shows autofill popups immediately upon click or focus
change. If the browser shows an IME (virtual keyboard), then it can zoom
and scroll the webpage, which will hide the popup. This behavior causes
a "flash" of autofill popup on platforms with IME (ChromeOS, Windows 8,
and Android). This patch mostly fixes Android and paves the way for
fixing Windows 8 and ChromeOS.
The fix involves roughly 5 parts.
1) If the platform does not support a virtual keyboard (Mac, Linux,
Windows 7), the renderer shows the autofill popup immediately.
2) If a virtual keyboard is supported, but disabled (ChromeOS without a
virtual keyboard and Android with a hardware keyboard), then the
browser notifies the renderer that the autofill popup can be shown.
3) If the virtual keyboard is already showing, then the browser notifies
the renderer that the autofill popup can be shown.
A corner case is the Android keyboard, which can change its own
size when switching between input fields, thus resizing the webpage
and hiding the autofill popup. The mitigation is to hide the
autofill popup only if the input field moves due to the
resize. This mitigation is not in this patch.
4) If the input field is already in a good position (visible on screen
and legible size) after the keyboard is shown, then there will be no
zoom and scroll animations. Then the renderer can show the autofill
popup.
5) If the input field is zooming and scrolling into a good position
(visible on screen and legible size) after the keyboard is shown,
then the renderer waits until the compositor notifies it that the
this animation has finished. Then the renderer can show the autofill
popup.
BUG=430318, 440161
Review URL: https://codereview.chromium.org/715733002
Cr-Commit-Position: refs/heads/master@{#312521}
Diffstat (limited to 'chrome/renderer/autofill')
-rw-r--r-- | chrome/renderer/autofill/page_click_tracker_browsertest.cc | 101 | ||||
-rw-r--r-- | chrome/renderer/autofill/password_generation_agent_browsertest.cc | 87 |
2 files changed, 135 insertions, 53 deletions
diff --git a/chrome/renderer/autofill/page_click_tracker_browsertest.cc b/chrome/renderer/autofill/page_click_tracker_browsertest.cc index 6495254..84231f6 100644 --- a/chrome/renderer/autofill/page_click_tracker_browsertest.cc +++ b/chrome/renderer/autofill/page_click_tracker_browsertest.cc @@ -8,11 +8,13 @@ #include "components/autofill/content/renderer/page_click_tracker.h" #include "content/public/renderer/render_view.h" #include "testing/gtest/include/gtest/gtest.h" +#include "third_party/WebKit/public/platform/WebFloatPoint.h" +#include "third_party/WebKit/public/platform/WebSize.h" #include "third_party/WebKit/public/web/WebDocument.h" #include "third_party/WebKit/public/web/WebInputElement.h" +#include "third_party/WebKit/public/web/WebSettings.h" #include "third_party/WebKit/public/web/WebTextAreaElement.h" #include "third_party/WebKit/public/web/WebView.h" -#include "third_party/WebKit/public/platform/WebSize.h" #include "ui/events/keycodes/keyboard_codes.h" namespace autofill { @@ -50,12 +52,17 @@ class PageClickTrackerTest : public ChromeRenderViewTest { // Rather than make it do so for the test, we create a new object. page_click_tracker_.reset(new PageClickTracker(view_, &test_listener_)); + // Must be set before loading HTML. + view_->GetWebView()->setDefaultPageScaleLimits(1, 4); + view_->GetWebView()->settings()->setPinchVirtualViewportEnabled(true); + LoadHTML("<form>" " <input type='text' id='text_1'></input><br>" " <input type='text' id='text_2'></input><br>" " <textarea id='textarea_1'></textarea><br>" " <textarea id='textarea_2'></textarea><br>" " <input type='button' id='button'></input><br>" + " <input type='button' id='button_2' disabled></input><br>" "</form>"); GetWebWidget()->resize(blink::WebSize(500, 500)); GetWebWidget()->setFocus(true); @@ -64,6 +71,10 @@ class PageClickTrackerTest : public ChromeRenderViewTest { textarea_ = document.getElementById("textarea_1"); ASSERT_FALSE(text_.isNull()); ASSERT_FALSE(textarea_.isNull()); + + // Enable show-ime event when element is focused by indicating that a user + // gesture has been processed since load. + EXPECT_TRUE(SimulateElementClick("button")); } void TearDown() override { @@ -74,29 +85,6 @@ class PageClickTrackerTest : public ChromeRenderViewTest { ChromeRenderViewTest::TearDown(); } - // Simulates a click on the given element and then waits for the stack - // to unwind. - void SendElementClick(const std::string& element_id) { - EXPECT_TRUE(SimulateElementClick(element_id)); - ProcessPendingMessages(); - } - - // Send all the messages required for a complete key press. - void SendKeyPress(int key_code) { - blink::WebKeyboardEvent keyboard_event; - keyboard_event.windowsKeyCode = key_code; - keyboard_event.setKeyIdentifierFromWindowsKeyCode(); - - keyboard_event.type = blink::WebInputEvent::RawKeyDown; - SendWebKeyboardEvent(keyboard_event); - - keyboard_event.type = blink::WebInputEvent::Char; - SendWebKeyboardEvent(keyboard_event); - - keyboard_event.type = blink::WebInputEvent::KeyUp; - SendWebKeyboardEvent(keyboard_event); - } - scoped_ptr<PageClickTracker> page_click_tracker_; TestPageClickListener test_listener_; blink::WebElement text_; @@ -108,43 +96,96 @@ class PageClickTrackerTest : public ChromeRenderViewTest { TEST_F(PageClickTrackerTest, PageClickTrackerInputClicked) { EXPECT_NE(text_, text_.document().focusedElement()); // Click the text field once. - SendElementClick("text_1"); + EXPECT_TRUE(SimulateElementClick("text_1")); EXPECT_TRUE(test_listener_.form_control_element_clicked_called_); EXPECT_FALSE(test_listener_.was_focused_); EXPECT_TRUE(text_ == test_listener_.form_control_element_clicked_); test_listener_.ClearResults(); // Click the text field again to test that was_focused_ is set correctly. - SendElementClick("text_1"); + EXPECT_TRUE(SimulateElementClick("text_1")); EXPECT_TRUE(test_listener_.form_control_element_clicked_called_); EXPECT_TRUE(test_listener_.was_focused_); EXPECT_TRUE(text_ == test_listener_.form_control_element_clicked_); test_listener_.ClearResults(); // Click the button, no notification should happen (this is not a text-input). - SendElementClick("button"); + EXPECT_TRUE(SimulateElementClick("button")); EXPECT_FALSE(test_listener_.form_control_element_clicked_called_); } +TEST_F(PageClickTrackerTest, PageClickTrackerInputFocusedAndClicked) { + // Focus the text field without a click. + ExecuteJavaScript("document.getElementById('text_1').focus();"); + EXPECT_FALSE(test_listener_.form_control_element_clicked_called_); + test_listener_.ClearResults(); + + // Click the focused text field to test that was_focused_ is set correctly. + EXPECT_TRUE(SimulateElementClick("text_1")); + EXPECT_TRUE(test_listener_.form_control_element_clicked_called_); + EXPECT_TRUE(test_listener_.was_focused_); + EXPECT_TRUE(text_ == test_listener_.form_control_element_clicked_); +} + // Tests that PageClickTracker does notify correctly when a textarea // node is clicked. TEST_F(PageClickTrackerTest, PageClickTrackerTextAreaClicked) { // Click the textarea field once. - SendElementClick("textarea_1"); + EXPECT_TRUE(SimulateElementClick("textarea_1")); EXPECT_TRUE(test_listener_.form_control_element_clicked_called_); EXPECT_FALSE(test_listener_.was_focused_); EXPECT_TRUE(textarea_ == test_listener_.form_control_element_clicked_); test_listener_.ClearResults(); // Click the textarea field again to test that was_focused_ is set correctly. - SendElementClick("textarea_1"); + EXPECT_TRUE(SimulateElementClick("textarea_1")); EXPECT_TRUE(test_listener_.form_control_element_clicked_called_); EXPECT_TRUE(test_listener_.was_focused_); EXPECT_TRUE(textarea_ == test_listener_.form_control_element_clicked_); test_listener_.ClearResults(); // Click the button, no notification should happen (this is not a text-input). - SendElementClick("button"); + EXPECT_TRUE(SimulateElementClick("button")); + EXPECT_FALSE(test_listener_.form_control_element_clicked_called_); +} + +TEST_F(PageClickTrackerTest, PageClickTrackerTextAreaFocusedAndClicked) { + // Focus the textarea without a click. + ExecuteJavaScript("document.getElementById('textarea_1').focus();"); + EXPECT_FALSE(test_listener_.form_control_element_clicked_called_); + test_listener_.ClearResults(); + + // Click the focused text field to test that was_focused_ is set correctly. + EXPECT_TRUE(SimulateElementClick("textarea_1")); + EXPECT_TRUE(test_listener_.form_control_element_clicked_called_); + EXPECT_TRUE(test_listener_.was_focused_); + EXPECT_TRUE(textarea_ == test_listener_.form_control_element_clicked_); + test_listener_.ClearResults(); +} + +TEST_F(PageClickTrackerTest, PageClickTrackerScaledTextareaClicked) { + EXPECT_NE(text_, text_.document().focusedElement()); + view_->GetWebView()->setPageScaleFactor(3); + view_->GetWebView()->setPinchViewportOffset(blink::WebFloatPoint(50, 50)); + + // Click textarea_1. + SimulatePointClick(gfx::Point(30, 30)); + EXPECT_TRUE(test_listener_.form_control_element_clicked_called_); + EXPECT_FALSE(test_listener_.was_focused_); + EXPECT_TRUE(textarea_ == test_listener_.form_control_element_clicked_); +} + +TEST_F(PageClickTrackerTest, PageClickTrackerDisabledInputClickedNoEvent) { + EXPECT_NE(text_, text_.document().focusedElement()); + // Click the text field once. + EXPECT_TRUE(SimulateElementClick("text_1")); + EXPECT_TRUE(test_listener_.form_control_element_clicked_called_); + EXPECT_FALSE(test_listener_.was_focused_); + EXPECT_TRUE(text_ == test_listener_.form_control_element_clicked_); + test_listener_.ClearResults(); + + // Click the disabled element. + EXPECT_TRUE(SimulateElementClick("button_2")); EXPECT_FALSE(test_listener_.form_control_element_clicked_called_); } diff --git a/chrome/renderer/autofill/password_generation_agent_browsertest.cc b/chrome/renderer/autofill/password_generation_agent_browsertest.cc index b93cff6..a093f32 100644 --- a/chrome/renderer/autofill/password_generation_agent_browsertest.cc +++ b/chrome/renderer/autofill/password_generation_agent_browsertest.cc @@ -85,6 +85,14 @@ class PasswordGenerationAgentTest : public ChromeRenderViewTest { password_generation_->clear_messages(); } + void LoadHTMLWithUserGesture(const char* html) { + LoadHTML(html); + + // Enable show-ime event when element is focused by indicating that a user + // gesture has been processed since load. + EXPECT_TRUE(SimulateElementClick("dummy")); + } + private: DISALLOW_COPY_AND_ASSIGN(PasswordGenerationAgentTest); }; @@ -93,6 +101,7 @@ const char kSigninFormHTML[] = "<FORM name = 'blah' action = 'http://www.random.com/'> " " <INPUT type = 'text' id = 'username'/> " " <INPUT type = 'password' id = 'password'/> " + " <INPUT type = 'button' id = 'dummy'/> " " <INPUT type = 'submit' value = 'LOGIN' />" "</FORM>"; @@ -103,6 +112,19 @@ const char kAccountCreationFormHTML[] = " autocomplete = 'off' size = 5/>" " <INPUT type = 'password' id = 'second_password' size = 5/> " " <INPUT type = 'text' id = 'address'/> " + " <INPUT type = 'button' id = 'dummy'/> " + " <INPUT type = 'submit' value = 'LOGIN' />" + "</FORM>"; + +const char kDisabledElementAccountCreationFormHTML[] = + "<FORM name = 'blah' action = 'http://www.random.com/'> " + " <INPUT type = 'text' id = 'username'/> " + " <INPUT type = 'password' id = 'first_password' " + " autocomplete = 'off' size = 5/>" + " <INPUT type = 'password' id = 'second_password' size = 5/> " + " <INPUT type = 'text' id = 'address'/> " + " <INPUT type = 'text' id = 'disabled' disabled/> " + " <INPUT type = 'button' id = 'dummy'/> " " <INPUT type = 'submit' value = 'LOGIN' />" "</FORM>"; @@ -111,6 +133,7 @@ const char kHiddenPasswordAccountCreationFormHTML[] = " <INPUT type = 'text' id = 'username'/> " " <INPUT type = 'password' id = 'first_password'/> " " <INPUT type = 'password' id = 'second_password' style='display:none'/> " + " <INPUT type = 'button' id = 'dummy'/> " " <INPUT type = 'submit' value = 'LOGIN' />" "</FORM>"; @@ -119,6 +142,7 @@ const char kInvalidActionAccountCreationFormHTML[] = " <INPUT type = 'text' id = 'username'/> " " <INPUT type = 'password' id = 'first_password'/> " " <INPUT type = 'password' id = 'second_password'/> " + " <INPUT type = 'button' id = 'dummy'/> " " <INPUT type = 'submit' value = 'LOGIN' />" "</FORM>"; @@ -127,6 +151,7 @@ const char kMultipleAccountCreationFormHTML[] = " <INPUT type = 'text' id = 'random'/> " " <INPUT type = 'text' id = 'username'/> " " <INPUT type = 'password' id = 'password'/> " + " <INPUT type = 'button' id = 'dummy'/> " " <INPUT type = 'submit' value = 'LOGIN' />" "</FORM>" "<FORM name = 'signup' action = 'http://www.random.com/signup'> " @@ -152,29 +177,29 @@ const char ChangeDetectionScript[] = TEST_F(PasswordGenerationAgentTest, DetectionTest) { // Don't shown the icon for non account creation forms. - LoadHTML(kSigninFormHTML); + LoadHTMLWithUserGesture(kSigninFormHTML); ExpectPasswordGenerationAvailable("password", false); // We don't show the decoration yet because the feature isn't enabled. - LoadHTML(kAccountCreationFormHTML); + LoadHTMLWithUserGesture(kAccountCreationFormHTML); ExpectPasswordGenerationAvailable("first_password", false); // Pretend like We have received message indicating site is not blacklisted, // and we have received message indicating the form is classified as // ACCOUNT_CREATION_FORM form Autofill server. We should show the icon. - LoadHTML(kAccountCreationFormHTML); + LoadHTMLWithUserGesture(kAccountCreationFormHTML); SetNotBlacklistedMessage(kAccountCreationFormHTML); SetAccountCreationFormsDetectedMessage(0); ExpectPasswordGenerationAvailable("first_password", true); // Hidden fields are not treated differently. - LoadHTML(kHiddenPasswordAccountCreationFormHTML); + LoadHTMLWithUserGesture(kHiddenPasswordAccountCreationFormHTML); SetNotBlacklistedMessage(kHiddenPasswordAccountCreationFormHTML); SetAccountCreationFormsDetectedMessage(0); ExpectPasswordGenerationAvailable("first_password", true); // This doesn't trigger because the form action is invalid. - LoadHTML(kInvalidActionAccountCreationFormHTML); + LoadHTMLWithUserGesture(kInvalidActionAccountCreationFormHTML); SetNotBlacklistedMessage(kInvalidActionAccountCreationFormHTML); SetAccountCreationFormsDetectedMessage(0); ExpectPasswordGenerationAvailable("first_password", false); @@ -184,7 +209,7 @@ TEST_F(PasswordGenerationAgentTest, FillTest) { // Make sure that we are enabled before loading HTML. std::string html = std::string(kAccountCreationFormHTML) + ChangeDetectionScript; - LoadHTML(html.c_str()); + LoadHTMLWithUserGesture(html.c_str()); SetNotBlacklistedMessage(html.c_str()); SetAccountCreationFormsDetectedMessage(0); @@ -234,7 +259,7 @@ TEST_F(PasswordGenerationAgentTest, FillTest) { } TEST_F(PasswordGenerationAgentTest, EditingTest) { - LoadHTML(kAccountCreationFormHTML); + LoadHTMLWithUserGesture(kAccountCreationFormHTML); SetNotBlacklistedMessage(kAccountCreationFormHTML); SetAccountCreationFormsDetectedMessage(0); @@ -284,27 +309,27 @@ TEST_F(PasswordGenerationAgentTest, EditingTest) { TEST_F(PasswordGenerationAgentTest, BlacklistedTest) { // Did not receive not blacklisted message. Don't show password generation // icon. - LoadHTML(kAccountCreationFormHTML); + LoadHTMLWithUserGesture(kAccountCreationFormHTML); SetAccountCreationFormsDetectedMessage(0); ExpectPasswordGenerationAvailable("first_password", false); // Receive one not blacklisted message for non account creation form. Don't // show password generation icon. - LoadHTML(kAccountCreationFormHTML); + LoadHTMLWithUserGesture(kAccountCreationFormHTML); SetNotBlacklistedMessage(kSigninFormHTML); SetAccountCreationFormsDetectedMessage(0); ExpectPasswordGenerationAvailable("first_password", false); - // Receive one not blackliste message for account creation form. Show password - // generation icon. - LoadHTML(kAccountCreationFormHTML); + // Receive one not blacklisted message for account creation form. Show + // password generation icon. + LoadHTMLWithUserGesture(kAccountCreationFormHTML); SetNotBlacklistedMessage(kAccountCreationFormHTML); SetAccountCreationFormsDetectedMessage(0); ExpectPasswordGenerationAvailable("first_password", true); // Receive two not blacklisted messages, one is for account creation form and // the other is not. Show password generation icon. - LoadHTML(kAccountCreationFormHTML); + LoadHTMLWithUserGesture(kAccountCreationFormHTML); SetNotBlacklistedMessage(kAccountCreationFormHTML); SetNotBlacklistedMessage(kSigninFormHTML); SetAccountCreationFormsDetectedMessage(0); @@ -312,15 +337,15 @@ TEST_F(PasswordGenerationAgentTest, BlacklistedTest) { } TEST_F(PasswordGenerationAgentTest, AccountCreationFormsDetectedTest) { - // Did not receive account creation forms detected messege. Don't show + // Did not receive account creation forms detected message. Don't show // password generation icon. - LoadHTML(kAccountCreationFormHTML); + LoadHTMLWithUserGesture(kAccountCreationFormHTML); SetNotBlacklistedMessage(kAccountCreationFormHTML); ExpectPasswordGenerationAvailable("first_password", false); // Receive the account creation forms detected message. Show password // generation icon. - LoadHTML(kAccountCreationFormHTML); + LoadHTMLWithUserGesture(kAccountCreationFormHTML); SetNotBlacklistedMessage(kAccountCreationFormHTML); SetAccountCreationFormsDetectedMessage(0); ExpectPasswordGenerationAvailable("first_password", true); @@ -329,7 +354,7 @@ TEST_F(PasswordGenerationAgentTest, AccountCreationFormsDetectedTest) { TEST_F(PasswordGenerationAgentTest, MaximumOfferSize) { base::HistogramTester histogram_tester; - LoadHTML(kAccountCreationFormHTML); + LoadHTMLWithUserGesture(kAccountCreationFormHTML); SetNotBlacklistedMessage(kAccountCreationFormHTML); SetAccountCreationFormsDetectedMessage(0); ExpectPasswordGenerationAvailable("first_password", true); @@ -397,14 +422,14 @@ TEST_F(PasswordGenerationAgentTest, MaximumOfferSize) { // Focusing the password field will bring up the generation UI again. ExecuteJavaScript("document.getElementById('first_password').focus();"); - EXPECT_EQ(1u, password_generation_->messages().size()); + ASSERT_EQ(1u, password_generation_->messages().size()); EXPECT_EQ(AutofillHostMsg_ShowPasswordGenerationPopup::ID, password_generation_->messages()[0]->type()); password_generation_->clear_messages(); // Loading a different page triggers UMA stat upload. Verify that only one // display event is sent even though - LoadHTML(kSigninFormHTML); + LoadHTMLWithUserGesture(kSigninFormHTML); histogram_tester.ExpectBucketCount( "PasswordGeneration.Event", @@ -413,7 +438,7 @@ TEST_F(PasswordGenerationAgentTest, MaximumOfferSize) { } TEST_F(PasswordGenerationAgentTest, DynamicFormTest) { - LoadHTML(kSigninFormHTML); + LoadHTMLWithUserGesture(kSigninFormHTML); SetNotBlacklistedMessage(kSigninFormHTML); ExecuteJavaScript( @@ -438,7 +463,7 @@ TEST_F(PasswordGenerationAgentTest, DynamicFormTest) { // This needs to come after the DOM has been modified. SetAccountCreationFormsDetectedMessage(1); - // TODO(gcasto): I'm slighty worried about flakes in this test where + // TODO(gcasto): I'm slightly worried about flakes in this test where // didAssociateFormControls() isn't called. If this turns out to be a problem // adding a call to OnDynamicFormsSeen(GetMainFrame()) will fix it, though // it will weaken the test. @@ -448,7 +473,7 @@ TEST_F(PasswordGenerationAgentTest, DynamicFormTest) { TEST_F(PasswordGenerationAgentTest, MultiplePasswordFormsTest) { // If two forms on the page looks like possible account creation forms, make // sure to trigger on the one that is specified from Autofill. - LoadHTML(kMultipleAccountCreationFormHTML); + LoadHTMLWithUserGesture(kMultipleAccountCreationFormHTML); SetNotBlacklistedMessage(kMultipleAccountCreationFormHTML); // Should trigger on the second form. @@ -459,7 +484,7 @@ TEST_F(PasswordGenerationAgentTest, MultiplePasswordFormsTest) { } TEST_F(PasswordGenerationAgentTest, MessagesAfterAccountSignupFormFound) { - LoadHTML(kAccountCreationFormHTML); + LoadHTMLWithUserGesture(kAccountCreationFormHTML); SetNotBlacklistedMessage(kAccountCreationFormHTML); SetAccountCreationFormsDetectedMessage(0); @@ -475,4 +500,20 @@ TEST_F(PasswordGenerationAgentTest, MessagesAfterAccountSignupFormFound) { ExpectPasswordGenerationAvailable("first_password", true); } +// Losing focus should not trigger a password generation popup. +TEST_F(PasswordGenerationAgentTest, BlurTest) { + LoadHTMLWithUserGesture(kDisabledElementAccountCreationFormHTML); + SetNotBlacklistedMessage(kDisabledElementAccountCreationFormHTML); + SetAccountCreationFormsDetectedMessage(0); + + // Focus on the first password field: password generation popup should show + // up. + ExpectPasswordGenerationAvailable("first_password", true); + + // Remove focus from everywhere by clicking an unfocusable element: password + // generation popup should not show up. + EXPECT_TRUE(SimulateElementClick("disabled")); + EXPECT_EQ(0u, password_generation_->messages().size()); +} + } // namespace autofill |