diff options
author | dominickn <dominickn@chromium.org> | 2016-03-24 20:06:22 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-03-25 03:08:00 +0000 |
commit | 5cc29a417bcb7b86ca8ebc4304eefa4d74a7e919 (patch) | |
tree | 59e328c0344387a03921ca149ed88a7800922d21 | |
parent | 5f7188d95a25fca7424f0d6b56288139b85cf9e4 (diff) | |
download | chromium_src-5cc29a417bcb7b86ca8ebc4304eefa4d74a7e919.zip chromium_src-5cc29a417bcb7b86ca8ebc4304eefa4d74a7e919.tar.gz chromium_src-5cc29a417bcb7b86ca8ebc4304eefa4d74a7e919.tar.bz2 |
Replace MouseWheel events with GestureScrollBegin in WebContentsObserver::DidGetUserInteraction
crbug.com/568183 implements GestureScrollBegin for mouse wheel events.
Once this has landed, the mouse event coalescing currently performed by
WebContentsObserver::DidGetUserInteraction will be redundant.
This CL removes the mouse event coalescing, and changes
DidGetUserInteraction to fire on GestureScrollBegin events. The event
type used to signal scrolls in the method is changed from MouseWheel to
GestureScrollBegin, as scroll events will now be fired on mobile scrolls
as well as wheel scrolls. Finally, in clients of DidGetUserInteraction,
MouseWheel is replaced with GestureScrollBegin, and references to
'wheel' are replaced with 'scroll'.
BUG=590612
Review URL: https://codereview.chromium.org/1748553002
Cr-Commit-Position: refs/heads/master@{#383231}
12 files changed, 27 insertions, 56 deletions
diff --git a/chrome/browser/download/download_request_limiter.cc b/chrome/browser/download/download_request_limiter.cc index 5159276..194910b 100644 --- a/chrome/browser/download/download_request_limiter.cc +++ b/chrome/browser/download/download_request_limiter.cc @@ -99,7 +99,7 @@ void DownloadRequestLimiter::TabDownloadState::DidNavigateMainFrame( void DownloadRequestLimiter::TabDownloadState::DidGetUserInteraction( const blink::WebInputEvent::Type type) { - if (is_showing_prompt() || type == blink::WebInputEvent::MouseWheel) { + if (is_showing_prompt() || type == blink::WebInputEvent::GestureScrollBegin) { // Don't change state if a prompt is showing or if the user has scrolled. return; } diff --git a/chrome/browser/download/download_request_limiter_unittest.cc b/chrome/browser/download/download_request_limiter_unittest.cc index 91e9425..ad4cdc1 100644 --- a/chrome/browser/download/download_request_limiter_unittest.cc +++ b/chrome/browser/download/download_request_limiter_unittest.cc @@ -354,8 +354,8 @@ TEST_F(DownloadRequestLimiterTest, DownloadRequestLimiter_ResetOnUserGesture) { ASSERT_EQ(DownloadRequestLimiter::PROMPT_BEFORE_DOWNLOAD, download_request_limiter_->GetDownloadStatus(web_contents())); - // Do a user gesture with mouse scroll, which should be ignored. - OnUserInteraction(blink::WebInputEvent::MouseWheel); + // Do a user gesture with scroll, which should be ignored. + OnUserInteraction(blink::WebInputEvent::GestureScrollBegin); ASSERT_EQ(DownloadRequestLimiter::PROMPT_BEFORE_DOWNLOAD, download_request_limiter_->GetDownloadStatus(web_contents())); // Do a user gesture with mouse click, which should reset back to allow one. diff --git a/chrome/browser/engagement/site_engagement_helper.cc b/chrome/browser/engagement/site_engagement_helper.cc index 93a05ef..265580e 100644 --- a/chrome/browser/engagement/site_engagement_helper.cc +++ b/chrome/browser/engagement/site_engagement_helper.cc @@ -104,8 +104,8 @@ void SiteEngagementHelper::InputTracker::DidGetUserInteraction( helper()->RecordUserInput( SiteEngagementMetrics::ENGAGEMENT_TOUCH_GESTURE); break; - case blink::WebInputEvent::MouseWheel: - helper()->RecordUserInput(SiteEngagementMetrics::ENGAGEMENT_WHEEL); + case blink::WebInputEvent::GestureScrollBegin: + helper()->RecordUserInput(SiteEngagementMetrics::ENGAGEMENT_SCROLL); break; case blink::WebInputEvent::Undefined: // Explicitly ignore browser-initiated navigation input. diff --git a/chrome/browser/engagement/site_engagement_helper_unittest.cc b/chrome/browser/engagement/site_engagement_helper_unittest.cc index db00c59..88a0719 100644 --- a/chrome/browser/engagement/site_engagement_helper_unittest.cc +++ b/chrome/browser/engagement/site_engagement_helper_unittest.cc @@ -146,8 +146,8 @@ TEST_F(SiteEngagementHelperTest, MouseDownEventEngagementAccumulation) { UserInputAccumulation(blink::WebInputEvent::MouseDown); } -TEST_F(SiteEngagementHelperTest, MouseWheelEventEngagementAccumulation) { - UserInputAccumulation(blink::WebInputEvent::MouseWheel); +TEST_F(SiteEngagementHelperTest, ScrollEventEngagementAccumulation) { + UserInputAccumulation(blink::WebInputEvent::GestureScrollBegin); } TEST_F(SiteEngagementHelperTest, GestureEngagementAccumulation) { @@ -333,7 +333,8 @@ TEST_F(SiteEngagementHelperTest, MixedInputEngagementAccumulation) { SiteEngagementMetrics::kEngagementTypeHistogram, SiteEngagementMetrics::ENGAGEMENT_FIRST_DAILY_ENGAGEMENT, 1); - HandleUserInputAndRestartTracking(helper, blink::WebInputEvent::MouseWheel); + HandleUserInputAndRestartTracking(helper, + blink::WebInputEvent::GestureScrollBegin); HandleUserInputAndRestartTracking(helper, blink::WebInputEvent::MouseDown); HandleMediaPlaying(helper, true); HandleUserInputAndRestartTracking(helper, @@ -347,7 +348,7 @@ TEST_F(SiteEngagementHelperTest, MixedInputEngagementAccumulation) { histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram, SiteEngagementMetrics::ENGAGEMENT_MOUSE, 2); histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram, - SiteEngagementMetrics::ENGAGEMENT_WHEEL, 1); + SiteEngagementMetrics::ENGAGEMENT_SCROLL, 1); histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram, SiteEngagementMetrics::ENGAGEMENT_TOUCH_GESTURE, 3); diff --git a/chrome/browser/engagement/site_engagement_metrics.h b/chrome/browser/engagement/site_engagement_metrics.h index 9f77cbf..85434a6 100644 --- a/chrome/browser/engagement/site_engagement_metrics.h +++ b/chrome/browser/engagement/site_engagement_metrics.h @@ -22,7 +22,7 @@ class SiteEngagementMetrics { ENGAGEMENT_KEYPRESS, ENGAGEMENT_MOUSE, ENGAGEMENT_TOUCH_GESTURE, - ENGAGEMENT_WHEEL, + ENGAGEMENT_SCROLL, ENGAGEMENT_MEDIA_HIDDEN, ENGAGEMENT_MEDIA_VISIBLE, ENGAGEMENT_WEBAPP_SHORTCUT_LAUNCH, diff --git a/chrome/browser/engagement/site_engagement_service_unittest.cc b/chrome/browser/engagement/site_engagement_service_unittest.cc index be3fc1e..44abc37 100644 --- a/chrome/browser/engagement/site_engagement_service_unittest.cc +++ b/chrome/browser/engagement/site_engagement_service_unittest.cc @@ -643,7 +643,7 @@ TEST_F(SiteEngagementServiceTest, GetTotalUserInputPoints) { EXPECT_DOUBLE_EQ(0.15, service->GetScore(url1)); EXPECT_DOUBLE_EQ(0.3, service->GetTotalEngagementPoints()); - service->HandleUserInput(url2, SiteEngagementMetrics::ENGAGEMENT_WHEEL); + service->HandleUserInput(url2, SiteEngagementMetrics::ENGAGEMENT_SCROLL); service->HandleUserInput(url3, SiteEngagementMetrics::ENGAGEMENT_TOUCH_GESTURE); EXPECT_DOUBLE_EQ(0.15, service->GetScore(url2)); @@ -906,7 +906,7 @@ TEST_F(SiteEngagementServiceTest, CheckHistograms) { service->HandleNavigation(url1, ui::PAGE_TRANSITION_GENERATED); service->HandleNavigation(url1, ui::PAGE_TRANSITION_TYPED); - service->HandleUserInput(url2, SiteEngagementMetrics::ENGAGEMENT_WHEEL); + service->HandleUserInput(url2, SiteEngagementMetrics::ENGAGEMENT_SCROLL); service->HandleUserInput(url1, SiteEngagementMetrics::ENGAGEMENT_KEYPRESS); service->HandleUserInput(url3, SiteEngagementMetrics::ENGAGEMENT_MOUSE); @@ -922,7 +922,7 @@ TEST_F(SiteEngagementServiceTest, CheckHistograms) { SiteEngagementMetrics::ENGAGEMENT_TOUCH_GESTURE, 1); histograms.ExpectBucketCount(SiteEngagementMetrics::kEngagementTypeHistogram, - SiteEngagementMetrics::ENGAGEMENT_WHEEL, 1); + SiteEngagementMetrics::ENGAGEMENT_SCROLL, 1); histograms.ExpectBucketCount( SiteEngagementMetrics::kEngagementTypeHistogram, SiteEngagementMetrics::ENGAGEMENT_FIRST_DAILY_ENGAGEMENT, 3); diff --git a/chrome/browser/external_protocol/external_protocol_observer.cc b/chrome/browser/external_protocol/external_protocol_observer.cc index ec66503..d6ac7a8 100644 --- a/chrome/browser/external_protocol/external_protocol_observer.cc +++ b/chrome/browser/external_protocol/external_protocol_observer.cc @@ -20,6 +20,6 @@ ExternalProtocolObserver::~ExternalProtocolObserver() { void ExternalProtocolObserver::DidGetUserInteraction( const blink::WebInputEvent::Type type) { // Ignore scroll events for allowing external protocol launch. - if (type != blink::WebInputEvent::MouseWheel) + if (type != blink::WebInputEvent::GestureScrollBegin) ExternalProtocolHandler::PermitLaunchUrl(); } diff --git a/content/browser/renderer_host/render_widget_host_delegate.h b/content/browser/renderer_host/render_widget_host_delegate.h index d80d502..aa38066 100644 --- a/content/browser/renderer_host/render_widget_host_delegate.h +++ b/content/browser/renderer_host/render_widget_host_delegate.h @@ -78,11 +78,10 @@ class CONTENT_EXPORT RenderWidgetHostDelegate { // the event itself. virtual bool HandleWheelEvent(const blink::WebMouseWheelEvent& event); - // Notification the user has performed a direct interaction (mouse down, mouse - // wheel, raw key down, gesture tap, or browser-initiated navigation) while + // Notification the user has performed a direct interaction (mouse down, + // scroll, raw key down, gesture tap, or browser-initiated navigation) while // focus was on the page. Informs the delegate that a user is interacting with - // a site. Only the first mouse wheel event during a scroll will trigger this - // method. + // a site. virtual void OnUserInteraction(RenderWidgetHostImpl* render_widget_host, const blink::WebInputEvent::Type type) {} diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc index 07fb77a..d50b558 100644 --- a/content/browser/renderer_host/render_widget_host_impl.cc +++ b/content/browser/renderer_host/render_widget_host_impl.cc @@ -98,13 +98,6 @@ using blink::WebTextDirection; namespace content { namespace { -// The amount of time after a mouse wheel event is sent to the delegate -// OnUserInteraction method before another mouse wheel event will be sent. This -// interval is used by the Blink EventHandler in its orthogonal heuristic for -// detecting the end of a scroll event (if no event has been seen in 0.1 -// seconds, send an end scroll). -const double kMouseWheelCoalesceIntervalInSeconds = 0.1; - bool g_check_for_pending_resize_ack = true; // <process id, routing id> @@ -218,7 +211,6 @@ RenderWidgetHostImpl::RenderWidgetHostImpl(RenderWidgetHostDelegate* delegate, base::TimeDelta::FromMilliseconds(kHungRendererDelayMs)), new_content_rendering_delay_( base::TimeDelta::FromMilliseconds(kNewContentRenderingDelayMs)), - mouse_wheel_coalesce_timer_(new base::ElapsedTimer()), weak_factory_(this) { CHECK(delegate_); CHECK_NE(MSG_ROUTING_NONE, routing_id_); @@ -1849,21 +1841,11 @@ InputEventAckState RenderWidgetHostImpl::FilterInputEvent( if (!process_->HasConnection()) return INPUT_EVENT_ACK_STATE_UNKNOWN; - if (delegate_) { - if (event.type == WebInputEvent::MouseDown || - event.type == WebInputEvent::GestureTapDown || - event.type == WebInputEvent::RawKeyDown) { - delegate_->OnUserInteraction(this, event.type); - } else if (event.type == WebInputEvent::MouseWheel) { - if (mouse_wheel_coalesce_timer_->Elapsed().InSecondsF() > - kMouseWheelCoalesceIntervalInSeconds) { - // TODO(dominickn): once GestureScrollBegin has landed on all platforms, - // replace this branch and remove. - delegate_->OnUserInteraction(this, event.type); - } - - mouse_wheel_coalesce_timer_.reset(new base::ElapsedTimer()); - } + if (delegate_ && (event.type == WebInputEvent::MouseDown || + event.type == WebInputEvent::GestureScrollBegin || + event.type == WebInputEvent::GestureTapDown || + event.type == WebInputEvent::RawKeyDown)) { + delegate_->OnUserInteraction(this, event.type); } return view_ ? view_->FilterInputEvent(event) diff --git a/content/browser/renderer_host/render_widget_host_impl.h b/content/browser/renderer_host/render_widget_host_impl.h index 567d51e..7a079da 100644 --- a/content/browser/renderer_host/render_widget_host_impl.h +++ b/content/browser/renderer_host/render_widget_host_impl.h @@ -821,12 +821,6 @@ class CONTENT_EXPORT RenderWidgetHostImpl : public RenderWidgetHost, // renderer process before clearing any previously displayed content. base::TimeDelta new_content_rendering_delay_; - // Timer used to batch together mouse wheel events for the delegate - // OnUserInteraction method. A wheel event is only dispatched when a wheel - // event has not been seen for kMouseWheelCoalesceInterval seconds prior. - // TODO(dominickn): remove this when GestureScrollBegin has landed. - scoped_ptr<base::ElapsedTimer> mouse_wheel_coalesce_timer_; - base::WeakPtrFactory<RenderWidgetHostImpl> weak_factory_; DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostImpl); diff --git a/content/public/browser/web_contents_observer.h b/content/public/browser/web_contents_observer.h index 226f309..a3bc77f 100644 --- a/content/public/browser/web_contents_observer.h +++ b/content/public/browser/web_contents_observer.h @@ -327,15 +327,10 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener, // The type argument specifies the kind of interaction. Direct user input // signalled through this callback includes: // 1) any mouse down event (blink::WebInputEvent::MouseDown); - // 2) the start of a mouse wheel scroll (blink::WebInputEvent::MouseWheel); + // 2) the start of a scroll (blink::WebInputEvent::GestureScrollBegin); // 3) any raw key down event (blink::WebInputEvent::RawKeyDown); // 4) any gesture tap event (blink::WebInputEvent::GestureTapDown); and // 5) a browser navigation or reload (blink::WebInputEvent::Undefined). - // The start of a mouse wheel scroll is heuristically detected: a mouse - // wheel event fired at least 0.1 seconds after any other wheel event is - // regarded as the beginning of a scroll. This matches the interval used by - // the Blink EventHandler to detect the end of scrolls. - // TODO(dominickn): replace MouseWheel with GestureScrollBegin. virtual void DidGetUserInteraction(const blink::WebInputEvent::Type type) {} // This method is invoked when a RenderViewHost of this WebContents was diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index d701cba..50a4aef 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -81442,9 +81442,9 @@ To add a new entry, add it with any value and run test to compute valid value. <enum name="SiteEngagementServiceEngagementType" type="int"> <int value="0" label="Navigation"/> <int value="1" label="Keypress"/> - <int value="2" label="Mouse"/> - <int value="3" label="Touch gesture"/> - <int value="4" label="Mouse wheel"/> + <int value="2" label="Mouse down"/> + <int value="3" label="Tap gesture"/> + <int value="4" label="Scroll (mouse wheel or touch)"/> <int value="5" label="Media (foreground tab)"/> <int value="6" label="Media (background tab)"/> <int value="7" label="Webapp shortcut launch"/> |