Description was changed from ========== Async Wheel Event with only the first one cancellable behind ...
3 years, 6 months ago
(2017-06-16 16:56:42 UTC)
#3
Description was changed from
==========
Async Wheel Event with only the first one cancellable behind a flag.
To test it locally use --enable-features=AsyncWheelEvents runtime flag.
BUG=706175
==========
to
==========
Async Wheel Event with only the first one cancellable behind a flag.
To test it locally use --enable-features=AsyncWheelEvents runtime flag.
BUG=706175
TEST=MouseWheelEventQueueTests/*/2,
LegacyInputRouterImplAsyncWheelEventEnabledTest/*,
RenderWidgetHostAsyncWheelEventsEnabledTest
==========
3 years, 6 months ago
(2017-06-16 17:32:19 UTC)
#5
dtapuska
https://codereview.chromium.org/2943133002/diff/20001/content/browser/renderer_host/input/mouse_wheel_event_queue.cc File content/browser/renderer_host/input/mouse_wheel_event_queue.cc (right): https://codereview.chromium.org/2943133002/diff/20001/content/browser/renderer_host/input/mouse_wheel_event_queue.cc#newcode274 content/browser/renderer_host/input/mouse_wheel_event_queue.cc:274: if (base::FeatureList::IsEnabled(features::kAsyncWheelEvents)) { don't check the feature list on ...
3 years, 6 months ago
(2017-06-16 17:36:09 UTC)
#6
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linux/builds/392848)
3 years, 6 months ago
(2017-06-20 16:03:11 UTC)
#13
https://codereview.chromium.org/2943133002/diff/80001/content/browser/renderer_host/input/legacy_input_router_impl_unittest.cc File content/browser/renderer_host/input/legacy_input_router_impl_unittest.cc (right): https://codereview.chromium.org/2943133002/diff/80001/content/browser/renderer_host/input/legacy_input_router_impl_unittest.cc#newcode1073 content/browser/renderer_host/input/legacy_input_router_impl_unittest.cc:1073: void LegacyInputRouterImplTest::UnhandledWheelEvent() { Moving code around like this makes ...
3 years, 6 months ago
(2017-06-20 20:53:15 UTC)
#19
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/454449)
3 years, 6 months ago
(2017-06-21 16:04:32 UTC)
#23
3 years, 6 months ago
(2017-06-21 20:31:51 UTC)
#29
On 2017/06/21 18:09:59, sahel wrote:
>
https://codereview.chromium.org/2943133002/diff/80001/content/browser/rendere...
> File content/browser/renderer_host/input/legacy_input_router_impl_unittest.cc
> (right):
>
>
https://codereview.chromium.org/2943133002/diff/80001/content/browser/rendere...
> content/browser/renderer_host/input/legacy_input_router_impl_unittest.cc:1073:
> void LegacyInputRouterImplTest::UnhandledWheelEvent() {
> On 2017/06/20 20:53:14, dtapuska wrote:
> > Moving code around like this makes it *really* hard to review. I've looked
at
> > the individual lines changed and it is fine. Just something useful for
reviews
> > in the future.
>
> I am sorry for the inconvenience, so I should apply the changes in place in
one
> patch, and only move the function in a separate patch?
>
>
https://codereview.chromium.org/2943133002/diff/80001/content/browser/rendere...
> File content/browser/renderer_host/input/mouse_wheel_event_queue.h (right):
>
>
https://codereview.chromium.org/2943133002/diff/80001/content/browser/rendere...
> content/browser/renderer_host/input/mouse_wheel_event_queue.h:96: bool
> enable_async_wheel_events_;
> On 2017/06/20 20:53:14, dtapuska wrote:
> > Could probably use some comments. Looking at the names they seem very
similar.
> I
> > understand what they do; one is a feature flag. The other is the current
> state.
> > But that should be in the code.
>
> Done.
>
>
https://codereview.chromium.org/2943133002/diff/80001/content/browser/rendere...
> File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
> (right):
>
>
https://codereview.chromium.org/2943133002/diff/80001/content/browser/rendere...
> content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:1161:
> expected_message_count = enqueued_wheel_event_count + 2;
> On 2017/06/20 20:53:14, dtapuska wrote:
> > Can we add some more comments here? These seem magical.
>
> Done.
>
>
https://codereview.chromium.org/2943133002/diff/80001/ui/events/blink/input_h...
> File ui/events/blink/input_handler_proxy.cc (right):
>
>
https://codereview.chromium.org/2943133002/diff/80001/ui/events/blink/input_h...
> ui/events/blink/input_handler_proxy.cc:684: return result;
> On 2017/06/20 20:53:14, dtapuska wrote:
> > So should we really be sending back undefined? ie; in the release version.
> What
> > happens?
> >
> > The touch path will force a hit test if it is undefined. Do we want to send
> this
> > to main?
> >
>
> Done. I changed it to follow the blocking path if the result is
> undefined. I can return DID_NOT_HANDLE directly if you prefer it.
lgtm
CQ is committing da patch. Bot data: {"patchset_id": 130001, "attempt_start_ts": 1498490788237520, "parent_rev": "fb7942c7ff5bb9b0282483a5159c86537a2cfeb9", "commit_rev": "44851965d0862ce66a1cad576686125ef8864d25"}
3 years, 5 months ago
(2017-06-26 16:34:51 UTC)
#41
CQ is committing da patch.
Bot data: {"patchset_id": 130001, "attempt_start_ts": 1498490788237520,
"parent_rev": "fb7942c7ff5bb9b0282483a5159c86537a2cfeb9", "commit_rev":
"44851965d0862ce66a1cad576686125ef8864d25"}
commit-bot: I haz the power
Description was changed from ========== Async Wheel Event with only the first one cancellable behind ...
3 years, 5 months ago
(2017-06-26 16:35:03 UTC)
#42
Message was sent while issue was closed.
Description was changed from
==========
Async Wheel Event with only the first one cancellable behind a flag.
To test it locally use --enable-features=AsyncWheelEvents runtime flag.
BUG=706175
TEST=MouseWheelEventQueueTests/*/2,
LegacyInputRouterImplAsyncWheelEventEnabledTest/*,
RenderWidgetHostAsyncWheelEventsEnabledTest
==========
to
==========
Async Wheel Event with only the first one cancellable behind a flag.
To test it locally use --enable-features=AsyncWheelEvents runtime flag.
BUG=706175
TEST=MouseWheelEventQueueTests/*/2,
LegacyInputRouterImplAsyncWheelEventEnabledTest/*,
RenderWidgetHostAsyncWheelEventsEnabledTest
Review-Url: https://codereview.chromium.org/2943133002
Cr-Commit-Position: refs/heads/master@{#482289}
Committed:
https://chromium.googlesource.com/chromium/src/+/44851965d0862ce66a1cad576686...
==========
commit-bot: I haz the power
Committed patchset #6 (id:130001) as https://chromium.googlesource.com/chromium/src/+/44851965d0862ce66a1cad576686125ef8864d25
3 years, 5 months ago
(2017-06-26 16:35:05 UTC)
#43
Issue 2943133002: Async Wheel Event with only the first one cancellable behind a flag.
(Closed)
Created 3 years, 6 months ago by sahel
Modified 3 years, 5 months ago
Reviewers: dtapuska, kinuko
Base URL:
Comments: 14