diff options
40 files changed, 514 insertions, 120 deletions
diff --git a/android_webview/browser/hardware_renderer.h b/android_webview/browser/hardware_renderer.h index 7f0459d..671b3ca 100644 --- a/android_webview/browser/hardware_renderer.h +++ b/android_webview/browser/hardware_renderer.h @@ -59,6 +59,7 @@ class HardwareRenderer : public cc::LayerTreeHostClient, virtual void DidCommit() override {} virtual void DidCommitAndDrawFrame() override {} virtual void DidCompleteSwapBuffers() override {} + virtual void DidCompletePageScaleAnimation() override {} // cc::LayerTreeHostSingleThreadClient overrides. virtual void DidPostSwapBuffers() override {} diff --git a/cc/test/fake_layer_tree_host_client.h b/cc/test/fake_layer_tree_host_client.h index f256ab1..5f7afa5 100644 --- a/cc/test/fake_layer_tree_host_client.h +++ b/cc/test/fake_layer_tree_host_client.h @@ -50,6 +50,7 @@ class FakeLayerTreeHostClient : public LayerTreeHostClient, void DidCommit() override {} void DidCommitAndDrawFrame() override {} void DidCompleteSwapBuffers() override {} + void DidCompletePageScaleAnimation() override {} // LayerTreeHostSingleThreadClient implementation. void DidPostSwapBuffers() override {} diff --git a/cc/test/fake_layer_tree_host_impl_client.h b/cc/test/fake_layer_tree_host_impl_client.h index 98aafa0..d7a785b 100644 --- a/cc/test/fake_layer_tree_host_impl_client.h +++ b/cc/test/fake_layer_tree_host_impl_client.h @@ -39,6 +39,7 @@ class FakeLayerTreeHostImplClient : public LayerTreeHostImplClient { base::TimeDelta delay) override {} void DidActivateSyncTree() override {} void DidPrepareTiles() override {} + void DidCompletePageScaleAnimationOnImplThread() override {} }; } // namespace cc diff --git a/cc/test/layer_tree_test.cc b/cc/test/layer_tree_test.cc index 9653d8d..a40a8c7 100644 --- a/cc/test/layer_tree_test.cc +++ b/cc/test/layer_tree_test.cc @@ -373,6 +373,7 @@ class LayerTreeHostClientForTesting : public LayerTreeHostClient, void DidPostSwapBuffers() override {} void DidAbortSwapBuffers() override {} void ScheduleComposite() override { test_hooks_->ScheduleComposite(); } + void DidCompletePageScaleAnimation() override {} private: explicit LayerTreeHostClientForTesting(TestHooks* test_hooks) diff --git a/cc/trees/layer_tree_host.cc b/cc/trees/layer_tree_host.cc index b6e6473..a24ac172 100644 --- a/cc/trees/layer_tree_host.cc +++ b/cc/trees/layer_tree_host.cc @@ -130,6 +130,7 @@ LayerTreeHost::LayerTreeHost( background_color_(SK_ColorWHITE), has_transparent_background_(false), partial_texture_update_requests_(0), + did_complete_scale_animation_(false), in_paint_layer_contents_(false), total_frames_used_for_lcd_text_metrics_(0), id_(s_layer_tree_host_sequence_number.GetNext() + 1), @@ -403,6 +404,10 @@ void LayerTreeHost::UpdateHudLayer() { void LayerTreeHost::CommitComplete() { source_frame_number_++; client_->DidCommit(); + if (did_complete_scale_animation_) { + client_->DidCompletePageScaleAnimation(); + did_complete_scale_animation_ = false; + } } void LayerTreeHost::SetOutputSurface(scoped_ptr<OutputSurface> surface) { @@ -788,6 +793,10 @@ bool LayerTreeHost::UpdateLayers(ResourceUpdateQueue* queue) { return result || next_commit_forces_redraw_; } +void LayerTreeHost::DidCompletePageScaleAnimation() { + did_complete_scale_animation_ = true; +} + static Layer* FindFirstScrollableLayer(Layer* layer) { if (!layer) return NULL; diff --git a/cc/trees/layer_tree_host.h b/cc/trees/layer_tree_host.h index 9069b9d..74ecfa3 100644 --- a/cc/trees/layer_tree_host.h +++ b/cc/trees/layer_tree_host.h @@ -135,6 +135,9 @@ class CC_EXPORT LayerTreeHost { void DeleteContentsTexturesOnImplThread(ResourceProvider* resource_provider); bool UpdateLayers(ResourceUpdateQueue* queue); + // Called when the compositor completed page scale animation. + void DidCompletePageScaleAnimation(); + LayerTreeHostClient* client() { return client_; } const base::WeakPtr<InputHandler>& GetInputHandler() { return input_handler_weak_ptr_; @@ -449,6 +452,10 @@ class CC_EXPORT LayerTreeHost { scoped_ptr<PendingPageScaleAnimation> pending_page_scale_animation_; + // If set, then page scale animation has completed, but the client hasn't been + // notified about it yet. + bool did_complete_scale_animation_; + bool in_paint_layer_contents_; static const int kTotalFramesToUseForLCDTextMetrics = 50; diff --git a/cc/trees/layer_tree_host_client.h b/cc/trees/layer_tree_host_client.h index a09693f..ac025f2 100644 --- a/cc/trees/layer_tree_host_client.h +++ b/cc/trees/layer_tree_host_client.h @@ -49,6 +49,9 @@ class LayerTreeHostClient { virtual void DidCommitAndDrawFrame() = 0; virtual void DidCompleteSwapBuffers() = 0; + // Called when page scale animation has completed. + virtual void DidCompletePageScaleAnimation() = 0; + // TODO(simonhong): Makes this to pure virtual function when client // implementation is ready. virtual void SendBeginFramesToChildren(const BeginFrameArgs& args) {} diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc index 526d456..73ad980 100644 --- a/cc/trees/layer_tree_host_impl.cc +++ b/cc/trees/layer_tree_host_impl.cc @@ -3106,6 +3106,7 @@ void LayerTreeHostImpl::AnimatePageScale(base::TimeTicks monotonic_time) { page_scale_animation_ = nullptr; client_->SetNeedsCommitOnImplThread(); client_->RenewTreePriority(); + client_->DidCompletePageScaleAnimationOnImplThread(); } else { SetNeedsAnimate(); } diff --git a/cc/trees/layer_tree_host_impl.h b/cc/trees/layer_tree_host_impl.h index 98d7e4b..9f146ca 100644 --- a/cc/trees/layer_tree_host_impl.h +++ b/cc/trees/layer_tree_host_impl.h @@ -108,6 +108,9 @@ class LayerTreeHostImplClient { virtual void DidActivateSyncTree() = 0; virtual void DidPrepareTiles() = 0; + // Called when page scale animation has completed on the impl thread. + virtual void DidCompletePageScaleAnimationOnImplThread() = 0; + protected: virtual ~LayerTreeHostImplClient() {} }; diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc index fb2fdb6..f03ab6b 100644 --- a/cc/trees/layer_tree_host_impl_unittest.cc +++ b/cc/trees/layer_tree_host_impl_unittest.cc @@ -92,6 +92,7 @@ class LayerTreeHostImplTest : public testing::Test, did_request_redraw_(false), did_request_animate_(false), did_request_prepare_tiles_(false), + did_complete_page_scale_animation_(false), reduce_memory_result_(true), current_limit_bytes_(0), current_priority_cutoff_value_(0) { @@ -156,6 +157,9 @@ class LayerTreeHostImplTest : public testing::Test, } void DidActivateSyncTree() override {} void DidPrepareTiles() override {} + void DidCompletePageScaleAnimationOnImplThread() override { + did_complete_page_scale_animation_ = true; + } void set_reduce_memory_result(bool reduce_memory_result) { reduce_memory_result_ = reduce_memory_result; @@ -399,6 +403,7 @@ class LayerTreeHostImplTest : public testing::Test, bool did_request_redraw_; bool did_request_animate_; bool did_request_prepare_tiles_; + bool did_complete_page_scale_animation_; bool reduce_memory_result_; base::Closure scrollbar_fade_start_; base::TimeDelta requested_scrollbar_animation_delay_; @@ -1346,6 +1351,38 @@ TEST_F(LayerTreeHostImplTest, PageScaleAnimationTransferedOnSyncTreeActivate) { ExpectContains(*scroll_info, scroll_layer->id(), gfx::Vector2d(-50, -50)); } +TEST_F(LayerTreeHostImplTest, PageScaleAnimationCompletedNotification) { + SetupScrollAndContentsLayers(gfx::Size(100, 100)); + host_impl_->SetViewportSize(gfx::Size(50, 50)); + DrawFrame(); + + LayerImpl* scroll_layer = host_impl_->InnerViewportScrollLayer(); + DCHECK(scroll_layer); + + base::TimeTicks start_time = + base::TimeTicks() + base::TimeDelta::FromSeconds(1); + base::TimeDelta duration = base::TimeDelta::FromMilliseconds(100); + base::TimeTicks halfway_through_animation = start_time + duration / 2; + base::TimeTicks end_time = start_time + duration; + + host_impl_->active_tree()->PushPageScaleFromMainThread(1.f, 0.5f, 4.f); + scroll_layer->SetScrollOffset(gfx::ScrollOffset(50, 50)); + + did_complete_page_scale_animation_ = false; + host_impl_->active_tree()->SetPendingPageScaleAnimation( + scoped_ptr<PendingPageScaleAnimation>(new PendingPageScaleAnimation( + gfx::Vector2d(), false, 2.f, duration))); + host_impl_->ActivateSyncTree(); + host_impl_->Animate(start_time); + EXPECT_FALSE(did_complete_page_scale_animation_); + + host_impl_->Animate(halfway_through_animation); + EXPECT_FALSE(did_complete_page_scale_animation_); + + host_impl_->Animate(end_time); + EXPECT_TRUE(did_complete_page_scale_animation_); +} + class LayerTreeHostImplOverridePhysicalTime : public LayerTreeHostImpl { public: LayerTreeHostImplOverridePhysicalTime( diff --git a/cc/trees/layer_tree_host_unittest_no_message_loop.cc b/cc/trees/layer_tree_host_unittest_no_message_loop.cc index 3e87b55..75eda92 100644 --- a/cc/trees/layer_tree_host_unittest_no_message_loop.cc +++ b/cc/trees/layer_tree_host_unittest_no_message_loop.cc @@ -78,6 +78,7 @@ class LayerTreeHostNoMessageLoopTest void DidCommit() override { did_commit_ = true; } void DidCommitAndDrawFrame() override { did_commit_and_draw_frame_ = true; } void DidCompleteSwapBuffers() override {} + void DidCompletePageScaleAnimation() override {} // LayerTreeHostSingleThreadClient overrides. void DidPostSwapBuffers() override {} diff --git a/cc/trees/single_thread_proxy.cc b/cc/trees/single_thread_proxy.cc index adc42c9..e29f0d1 100644 --- a/cc/trees/single_thread_proxy.cc +++ b/cc/trees/single_thread_proxy.cc @@ -447,6 +447,10 @@ void SingleThreadProxy::DidPrepareTiles() { scheduler_on_impl_thread_->DidPrepareTiles(); } +void SingleThreadProxy::DidCompletePageScaleAnimationOnImplThread() { + layer_tree_host_->DidCompletePageScaleAnimation(); +} + void SingleThreadProxy::UpdateRendererCapabilitiesOnImplThread() { DCHECK(IsImplThread()); renderer_capabilities_for_main_thread_ = diff --git a/cc/trees/single_thread_proxy.h b/cc/trees/single_thread_proxy.h index bdc1030..a6c348a 100644 --- a/cc/trees/single_thread_proxy.h +++ b/cc/trees/single_thread_proxy.h @@ -106,6 +106,7 @@ class CC_EXPORT SingleThreadProxy : public Proxy, base::TimeDelta delay) override {} void DidActivateSyncTree() override; void DidPrepareTiles() override; + void DidCompletePageScaleAnimationOnImplThread() override; void SetDebugState(const LayerTreeDebugState& debug_state) override {} void RequestNewOutputSurface(); diff --git a/cc/trees/thread_proxy.cc b/cc/trees/thread_proxy.cc index 243e603..4b360a4 100644 --- a/cc/trees/thread_proxy.cc +++ b/cc/trees/thread_proxy.cc @@ -267,6 +267,11 @@ void ThreadProxy::SendCommitRequestToImplThreadIfNeeded() { impl_thread_weak_ptr_)); } +void ThreadProxy::DidCompletePageScaleAnimation() { + DCHECK(IsMainThread()); + layer_tree_host()->DidCompletePageScaleAnimation(); +} + const RendererCapabilities& ThreadProxy::GetRendererCapabilities() const { DCHECK(IsMainThread()); DCHECK(!layer_tree_host()->output_surface_lost()); @@ -1353,4 +1358,11 @@ void ThreadProxy::DidPrepareTiles() { impl().scheduler->DidPrepareTiles(); } +void ThreadProxy::DidCompletePageScaleAnimationOnImplThread() { + DCHECK(IsImplThread()); + Proxy::MainThreadTaskRunner()->PostTask( + FROM_HERE, base::Bind(&ThreadProxy::DidCompletePageScaleAnimation, + main_thread_weak_ptr_)); +} + } // namespace cc diff --git a/cc/trees/thread_proxy.h b/cc/trees/thread_proxy.h index f8808f9..0d15b5a 100644 --- a/cc/trees/thread_proxy.h +++ b/cc/trees/thread_proxy.h @@ -209,6 +209,7 @@ class CC_EXPORT ThreadProxy : public Proxy, base::TimeDelta delay) override; void DidActivateSyncTree() override; void DidPrepareTiles() override; + void DidCompletePageScaleAnimationOnImplThread() override; // SchedulerClient implementation void WillBeginImplFrame(const BeginFrameArgs& args) override; @@ -251,6 +252,7 @@ class CC_EXPORT ThreadProxy : public Proxy, void DidInitializeOutputSurface(bool success, const RendererCapabilities& capabilities); void SendCommitRequestToImplThreadIfNeeded(); + void DidCompletePageScaleAnimation(); // Called on impl thread. struct SchedulerStateRequest; diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillPopupWithKeyboardTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillPopupWithKeyboardTest.java new file mode 100644 index 0000000..e023517 --- /dev/null +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillPopupWithKeyboardTest.java @@ -0,0 +1,146 @@ +// Copyright 2015 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. + +package org.chromium.chrome.browser.autofill; + +import android.test.suitebuilder.annotation.MediumTest; +import android.view.ViewGroup; + +import org.chromium.base.ThreadUtils; +import org.chromium.base.test.util.Feature; +import org.chromium.base.test.util.UrlUtils; +import org.chromium.chrome.R; +import org.chromium.chrome.browser.autofill.PersonalDataManager.AutofillProfile; +import org.chromium.chrome.shell.ChromeShellTestBase; +import org.chromium.content.browser.ContentViewCore; +import org.chromium.content.browser.test.util.Criteria; +import org.chromium.content.browser.test.util.CriteriaHelper; +import org.chromium.content.browser.test.util.DOMUtils; +import org.chromium.content_public.browser.WebContents; +import org.chromium.ui.UiUtils; +import org.chromium.ui.autofill.AutofillPopup; + +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicReference; + +/** + * Integration tests for interaction of the AutofillPopup and a keyboard. + */ +public class AutofillPopupWithKeyboardTest extends ChromeShellTestBase { + /** + * Test that showing autofill popup and keyboard will not hide the autofill popup. + */ + @MediumTest + @Feature({"autofill-keyboard"}) + public void testShowAutofillPopupAndKeyboardimultaneously() + throws InterruptedException, ExecutionException, TimeoutException { + launchChromeShellWithUrl(UrlUtils.encodeHtmlDataUri("<html><head>" + + "<meta name=\"viewport\"" + + "content=\"width=device-width, initial-scale=1.0, maximum-scale=1.0\" /></head>" + + "<body><form method=\"POST\">" + + "<input type=\"text\" id=\"fn\" autocomplete=\"given-name\" /><br>" + + "<input type=\"text\" id=\"ln\" autocomplete=\"family-name\" /><br>" + + "<textarea id=\"sa\" autocomplete=\"street-address\"></textarea><br>" + + "<input type=\"text\" id=\"a1\" autocomplete=\"address-line1\" /><br>" + + "<input type=\"text\" id=\"a2\" autocomplete=\"address-line2\" /><br>" + + "<input type=\"text\" id=\"ct\" autocomplete=\"locality\" /><br>" + + "<input type=\"text\" id=\"zc\" autocomplete=\"postal-code\" /><br>" + + "<input type=\"text\" id=\"em\" autocomplete=\"email\" /><br>" + + "<input type=\"text\" id=\"ph\" autocomplete=\"tel\" /><br>" + + "<input type=\"text\" id=\"fx\" autocomplete=\"fax\" /><br>" + + "<select id=\"co\" autocomplete=\"country\"><br>" + + "<option value=\"BR\">Brazil</option>" + + "<option value=\"US\">United States</option>" + + "</select>" + + "<input type=\"submit\" />" + + "</form></body></html>")); + assertTrue(waitForActiveShellToBeDoneLoading()); + new AutofillTestHelper().setProfile(new AutofillProfile("", "https://www.example.com", + "John Smith", "Acme Inc", "1 Main\nApt A", "CA", "San Francisco", "", "94102", "", + "US", "(415) 888-9999", "john@acme.inc", "en")); + final AtomicReference<ContentViewCore> viewCoreRef = new AtomicReference<ContentViewCore>(); + final AtomicReference<WebContents> webContentsRef = new AtomicReference<WebContents>(); + final AtomicReference<ViewGroup> viewRef = new AtomicReference<ViewGroup>(); + ThreadUtils.runOnUiThreadBlocking(new Runnable() { + @Override + public void run() { + viewCoreRef.set(getActivity().getActiveContentViewCore()); + webContentsRef.set(viewCoreRef.get().getWebContents()); + viewRef.set(viewCoreRef.get().getContainerView()); + } + }); + assertTrue(DOMUtils.waitForNonZeroNodeBounds(webContentsRef.get(), "fn")); + + // Click on the unfocused input element for the first time to focus on it. This brings up + // the keyboard, but does not show the autofill popup. + DOMUtils.clickNode(this, viewCoreRef.get(), "fn"); + waitForKeyboardShown(true); + + // Hide the keyboard while still keeping the focus on the input field. + ThreadUtils.runOnUiThreadBlocking(new Runnable() { + @Override + public void run() { + UiUtils.hideKeyboard(viewRef.get()); + } + }); + waitForKeyboardShown(false); + + // Click on the focused input element for the second time. This brings up the autofill popup + // and shows the keyboard at the same time. Showing the keyboard should not hide the + // autofill popup. + DOMUtils.clickNode(this, viewCoreRef.get(), "fn"); + waitForKeyboardShown(true); + + // Verify that the autofill popup is showing. + assertTrue("Autofill Popup anchor view was never added.", + CriteriaHelper.pollForUIThreadCriteria(new Criteria() { + @Override + public boolean isSatisfied() { + return viewRef.get().findViewById(R.id.dropdown_popup_window) != null; + } + })); + Object popupObject = ThreadUtils.runOnUiThreadBlocking(new Callable<Object>() { + @Override + public Object call() { + return viewRef.get().findViewById(R.id.dropdown_popup_window).getTag(); + } + }); + assertTrue(popupObject instanceof AutofillPopup); + final AutofillPopup popup = (AutofillPopup) popupObject; + assertTrue("Autofill Popup was never shown.", + CriteriaHelper.pollForUIThreadCriteria(new Criteria() { + @Override + public boolean isSatisfied() { + return popup.isShowing(); + } + })); + } + + /** + * Wait until the keyboard is showing and notify the ContentViewCore that its height was changed + * on the UI thread. + */ + private void waitForKeyboardShown(final boolean shown) throws InterruptedException { + assertTrue((shown ? "Keyboard was never shown" : "Keyboard was never hidden"), + CriteriaHelper.pollForUIThreadCriteria(new Criteria() { + @Override + public boolean isSatisfied() { + return shown == UiUtils.isKeyboardShowing( + getActivity(), + getActivity().getActiveContentViewCore().getContainerView()); + } + })); + ThreadUtils.runOnUiThreadBlocking(new Runnable() { + @Override + public void run() { + ContentViewCore viewCore = getActivity().getActiveContentViewCore(); + viewCore.onSizeChanged(viewCore.getViewportWidthPix(), + viewCore.getViewportHeightPix() + (shown ? -100 : 100), + viewCore.getViewportWidthPix(), viewCore.getViewportHeightPix()); + } + }); + } +} 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 diff --git a/components/autofill/content/renderer/autofill_agent.cc b/components/autofill/content/renderer/autofill_agent.cc index 2c48d11..93d06956 100644 --- a/components/autofill/content/renderer/autofill_agent.cc +++ b/components/autofill/content/renderer/autofill_agent.cc @@ -49,6 +49,7 @@ using blink::WebAutofillClient; using blink::WebConsoleMessage; +using blink::WebDocument; using blink::WebElement; using blink::WebElementCollection; using blink::WebFormControlElement; @@ -215,12 +216,6 @@ void AutofillAgent::FocusedNodeChanged(const WebNode& node) { if (node.document().frame() != render_frame()->GetWebFrame()) return; - if (password_generation_agent_ && - password_generation_agent_->FocusedNodeHasChanged(node)) { - is_popup_possibly_visible_ = true; - return; - } - WebElement web_element = node.toConst<WebElement>(); if (!web_element.document().frame()) @@ -229,12 +224,24 @@ void AutofillAgent::FocusedNodeChanged(const WebNode& node) { const WebInputElement* element = toWebInputElement(&web_element); if (!element || !element->isEnabled() || element->isReadOnly() || - !element->isTextField() || element->isPasswordField()) + !element->isTextField()) return; element_ = *element; } +void AutofillAgent::FocusChangeComplete() { + WebDocument doc = render_frame()->GetWebFrame()->document(); + WebElement focused_element; + if (!doc.isNull()) + focused_element = doc.focusedElement(); + + if (!focused_element.isNull() && password_generation_agent_ && + password_generation_agent_->FocusedNodeHasChanged(focused_element)) { + is_popup_possibly_visible_ = true; + } +} + void AutofillAgent::didRequestAutocomplete( const WebFormElement& form) { DCHECK_EQ(form.document().frame(), render_frame()->GetWebFrame()); @@ -783,4 +790,8 @@ void AutofillAgent::LegacyAutofillAgent::FocusedNodeChanged( agent_->FocusedNodeChanged(node); } +void AutofillAgent::LegacyAutofillAgent::FocusChangeComplete() { + agent_->FocusChangeComplete(); +} + } // namespace autofill diff --git a/components/autofill/content/renderer/autofill_agent.h b/components/autofill/content/renderer/autofill_agent.h index 13c35ad..c03ce87 100644 --- a/components/autofill/content/renderer/autofill_agent.h +++ b/components/autofill/content/renderer/autofill_agent.h @@ -67,6 +67,7 @@ class AutofillAgent : public content::RenderFrameObserver, // content::RenderViewObserver: void OnDestruct() override; void FocusedNodeChanged(const blink::WebNode& node) override; + void FocusChangeComplete() override; AutofillAgent* agent_; @@ -117,6 +118,7 @@ class AutofillAgent : public content::RenderFrameObserver, // Pass-through from LegacyAutofillAgent. This correlates with the // RenderViewObserver method. void FocusedNodeChanged(const blink::WebNode& node); + void FocusChangeComplete(); // PageClickListener: void FormControlElementClicked(const blink::WebFormControlElement& element, diff --git a/components/autofill/content/renderer/form_autofill_util.cc b/components/autofill/content/renderer/form_autofill_util.cc index 8c62c8b7..93aeaa0 100644 --- a/components/autofill/content/renderer/form_autofill_util.cc +++ b/components/autofill/content/renderer/form_autofill_util.cc @@ -1364,7 +1364,7 @@ bool IsWebElementEmpty(const blink::WebElement& element) { return true; } -gfx::RectF GetScaledBoundingBox(float scale, WebFormControlElement* element) { +gfx::RectF GetScaledBoundingBox(float scale, WebElement* element) { gfx::Rect bounding_box(element->boundsInViewportSpace()); return gfx::RectF(bounding_box.x() * scale, bounding_box.y() * scale, diff --git a/components/autofill/content/renderer/form_autofill_util.h b/components/autofill/content/renderer/form_autofill_util.h index 061332b..b3a299d 100644 --- a/components/autofill/content/renderer/form_autofill_util.h +++ b/components/autofill/content/renderer/form_autofill_util.h @@ -186,8 +186,7 @@ bool IsWebpageEmpty(const blink::WebFrame* frame); bool IsWebElementEmpty(const blink::WebElement& element); // Return a gfx::RectF that is the bounding box for |element| scaled by |scale|. -gfx::RectF GetScaledBoundingBox(float scale, - blink::WebFormControlElement* element); +gfx::RectF GetScaledBoundingBox(float scale, blink::WebElement* element); } // namespace autofill diff --git a/components/autofill/content/renderer/page_click_tracker.cc b/components/autofill/content/renderer/page_click_tracker.cc index 14ceda8..4e54f9e 100644 --- a/components/autofill/content/renderer/page_click_tracker.cc +++ b/components/autofill/content/renderer/page_click_tracker.cc @@ -4,13 +4,9 @@ #include "components/autofill/content/renderer/page_click_tracker.h" -#include "base/bind.h" -#include "base/message_loop/message_loop.h" #include "components/autofill/content/renderer/form_autofill_util.h" #include "components/autofill/content/renderer/page_click_listener.h" #include "content/public/renderer/render_view.h" -#include "third_party/WebKit/public/platform/WebString.h" -#include "third_party/WebKit/public/web/WebDOMMouseEvent.h" #include "third_party/WebKit/public/web/WebDocument.h" #include "third_party/WebKit/public/web/WebInputElement.h" #include "third_party/WebKit/public/web/WebInputEvent.h" @@ -18,19 +14,13 @@ #include "third_party/WebKit/public/web/WebTextAreaElement.h" #include "third_party/WebKit/public/web/WebView.h" -using blink::WebDOMEvent; -using blink::WebDOMMouseEvent; using blink::WebElement; -using blink::WebFormControlElement; -using blink::WebFrame; using blink::WebGestureEvent; using blink::WebInputElement; using blink::WebInputEvent; using blink::WebMouseEvent; using blink::WebNode; -using blink::WebString; using blink::WebTextAreaElement; -using blink::WebView; namespace { @@ -68,8 +58,7 @@ PageClickTracker::PageClickTracker(content::RenderView* render_view, PageClickListener* listener) : content::RenderViewObserver(render_view), was_focused_before_now_(false), - listener_(listener), - weak_ptr_factory_(this) { + listener_(listener) { } PageClickTracker::~PageClickTracker() { @@ -88,50 +77,48 @@ void PageClickTracker::DidHandleMouseEvent(const WebMouseEvent& event) { PotentialActivationAt(event.x, event.y); } -void PageClickTracker::DidHandleGestureEvent( - const blink::WebGestureEvent& event) { - if (event.type != blink::WebGestureEvent::GestureTap) +void PageClickTracker::DidHandleGestureEvent(const WebGestureEvent& event) { + if (event.type != WebGestureEvent::GestureTap) return; PotentialActivationAt(event.x, event.y); } -void PageClickTracker::FocusedNodeChanged(const blink::WebNode& node) { +void PageClickTracker::FocusedNodeChanged(const WebNode& node) { was_focused_before_now_ = false; - // If the focus change was a result of handling a click or tap, we'll soon get - // an associated event. Reset |was_focused_before_now_| to true only after the - // message loop unwinds. - base::MessageLoop::current()->PostTask( - FROM_HERE, - base::Bind(&PageClickTracker::SetWasFocused, - weak_ptr_factory_.GetWeakPtr())); } -void PageClickTracker::PotentialActivationAt(int x, int y) { - blink::WebNode focused_node = render_view()->GetFocusedElement(); - if (focused_node.isNull()) - return; +void PageClickTracker::FocusChangeComplete() { + if (!clicked_node_.isNull()) { + const WebInputElement input_element = GetTextWebInputElement(clicked_node_); + if (!input_element.isNull()) { + listener_->FormControlElementClicked(input_element, + was_focused_before_now_); + } else { + const WebTextAreaElement textarea_element = + GetWebTextAreaElement(clicked_node_); + if (!textarea_element.isNull()) { + listener_->FormControlElementClicked(textarea_element, + was_focused_before_now_); + } + } + } - if (!render_view()->NodeContainsPoint(focused_node, gfx::Point(x, y))) - return; + clicked_node_.reset(); + was_focused_before_now_ = true; +} - const WebInputElement input_element = GetTextWebInputElement(focused_node); - if (!input_element.isNull()) { - listener_->FormControlElementClicked(input_element, - was_focused_before_now_); +void PageClickTracker::PotentialActivationAt(int x, int y) { + WebElement focused_element = render_view()->GetFocusedElement(); + if (focused_element.isNull()) return; - } - const WebTextAreaElement textarea_element = - GetWebTextAreaElement(focused_node); - if (!textarea_element.isNull()) { - listener_->FormControlElementClicked(textarea_element, - was_focused_before_now_); + if (!GetScaledBoundingBox(render_view()->GetWebView()->pageScaleFactor(), + &focused_element).Contains(x, y)) { + return; } -} -void PageClickTracker::SetWasFocused() { - was_focused_before_now_ = true; + clicked_node_ = focused_element; } } // namespace autofill diff --git a/components/autofill/content/renderer/page_click_tracker.h b/components/autofill/content/renderer/page_click_tracker.h index 0bcc0f6..9cb6353 100644 --- a/components/autofill/content/renderer/page_click_tracker.h +++ b/components/autofill/content/renderer/page_click_tracker.h @@ -16,7 +16,7 @@ namespace autofill { class PageClickListener; -// This class is responsible notifiying the associated listener when a node is +// This class is responsible notifying the associated listener when a node is // clicked or tapped. It also tracks whether a node was focused before the event // was handled. // @@ -39,12 +39,14 @@ class PageClickTracker : public content::RenderViewObserver { void DidHandleMouseEvent(const blink::WebMouseEvent& event) override; void DidHandleGestureEvent(const blink::WebGestureEvent& event) override; void FocusedNodeChanged(const blink::WebNode& node) override; + void FocusChangeComplete() override; // Called there is a tap or click at |x|, |y|. void PotentialActivationAt(int x, int y); - // Sets |was_focused_before_now_| to true. - void SetWasFocused(); + // The node that was clicked. Non-null only when the animations caused by + // focus change are still ongoing. + blink::WebNode clicked_node_; // This is set to false when the focus changes, then set back to true soon // afterwards. This helps track whether an event happened after a node was @@ -54,8 +56,6 @@ class PageClickTracker : public content::RenderViewObserver { // The listener getting the actual notifications. PageClickListener* listener_; - base::WeakPtrFactory<PageClickTracker> weak_ptr_factory_; - DISALLOW_COPY_AND_ASSIGN(PageClickTracker); }; diff --git a/content/browser/renderer_host/compositor_impl_android.h b/content/browser/renderer_host/compositor_impl_android.h index 0b60c23..9bffa4b 100644 --- a/content/browser/renderer_host/compositor_impl_android.h +++ b/content/browser/renderer_host/compositor_impl_android.h @@ -84,6 +84,7 @@ class CONTENT_EXPORT CompositorImpl virtual void DidCommit() override; virtual void DidCommitAndDrawFrame() override {} virtual void DidCompleteSwapBuffers() override; + virtual void DidCompletePageScaleAnimation() override {} // LayerTreeHostSingleThreadClient implementation. virtual void ScheduleComposite() override; diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java index c45d61d..674f172 100644 --- a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java +++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java @@ -660,6 +660,12 @@ public class ContentViewCore } @Override + public void onKeyboardBoundsUnchanged() { + assert mWebContents != null; + mWebContents.scrollFocusedEditableNodeIntoView(); + } + + @Override public View getAttachedView() { return mContainerView; } diff --git a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java b/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java index 6f49bb4..a77aaa3 100644 --- a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java +++ b/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java @@ -4,6 +4,7 @@ package org.chromium.content.browser.input; +import android.content.res.Configuration; import android.os.Handler; import android.os.ResultReceiver; import android.os.SystemClock; @@ -67,6 +68,11 @@ public class ImeAdapter { void onDismissInput(); /** + * Called when the keyboard could not be shown due to the hardware keyboard being present. + */ + void onKeyboardBoundsUnchanged(); + + /** * @return View that the keyboard should be attached to. */ View getAttachedView(); @@ -274,8 +280,13 @@ public class ImeAdapter { private void showKeyboard() { mIsShowWithoutHideOutstanding = true; - mInputMethodManagerWrapper.showSoftInput(mViewEmbedder.getAttachedView(), 0, - mViewEmbedder.getNewShowKeyboardReceiver()); + if (mViewEmbedder.getAttachedView().getResources().getConfiguration().keyboard + == Configuration.KEYBOARD_NOKEYS) { + mInputMethodManagerWrapper.showSoftInput(mViewEmbedder.getAttachedView(), 0, + mViewEmbedder.getNewShowKeyboardReceiver()); + } else { + mViewEmbedder.onKeyboardBoundsUnchanged(); + } } private void dismissInput(boolean unzoomIfNeeded) { diff --git a/content/public/android/javatests/src/org/chromium/content/browser/input/AdapterInputConnectionTest.java b/content/public/android/javatests/src/org/chromium/content/browser/input/AdapterInputConnectionTest.java index 96a52d2..deb151e 100644 --- a/content/public/android/javatests/src/org/chromium/content/browser/input/AdapterInputConnectionTest.java +++ b/content/public/android/javatests/src/org/chromium/content/browser/input/AdapterInputConnectionTest.java @@ -193,6 +193,9 @@ public class AdapterInputConnectionTest extends ContentShellTestBase { public void onDismissInput() {} @Override + public void onKeyboardBoundsUnchanged() {} + + @Override public View getAttachedView() { return null; } diff --git a/content/public/renderer/render_view_observer.h b/content/public/renderer/render_view_observer.h index 9b8c71e..6952735 100644 --- a/content/public/renderer/render_view_observer.h +++ b/content/public/renderer/render_view_observer.h @@ -90,6 +90,11 @@ class CONTENT_EXPORT RenderViewObserver : public IPC::Listener, virtual void Navigate(const GURL& url) {} virtual void ClosePage() {} + // This indicates that animations to scroll the focused element into view (if + // any) have completed. May be called more than once for a single focus. Can + // be called from browser, renderer, or compositor. + virtual void FocusChangeComplete() {} + virtual void OnStop() {} // IPC::Listener implementation. diff --git a/content/public/test/render_view_test.cc b/content/public/test/render_view_test.cc index abda557..549dd9f 100644 --- a/content/public/test/render_view_test.cc +++ b/content/public/test/render_view_test.cc @@ -322,17 +322,23 @@ bool RenderViewTest::SimulateElementClick(const std::string& element_id) { gfx::Rect bounds = GetElementBounds(element_id); if (bounds.IsEmpty()) return false; + SimulatePointClick(bounds.CenterPoint()); + return true; +} + +void RenderViewTest::SimulatePointClick(const gfx::Point& point) { WebMouseEvent mouse_event; mouse_event.type = WebInputEvent::MouseDown; mouse_event.button = WebMouseEvent::ButtonLeft; - mouse_event.x = bounds.CenterPoint().x(); - mouse_event.y = bounds.CenterPoint().y(); + mouse_event.x = point.x(); + mouse_event.y = point.y(); mouse_event.clickCount = 1; - scoped_ptr<IPC::Message> input_message( - new InputMsg_HandleInputEvent(0, &mouse_event, ui::LatencyInfo(), false)); RenderViewImpl* impl = static_cast<RenderViewImpl*>(view_); - impl->OnMessageReceived(*input_message); - return true; + impl->OnMessageReceived( + InputMsg_HandleInputEvent(0, &mouse_event, ui::LatencyInfo(), false)); + mouse_event.type = WebInputEvent::MouseUp; + impl->OnMessageReceived( + InputMsg_HandleInputEvent(0, &mouse_event, ui::LatencyInfo(), false)); } void RenderViewTest::SetFocused(const blink::WebNode& node) { diff --git a/content/public/test/render_view_test.h b/content/public/test/render_view_test.h index 7f1709e..936c329 100644 --- a/content/public/test/render_view_test.h +++ b/content/public/test/render_view_test.h @@ -107,6 +107,9 @@ class RenderViewTest : public testing::Test { // the element was not found). bool SimulateElementClick(const std::string& element_id); + // Sends a left mouse click at the |point|. + void SimulatePointClick(const gfx::Point& point); + // Simulates |node| being focused. void SetFocused(const blink::WebNode& node); diff --git a/content/renderer/gpu/render_widget_compositor.cc b/content/renderer/gpu/render_widget_compositor.cc index c5986e8..51cf48e 100644 --- a/content/renderer/gpu/render_widget_compositor.cc +++ b/content/renderer/gpu/render_widget_compositor.cc @@ -858,6 +858,10 @@ void RenderWidgetCompositor::DidCompleteSwapBuffers() { widget_->OnSwapBuffersComplete(); } +void RenderWidgetCompositor::DidCompletePageScaleAnimation() { + widget_->DidCompletePageScaleAnimation(); +} + void RenderWidgetCompositor::ScheduleAnimation() { widget_->scheduleAnimation(); } diff --git a/content/renderer/gpu/render_widget_compositor.h b/content/renderer/gpu/render_widget_compositor.h index d1e1e76..c36a945 100644 --- a/content/renderer/gpu/render_widget_compositor.h +++ b/content/renderer/gpu/render_widget_compositor.h @@ -152,6 +152,7 @@ class CONTENT_EXPORT RenderWidgetCompositor void DidCommit() override; void DidCommitAndDrawFrame() override; void DidCompleteSwapBuffers() override; + void DidCompletePageScaleAnimation() override; void RateLimitSharedMainThreadContext() override; // cc::LayerTreeHostSingleThreadClient implementation. diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc index 851e81c..79dce61 100644 --- a/content/renderer/render_view_impl.cc +++ b/content/renderer/render_view_impl.cc @@ -1423,15 +1423,20 @@ void RenderViewImpl::OnScrollFocusedEditableNodeIntoRect( const gfx::Rect& rect) { if (has_scrolled_focused_editable_node_into_rect_ && rect == rect_for_scrolled_focused_editable_node_) { + FocusChangeComplete(); return; } blink::WebElement element = GetFocusedElement(); + bool will_animate = false; if (!element.isNull() && IsEditableNode(element)) { rect_for_scrolled_focused_editable_node_ = rect; has_scrolled_focused_editable_node_into_rect_ = true; - webview()->scrollFocusedNodeIntoRect(rect); + will_animate = webview()->scrollFocusedNodeIntoRect(rect); } + + if (!will_animate) + FocusChangeComplete(); } void RenderViewImpl::OnSetEditCommandsForNextKeyEvent( @@ -3601,6 +3606,11 @@ void RenderViewImpl::GetSelectionBounds(gfx::Rect* start, gfx::Rect* end) { RenderWidget::GetSelectionBounds(start, end); } +void RenderViewImpl::FocusChangeComplete() { + RenderWidget::FocusChangeComplete(); + FOR_EACH_OBSERVER(RenderViewObserver, observers_, FocusChangeComplete()); +} + void RenderViewImpl::GetCompositionCharacterBounds( std::vector<gfx::Rect>* bounds) { DCHECK(bounds); @@ -3687,6 +3697,10 @@ void RenderViewImpl::InstrumentWillComposite() { webview()->devToolsAgent()->willComposite(); } +void RenderViewImpl::DidCompletePageScaleAnimation() { + FocusChangeComplete(); +} + void RenderViewImpl::SetScreenMetricsEmulationParameters( float device_scale_factor, const gfx::Point& root_layer_offset, diff --git a/content/renderer/render_view_impl.h b/content/renderer/render_view_impl.h index f3d55fa..c5029e4 100644 --- a/content/renderer/render_view_impl.h +++ b/content/renderer/render_view_impl.h @@ -497,6 +497,7 @@ class CONTENT_EXPORT RenderViewImpl void OnOrientationChange() override; ui::TextInputType GetTextInputType() override; void GetSelectionBounds(gfx::Rect* start, gfx::Rect* end) override; + void FocusChangeComplete() override; void GetCompositionCharacterBounds( std::vector<gfx::Rect>* character_bounds) override; void GetCompositionRange(gfx::Range* range) override; @@ -506,6 +507,7 @@ class CONTENT_EXPORT RenderViewImpl void InstrumentDidBeginFrame() override; void InstrumentDidCancelFrame() override; void InstrumentWillComposite() override; + void DidCompletePageScaleAnimation() override; protected: explicit RenderViewImpl(const ViewMsg_New_Params& params); diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc index c2eefbd..a22d374 100644 --- a/content/renderer/render_widget.cc +++ b/content/renderer/render_widget.cc @@ -1230,8 +1230,9 @@ void RenderWidget::OnHandleInputEvent(const blink::WebInputEvent* input_event, // Show the virtual keyboard if enabled and a user gesture triggers a focus // change. if (processed && (input_event->type == WebInputEvent::TouchEnd || - input_event->type == WebInputEvent::MouseUp)) + input_event->type == WebInputEvent::MouseUp)) { UpdateTextInputState(SHOW_IME_IF_NEEDED, FROM_IME); + } #endif if (!prevent_default) { @@ -1242,6 +1243,16 @@ void RenderWidget::OnHandleInputEvent(const blink::WebInputEvent* input_event, if (WebInputEvent::isTouchEventType(input_event->type)) DidHandleTouchEvent(*(static_cast<const WebTouchEvent*>(input_event))); } + +// TODO(rouslan): Fix ChromeOS and Windows 8 behavior of autofill popup with +// virtual keyboard. +#if !defined(OS_ANDROID) + // Virtual keyboard is not supported, so react to focus change immediately. + if (processed && (input_event->type == WebInputEvent::TouchEnd || + input_event->type == WebInputEvent::MouseUp)) { + FocusChangeComplete(); + } +#endif } void RenderWidget::OnCursorVisibilityChange(bool is_visible) { @@ -1697,6 +1708,12 @@ void RenderWidget::OnShowImeIfNeeded() { #if defined(OS_ANDROID) || defined(USE_AURA) UpdateTextInputState(SHOW_IME_IF_NEEDED, FROM_NON_IME); #endif + +// TODO(rouslan): Fix ChromeOS and Windows 8 behavior of autofill popup with +// virtual keyboard. +#if !defined(OS_ANDROID) + FocusChangeComplete(); +#endif } #if defined(OS_ANDROID) diff --git a/content/renderer/render_widget.h b/content/renderer/render_widget.h index 093ea8d9..a9491cc 100644 --- a/content/renderer/render_widget.h +++ b/content/renderer/render_widget.h @@ -243,6 +243,9 @@ class CONTENT_EXPORT RenderWidget virtual void InstrumentDidCancelFrame() {} virtual void InstrumentWillComposite() {} + // Called by the compositor when page scale animation completed. + virtual void DidCompletePageScaleAnimation() {} + // When paused in debugger, we send ack for mouse event early. This ensures // that we continue receiving mouse moves and pass them to debugger. Returns // whether we are paused in mouse move event and have sent the ack. @@ -306,6 +309,10 @@ class CONTENT_EXPORT RenderWidget void UpdateTextInputState(ShowIme show_ime, ChangeSource change_source); #endif + // Called when animations due to focus change have completed (if any). Can be + // called from the renderer, browser, or compositor. + virtual void FocusChangeComplete() {} + // Checks if the composition range or composition character bounds have been // changed. If they are changed, the new value will be sent to the browser // process. This method does nothing when the browser process is not able to diff --git a/content/test/web_layer_tree_view_impl_for_testing.h b/content/test/web_layer_tree_view_impl_for_testing.h index 2d316c9..9dabf4c 100644 --- a/content/test/web_layer_tree_view_impl_for_testing.h +++ b/content/test/web_layer_tree_view_impl_for_testing.h @@ -84,6 +84,7 @@ class WebLayerTreeViewImplForTesting void DidCommit() override {} void DidCommitAndDrawFrame() override {} void DidCompleteSwapBuffers() override {} + void DidCompletePageScaleAnimation() override {} // cc::LayerTreeHostSingleThreadClient implementation. void DidPostSwapBuffers() override {} diff --git a/mojo/services/html_viewer/weblayertreeview_impl.h b/mojo/services/html_viewer/weblayertreeview_impl.h index 3212643..02cb979 100644 --- a/mojo/services/html_viewer/weblayertreeview_impl.h +++ b/mojo/services/html_viewer/weblayertreeview_impl.h @@ -67,6 +67,7 @@ class WebLayerTreeViewImpl : public blink::WebLayerTreeView, void DidCommit() override; void DidCommitAndDrawFrame() override; void DidCompleteSwapBuffers() override; + void DidCompletePageScaleAnimation() override {} void RateLimitSharedMainThreadContext() override {} // blink::WebLayerTreeView implementation. diff --git a/ui/compositor/compositor.h b/ui/compositor/compositor.h index 94cd1a91..20a8da1 100644 --- a/ui/compositor/compositor.h +++ b/ui/compositor/compositor.h @@ -264,6 +264,7 @@ class COMPOSITOR_EXPORT Compositor void DidCommit() override; void DidCommitAndDrawFrame() override; void DidCompleteSwapBuffers() override; + void DidCompletePageScaleAnimation() override {} // cc::LayerTreeHostSingleThreadClient implementation. void ScheduleComposite() override; |