ScrollBegin/End DCHECKs in RenderWidgetHostImpl are unreliable in the presence of GesturePinch events |
|||||
Issue descriptionThe two DCHECKS in question are: https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_impl.cc?rcl=1a8346a53e0fddbd788c40e849fd2feb731a98bc&l=1289 and https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_impl.cc?rcl=1a8346a53e0fddbd788c40e849fd2feb731a98bc&l=1294 Consider the following sequence of Gesture events, all targeted over top of an OOPIF 1) GestureScrollBegin -> Goes to OOPIF 2) GestureScrollUpdate -> Goes to OOPIF, bubbles ... 3) GesturePinchBegin -> Causes GSB to be sent to root view *before* ack from (2) comes back 4) GesturePinchUpdate 2a) Ack comes back, causes GSB to be sent to root view, but it's already seen one, so DCHECK fires. Solution: Scroll bubbling should not send a GestureScrollBegin/End to a target that has already received one.
,
Oct 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ee201890700ba6dadc73f68a97524e39e7c7129b commit ee201890700ba6dadc73f68a97524e39e7c7129b Author: W. James MacLean <wjmaclean@chromium.org> Date: Fri Oct 26 16:06:24 2018 Disable DCHECKS. These DCHECKs can be triggered during user touchscreen pinch, and are blocking work in https://chromium-review.googlesource.com/c/chromium/src/+/1286437/. Temporarily disable them until we can handle GesturePinch events properly. Bug: 897216 Change-Id: I2a37c468133340a95ec2b3f580a966dd71451482 Reviewed-on: https://chromium-review.googlesource.com/c/1301567 Commit-Queue: James MacLean <wjmaclean@chromium.org> Reviewed-by: Ken Buchanan <kenrb@chromium.org> Cr-Commit-Position: refs/heads/master@{#603105} [modify] https://crrev.com/ee201890700ba6dadc73f68a97524e39e7c7129b/content/browser/renderer_host/render_widget_host_impl.cc
,
Nov 1
,
Nov 1
Comment 4 deleted due to wrong duplication
,
Nov 1
,
Nov 15
***UI Mass Triage*** Since work is in progress as per C#2, adding appropriate labels.
,
Dec 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b9e5ed2a6f6cacce39c19a36bb76a940244def99 commit b9e5ed2a6f6cacce39c19a36bb76a940244def99 Author: Kevin McNee <mcnee@chromium.org> Date: Mon Dec 10 18:41:30 2018 Refactor OOPIF scroll bubbling state and cancellation Whether or not a child is bubbling scroll was being tracked in both CrossProcessFrameConnector and RenderWidgetHostViewChildFrame. It's simpler to just have it in RWHVCF. There are several cases where we generate fake scroll updates when cancelling scroll bubbling. This is a relic from the non-scroll latching code paths and has been removed. We also deduplicate the scroll bubbling cancellation code. We were tracking the view from which scroll events are bubbled by keeping a reference to that view's parent. We now just keep a reference to the originating view directly. We also now properly cancel scroll bubbling when a child detaches. Bug: 828422, 897216 Change-Id: Iee983bd9ea05b324d556c66320a1bc5e544de057 Reviewed-on: https://chromium-review.googlesource.com/c/1357563 Reviewed-by: Ken Buchanan <kenrb@chromium.org> Commit-Queue: Kevin McNee <mcnee@chromium.org> Cr-Commit-Position: refs/heads/master@{#615184} [modify] https://crrev.com/b9e5ed2a6f6cacce39c19a36bb76a940244def99/content/browser/frame_host/cross_process_frame_connector.cc [modify] https://crrev.com/b9e5ed2a6f6cacce39c19a36bb76a940244def99/content/browser/frame_host/cross_process_frame_connector.h [modify] https://crrev.com/b9e5ed2a6f6cacce39c19a36bb76a940244def99/content/browser/renderer_host/render_widget_host_input_event_router.cc [modify] https://crrev.com/b9e5ed2a6f6cacce39c19a36bb76a940244def99/content/browser/renderer_host/render_widget_host_input_event_router.h [modify] https://crrev.com/b9e5ed2a6f6cacce39c19a36bb76a940244def99/content/browser/renderer_host/render_widget_host_view_child_frame.cc
,
Dec 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c4c1ad0a13df3812ff670821e4c7325f181e192b commit c4c1ad0a13df3812ff670821e4c7325f181e192b Author: Samuel Huang <huangs@chromium.org> Date: Mon Dec 10 19:53:45 2018 Revert "Refactor OOPIF scroll bubbling state and cancellation" This reverts commit b9e5ed2a6f6cacce39c19a36bb76a940244def99. Reason for revert: Suspected of webkit_layout_tests failure for virtual/user-activation-v2/fast/events/middleClickAutoscroll-event-fired.html in "WebKit Linux Trusty Leak". Original change's description: > Refactor OOPIF scroll bubbling state and cancellation > > Whether or not a child is bubbling scroll was being tracked in both > CrossProcessFrameConnector and RenderWidgetHostViewChildFrame. It's > simpler to just have it in RWHVCF. > > There are several cases where we generate fake scroll updates when > cancelling scroll bubbling. This is a relic from the non-scroll latching > code paths and has been removed. We also deduplicate the scroll bubbling > cancellation code. > > We were tracking the view from which scroll events are bubbled by keeping > a reference to that view's parent. We now just keep a reference to the > originating view directly. > > We also now properly cancel scroll bubbling when a child detaches. > > Bug: 828422, 897216 > Change-Id: Iee983bd9ea05b324d556c66320a1bc5e544de057 > Reviewed-on: https://chromium-review.googlesource.com/c/1357563 > Reviewed-by: Ken Buchanan <kenrb@chromium.org> > Commit-Queue: Kevin McNee <mcnee@chromium.org> > Cr-Commit-Position: refs/heads/master@{#615184} TBR=kenrb@chromium.org,mcnee@chromium.org Change-Id: I7ac9817339a80408d3cf1849efef3c42be1973d6 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 828422, 897216 Reviewed-on: https://chromium-review.googlesource.com/c/1370466 Reviewed-by: Samuel Huang <huangs@chromium.org> Commit-Queue: Samuel Huang <huangs@chromium.org> Cr-Commit-Position: refs/heads/master@{#615205} [modify] https://crrev.com/c4c1ad0a13df3812ff670821e4c7325f181e192b/content/browser/frame_host/cross_process_frame_connector.cc [modify] https://crrev.com/c4c1ad0a13df3812ff670821e4c7325f181e192b/content/browser/frame_host/cross_process_frame_connector.h [modify] https://crrev.com/c4c1ad0a13df3812ff670821e4c7325f181e192b/content/browser/renderer_host/render_widget_host_input_event_router.cc [modify] https://crrev.com/c4c1ad0a13df3812ff670821e4c7325f181e192b/content/browser/renderer_host/render_widget_host_input_event_router.h [modify] https://crrev.com/c4c1ad0a13df3812ff670821e4c7325f181e192b/content/browser/renderer_host/render_widget_host_view_child_frame.cc
,
Dec 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e90c3a033b0743b58d3d790662b2788476dac5de commit e90c3a033b0743b58d3d790662b2788476dac5de Author: Kevin McNee <mcnee@chromium.org> Date: Tue Dec 11 16:58:08 2018 Reland "Refactor OOPIF scroll bubbling state and cancellation" This is a reland of b9e5ed2a6f6cacce39c19a36bb76a940244def99 Original change's description: > Refactor OOPIF scroll bubbling state and cancellation > > Whether or not a child is bubbling scroll was being tracked in both > CrossProcessFrameConnector and RenderWidgetHostViewChildFrame. It's > simpler to just have it in RWHVCF. > > There are several cases where we generate fake scroll updates when > cancelling scroll bubbling. This is a relic from the non-scroll latching > code paths and has been removed. We also deduplicate the scroll bubbling > cancellation code. > > We were tracking the view from which scroll events are bubbled by keeping > a reference to that view's parent. We now just keep a reference to the > originating view directly. > > We also now properly cancel scroll bubbling when a child detaches. > > Bug: 828422, 897216 > Change-Id: Iee983bd9ea05b324d556c66320a1bc5e544de057 > Reviewed-on: https://chromium-review.googlesource.com/c/1357563 > Reviewed-by: Ken Buchanan <kenrb@chromium.org> > Commit-Queue: Kevin McNee <mcnee@chromium.org> > Cr-Commit-Position: refs/heads/master@{#615184} Tbr: kenrb@chromium.org Bug: 828422, 897216 Change-Id: If529545300f6bbb7a7056cdbebbd8317cb274f7a Reviewed-on: https://chromium-review.googlesource.com/c/1371010 Reviewed-by: Kevin McNee <mcnee@chromium.org> Commit-Queue: Kevin McNee <mcnee@chromium.org> Cr-Commit-Position: refs/heads/master@{#615557} [modify] https://crrev.com/e90c3a033b0743b58d3d790662b2788476dac5de/content/browser/frame_host/cross_process_frame_connector.cc [modify] https://crrev.com/e90c3a033b0743b58d3d790662b2788476dac5de/content/browser/frame_host/cross_process_frame_connector.h [modify] https://crrev.com/e90c3a033b0743b58d3d790662b2788476dac5de/content/browser/renderer_host/render_widget_host_input_event_router.cc [modify] https://crrev.com/e90c3a033b0743b58d3d790662b2788476dac5de/content/browser/renderer_host/render_widget_host_input_event_router.h [modify] https://crrev.com/e90c3a033b0743b58d3d790662b2788476dac5de/content/browser/renderer_host/render_widget_host_view_child_frame.cc
,
Jan 11
This issue has an owner, a component and a priority, but is still listed as untriaged or unconfirmed. By definition, this bug is triaged. Changing status to "assigned". Please reach out to me if you disagree with how I've done this. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by wjmaclean@chromium.org
, Oct 19