Calls to ResourceDispatchHost::OnUserGesture may be filtering on the wrong signals |
|||||
Issue descriptionToday, WebContentsImpl::OnUserInteraction() filters for kMouseWheel before calling RDH::OnUserGesture(). However, RenderWidgetHostImpl::FilterInputEvent() won't even call OnUserInteraction() if the type is kMouseWheel (since it checks for a number of event types, none of which are kMouseWheel). However however, mouse wheel events *also* generate gesture scroll begin input events... so maybe we need to be filtering those out? *brain explodes from complexity of input events*
,
Apr 1 2018
Buenas noches no entiendo lo único que se es que no puedo entrar más en Chrome que es mi navegador y les pregunto cual es el problema yo no quiero cambiar notificaciones a cambio de mi correo electrónico lo único por favor quiero solución si son tan amable. Desde ya muy agradecida.
,
Apr 3 2018
The intention of the if was to preserve existing functionality. The old OnUserGesture() did not fire on scroll events, but UserInteraction did. At the time, the kGestureScrollBegin event had not been launched, so there was a timer on the mouse wheel scroll event to only send the first one (see https://codereview.chromium.org/1735833002). And hence the if statement to filter out the mouse wheel event Now that we have the GestureScrollBegin event, this if statement should probably say GestureScrollBegin, not MouseWheel. Not sure if this has caused unintended bugs along the way.
,
Jul 3
dcheng: did you fix this in https://chromium-review.googlesource.com/1028484?
,
Jul 3
No, I didn't fix this (and I didn't make the original change, so it's hard for me to judge what the intent is).
,
Jul 4
Hmmm, D'oh, I can't read code properly. :) I've sent a CL to fix this.
,
Jul 4
,
Jul 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/37a15052f10515874939931d9ed725e94bc94ee7 commit 37a15052f10515874939931d9ed725e94bc94ee7 Author: Dominick Ng <dominickn@chromium.org> Date: Tue Jul 17 23:56:35 2018 Remove browser-initiated navigations from WebContentsObserver::DidGetUserInteraction. Some types of user interactions are used as a signal to ResourceDispatcherHostImpl. Prior to https://crrev.com/1735833002, scrolling was not one of them, and part of that CL's work explicitly filtered out MouseWheel events from being passed to ResourceDispatcherHostImpl. It also changed user-initiated navigations to call the new DidGetUserInteraction method with the blink::WebInputEvent::Undefined type. However, only a limited subset of user-initiated navigations were passed through this method: Ctrl-R/refresh button reloads, omnibox navigations, and NTP/bookmark navigations. Other user-initiated navigations such as back/forward buttons were omitted. https://crrev.com/1776843003 introduced wheel gesture events that generalised touch and mouse scrolling into one concept. https://crrev.com/1748553002 updated the user interaction code to use the new GestureScrollBegin event in place of MouseWheel events, but did not update the type check for forwarding to ResourceDispatcherHostImpl. https://crrev.com/1964863002 introduced NavigationHandle::IsRendererInitiated, which returns true if a navigation was initiated by the renderer process, and false if it was initiated by the browser process (i.e. it was user-initiated). This is a broader scope that the existing user-initiated navigations signaled through DidGetUserInteraction, namely including back/forward navigations. This CL improves on the user gesture handling in WebContentsImpl by: * removing user-initiated navigations from DidGetUserInteraction, and updating clients to listen to Did{Start/Finish}Navigation and check NavigationHandle::IsRendererInitiated. This includes more types of user-initiated navigations. * restoring the user gesture signal to ResourceDispatcherHostImpl back to how it was prior to https://crrev.com/1748553002 - listening to keydown, mouse click, taps, and certain user-initiated navigations. BUG= 827659 Change-Id: I639270b21446104f22c8ae9b8b939eb96af3cabd Reviewed-on: https://chromium-review.googlesource.com/1125703 Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Min Qin <qinmin@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Jialiu Lin <jialiul@chromium.org> Reviewed-by: Mustafa Emre Acer <meacer@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Dominick Ng <dominickn@chromium.org> Cr-Commit-Position: refs/heads/master@{#575857} [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/chrome/browser/chrome_site_per_process_browsertest.cc [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/chrome/browser/download/download_request_limiter.cc [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/chrome/browser/download/download_request_limiter.h [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/chrome/browser/download/download_request_limiter_unittest.cc [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/chrome/browser/engagement/site_engagement_helper.cc [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/chrome/browser/metrics/tab_stats_tracker.cc [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/chrome/browser/metrics/tab_stats_tracker.h [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/chrome/browser/safe_browsing/safe_browsing_navigation_observer.cc [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/chrome/browser/safe_browsing/safe_browsing_navigation_observer.h [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/chrome/browser/ui/app_list/search/answer_card/answer_card_web_contents.cc [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/chrome/browser/ui/app_list/search/answer_card/answer_card_web_contents.h [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/chrome/browser/ui/blocked_content/popup_opener_tab_helper.cc [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/chrome/browser/ui/blocked_content/popup_opener_tab_helper.h [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/chrome/browser/ui/browser_browsertest.cc [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/content/browser/web_contents/web_contents_impl.cc [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/content/browser/web_contents/web_contents_impl.h [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/content/public/browser/navigation_handle.h [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/content/public/browser/web_contents.h [modify] https://crrev.com/37a15052f10515874939931d9ed725e94bc94ee7/content/public/browser/web_contents_observer.h
,
Jul 18
,
Jul 19
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by dcheng@chromium.org
, Mar 30 2018