Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2164673002:
Queued wheel events should not use ScopedInputScaleDisabler. (Closed)
|
Created:
4 years, 5 months ago by wjmaclean Modified:
4 years, 5 months ago CC:
chromium-reviews, darin-cc_chromium.org, nasko+codewatch_chromium.org, jam, creis+watch_chromium.org, gkihumba Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionQueued wheel events should not use ScopedInputScaleDisabler.
Since WebMouseWheelEvents returned by BrowserPlugin are sent to the
guest's RenderWidgetHostImpl, which queues them and (may) send them
later, they should have the device_scale_factor explicitly removed, as
RenderWidgetHostImpl/InputRouterImpl may re-add it before sending.
BUG=601875
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
Committed: https://crrev.com/288a99fe15afb1dfefbe253aaa3a5de3c249d866
Cr-Commit-Position: refs/heads/master@{#406417}
Patch Set 1 #Patch Set 2 : Rescale all necessary attributes. #
Total comments: 1
Patch Set 3 : Only opt out of ScopedInputScaleDisabler when IsUseZoomForDSFEnabled(). #Messages
Total messages: 23 (14 generated)
Description was changed from ========== Queued wheel events should not use ScopedInputScaleDisabler. Since WebMouseWheelEvents returned by BrowserPlugin are sent to the guest's RenderWidgetHostImpl, which queues them and (may) send them later, they should have the device_scale_factor explicitly removed, as RenderWidgetHostImpl/InputRouterImpl may re-add it before sending. BUG=601875 ========== to ========== Queued wheel events should not use ScopedInputScaleDisabler. Since WebMouseWheelEvents returned by BrowserPlugin are sent to the guest's RenderWidgetHostImpl, which queues them and (may) send them later, they should have the device_scale_factor explicitly removed, as RenderWidgetHostImpl/InputRouterImpl may re-add it before sending. BUG=601875 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ==========
The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run
Description was changed from ========== Queued wheel events should not use ScopedInputScaleDisabler. Since WebMouseWheelEvents returned by BrowserPlugin are sent to the guest's RenderWidgetHostImpl, which queues them and (may) send them later, they should have the device_scale_factor explicitly removed, as RenderWidgetHostImpl/InputRouterImpl may re-add it before sending. BUG=601875 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ========== to ========== Queued wheel events should not use ScopedInputScaleDisabler. Since WebMouseWheelEvents returned by BrowserPlugin are sent to the guest's RenderWidgetHostImpl, which queues them and (may) send them later, they should have the device_scale_factor explicitly removed, as RenderWidgetHostImpl/InputRouterImpl may re-add it before sending. BUG=601875 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ==========
wjmaclean@chromium.org changed reviewers: + oshima@chromium.org
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
wjmaclean@chromium.org changed reviewers: + nasko@chromium.org
oshima@ Can you look at this and let me know if it looks ok? nasko@ - This is needed for merging to a M52 release candidate being cut tomorrow, so can you provide OWNERS if oshima@ thinks it's ok?
Rubberstamp LGTM, assuming oshima is ok with the CL.
https://codereview.chromium.org/2164673002/diff/20001/content/browser/frame_h... File content/browser/frame_host/render_widget_host_view_guest.cc (right): https://codereview.chromium.org/2164673002/diff/20001/content/browser/frame_h... content/browser/frame_host/render_widget_host_view_guest.cc:562: if (event->type == blink::WebInputEvent::MouseWheel) { you also need to check "IsZoomForDSFEnabled()". Alternative way is to scale when queued, and then skip scaling for wheel event in OfferToRenderer, and I have a slight preference over this, although I can be convinced the other way. WDYT?
On 2016/07/19 21:40:48, oshima wrote: > https://codereview.chromium.org/2164673002/diff/20001/content/browser/frame_h... > File content/browser/frame_host/render_widget_host_view_guest.cc (right): > > https://codereview.chromium.org/2164673002/diff/20001/content/browser/frame_h... > content/browser/frame_host/render_widget_host_view_guest.cc:562: if (event->type > == blink::WebInputEvent::MouseWheel) { > you also need to check "IsZoomForDSFEnabled()". > > Alternative way is to scale when queued, and then skip scaling for wheel event > in OfferToRenderer, and I have a slight preference over this, although I can be > convinced the other way. WDYT? Since this is a problem that only seems to affect events forwarded from BrowserPlugin, and since BrowserPlugin will be going away in the next few quarters (along with RenderWidgetHostViewGuest), I was hoping not to put code into other places. Also, I'm hoping for a small CL that will merge easily.
lgtm
The CQ bit was checked by wjmaclean@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nasko@chromium.org Link to the patchset: https://codereview.chromium.org/2164673002/#ps40001 (title: "Only opt out of ScopedInputScaleDisabler when IsUseZoomForDSFEnabled().")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Queued wheel events should not use ScopedInputScaleDisabler. Since WebMouseWheelEvents returned by BrowserPlugin are sent to the guest's RenderWidgetHostImpl, which queues them and (may) send them later, they should have the device_scale_factor explicitly removed, as RenderWidgetHostImpl/InputRouterImpl may re-add it before sending. BUG=601875 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ========== to ========== Queued wheel events should not use ScopedInputScaleDisabler. Since WebMouseWheelEvents returned by BrowserPlugin are sent to the guest's RenderWidgetHostImpl, which queues them and (may) send them later, they should have the device_scale_factor explicitly removed, as RenderWidgetHostImpl/InputRouterImpl may re-add it before sending. BUG=601875 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== Queued wheel events should not use ScopedInputScaleDisabler. Since WebMouseWheelEvents returned by BrowserPlugin are sent to the guest's RenderWidgetHostImpl, which queues them and (may) send them later, they should have the device_scale_factor explicitly removed, as RenderWidgetHostImpl/InputRouterImpl may re-add it before sending. BUG=601875 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ========== to ========== Queued wheel events should not use ScopedInputScaleDisabler. Since WebMouseWheelEvents returned by BrowserPlugin are sent to the guest's RenderWidgetHostImpl, which queues them and (may) send them later, they should have the device_scale_factor explicitly removed, as RenderWidgetHostImpl/InputRouterImpl may re-add it before sending. BUG=601875 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Committed: https://crrev.com/288a99fe15afb1dfefbe253aaa3a5de3c249d866 Cr-Commit-Position: refs/heads/master@{#406417} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/288a99fe15afb1dfefbe253aaa3a5de3c249d866 Cr-Commit-Position: refs/heads/master@{#406417} |
