Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2830183003:
Re-enable ScrollBubblingFromOOPIFTest on CrOS. (Closed)
DescriptionRe-enable ScrollBubblingFromOOPIFTest on CrOS.
This CL re-enables this test on CrOS by fixing a timing issue.
BUG=627238
Review-Url: https://codereview.chromium.org/2830183003
Cr-Commit-Position: refs/heads/master@{#466718}
Committed: https://chromium.googlesource.com/chromium/src/+/6076ea0e5caca2693cf84db598b1f33e938720d4
Patch Set 1 #Patch Set 2 : Synchronize with an InputEventObserver instead of delay. #
Depends on Patchset: Messages
Total messages: 21 (14 generated)
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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Re-enable ScrollBubblingFromOOPIFTest on CrOS. This CL re-enables this test on CrOS by fixing a timing issue. BUG=627238 ========== to ========== Re-enable ScrollBubblingFromOOPIFTest on CrOS. This CL re-enables this test on CrOS by fixing a timing issue. BUG=627238 ==========
wjmaclean@chromium.org changed reviewers: + creis@chromium.org, kenrb@chromium.org
I'd like to go ahead and land this, while trying to come up with an improved mechanism for preventing the collision in the first place. Please take a look?
On 2017/04/24 13:49:30, wjmaclean wrote: > I'd like to go ahead and land this, while trying to come up with an improved > mechanism for preventing the collision in the first place. > > Please take a look? lgtm But given that somebody is actively working on scroll latching now, does it make sense to limit how much time we spend on this? OOPIF scroll bubbling is going to get refactored/simplified, which should fix problems with GSE confusion. We just need a basically working state, in the mean time.
On 2017/04/24 15:48:30, kenrb wrote: > On 2017/04/24 13:49:30, wjmaclean wrote: > > I'd like to go ahead and land this, while trying to come up with an improved > > mechanism for preventing the collision in the first place. > > > > Please take a look? > > lgtm > > But given that somebody is actively working on scroll latching now, does it make > sense to limit how much time we spend on this? > > OOPIF scroll bubbling is going to get refactored/simplified, which should fix > problems with GSE confusion. We just need a basically working state, in the mean > time. I suspect that scroll wheel bubbling with latching will take a while to implement. On the other hand, we could just kill the DCHECKs and it wouldn't be an issue. But it is a pain for Chrome devs when one hits the DCHECKs (see comments/bug on RWHI::ForwardGestureEventWithLatencyInfo() in the GestureScrollEnd case ...).
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...
I've revised the CL to synchronise off of InputEventObserver in RenderWidgetHost ... I think this will be better. PTAL
LGTM
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
The patchset sent to the CQ was uploaded after l-g-t-m from kenrb@chromium.org Link to the patchset: https://codereview.chromium.org/2830183003/#ps20001 (title: "Synchronize with an InputEventObserver instead of delay.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493062258893710,
"parent_rev": "b3fceaa3468034cdf36d33021b8b48d9f2e448ec", "commit_rev":
"6076ea0e5caca2693cf84db598b1f33e938720d4"}
Message was sent while issue was closed.
Description was changed from ========== Re-enable ScrollBubblingFromOOPIFTest on CrOS. This CL re-enables this test on CrOS by fixing a timing issue. BUG=627238 ========== to ========== Re-enable ScrollBubblingFromOOPIFTest on CrOS. This CL re-enables this test on CrOS by fixing a timing issue. BUG=627238 Review-Url: https://codereview.chromium.org/2830183003 Cr-Commit-Position: refs/heads/master@{#466718} Committed: https://chromium.googlesource.com/chromium/src/+/6076ea0e5caca2693cf84db598b1... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/6076ea0e5caca2693cf84db598b1... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
