Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2772443006:
Set trusted flag of coalesced events at creation (Closed)
|
Created:
3 years, 9 months ago by Navid Zolghadr Modified:
3 years, 8 months ago CC:
chromium-reviews, blink-reviews, chromium-apps-reviews_chromium.org, blink-reviews-w3ctests_chromium.org, extensions-reviews_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionSet trusted flag of coalesced events at creation
Set trusted, bubbles, and cancelable flags of
the coalesced events to true at the creation time.
Note that we didn't set it along with the original
event to avoid unnecessary traverse of all the
coalesced events for redispatching.
This CL also adds a wpt test and its automation for
coalesced events.
Note that the test doesn't quite pass as we don't
run them with compositor thread enabled.
BUG=704219
Review-Url: https://codereview.chromium.org/2772443006
Cr-Commit-Position: refs/heads/master@{#461155}
Committed: https://chromium.googlesource.com/chromium/src/+/779888ee1eba464ede92e539999ebb772afaa0ae
Patch Set 1 #
Total comments: 6
Patch Set 2 : Add comment and fix test timeout #
Total comments: 4
Patch Set 3 : Update the test to include pointercancel #Patch Set 4 : Skip the touch test on Mac #
Messages
Total messages: 36 (23 generated)
Description was changed from ========== Set trusted flag of coalesced events at creation Set trusted flag of coalesced events to true at the creation time. Note that we didn't set it along with the original event to avoid unnecessary traverse of all the coalesced events for redispatching. This CL also adds a wpt test and its automation for coalesced events. Note that the test doesn't quite pass as we don't run them with compositor thread enabled. BUG=704219 ========== to ========== Set trusted flag of coalesced events at creation Set trusted, bubbles, and cancelable flags of the coalesced events to true at the creation time. Note that we didn't set it along with the original event to avoid unnecessary traverse of all the coalesced events for redispatching. This CL also adds a wpt test and its automation for coalesced events. Note that the test doesn't quite pass as we don't run them with compositor thread enabled. BUG=704219 ==========
The CQ bit was checked by nzolghadr@chromium.org to run a CQ dry run
nzolghadr@chromium.org changed reviewers: + dtapuska@chromium.org
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2772443006/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html (right): https://codereview.chromium.org/2772443006/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html:56: setTimeout(function(){ why don't we check the date and make this more predictable? Like it will block for x number of milliseconds? https://codereview.chromium.org/2772443006/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/events/PointerEventFactory.cpp (right): https://codereview.chromium.org/2772443006/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/events/PointerEventFactory.cpp:288: event->setTrusted(true); likely this deserves a comment https://codereview.chromium.org/2772443006/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/events/PointerEventFactory.cpp:342: event->setTrusted(true); ditto
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by nzolghadr@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...
ptal https://codereview.chromium.org/2772443006/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html (right): https://codereview.chromium.org/2772443006/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html:56: setTimeout(function(){ On 2017/03/24 21:14:02, dtapuska wrote: > why don't we check the date and make this more predictable? Like it will block > for x number of milliseconds? Done. https://codereview.chromium.org/2772443006/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/events/PointerEventFactory.cpp (right): https://codereview.chromium.org/2772443006/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/events/PointerEventFactory.cpp:288: event->setTrusted(true); On 2017/03/24 21:14:02, dtapuska wrote: > likely this deserves a comment Done. https://codereview.chromium.org/2772443006/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/events/PointerEventFactory.cpp:342: event->setTrusted(true); On 2017/03/24 21:14:02, dtapuska wrote: > ditto Done.
nzolghadr@chromium.org changed reviewers: + mustaq@chromium.org
https://codereview.chromium.org/2772443006/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html (right): https://codereview.chromium.org/2772443006/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html:18: WaitingForUp: 4, Can we easily cover the pointercancel event too? We need to change this to |WaitingForUpOrCancel|, then repeat the instruction twice, and change the touch-action to auto after the first sequence. https://codereview.chromium.org/2772443006/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html:34: var test_pointerEvent = setup_pointerevent_test("pointerevent boundary events in capturing", ['mouse']); Please remove all text referring to "capturing".
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by nzolghadr@chromium.org to run a CQ dry run
ptal https://codereview.chromium.org/2772443006/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html (right): https://codereview.chromium.org/2772443006/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html:18: WaitingForUp: 4, On 2017/03/27 16:18:13, mustaq wrote: > Can we easily cover the pointercancel event too? We need to change this to > |WaitingForUpOrCancel|, then repeat the instruction twice, and change the > touch-action to auto after the first sequence. Done. https://codereview.chromium.org/2772443006/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html:34: var test_pointerEvent = setup_pointerevent_test("pointerevent boundary events in capturing", ['mouse']); On 2017/03/27 16:18:13, mustaq wrote: > Please remove all text referring to "capturing". Done.
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: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by nzolghadr@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.
On 2017/03/28 16:16:39, Navid Zolghadr wrote: > ptal > > https://codereview.chromium.org/2772443006/diff/20001/third_party/WebKit/Layo... > File > third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html > (right): > > https://codereview.chromium.org/2772443006/diff/20001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html:18: > WaitingForUp: 4, > On 2017/03/27 16:18:13, mustaq wrote: > > Can we easily cover the pointercancel event too? We need to change this to > > |WaitingForUpOrCancel|, then repeat the instruction twice, and change the > > touch-action to auto after the first sequence. > > Done. > > https://codereview.chromium.org/2772443006/diff/20001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html:34: > var test_pointerEvent = setup_pointerevent_test("pointerevent boundary events in > capturing", ['mouse']); > On 2017/03/27 16:18:13, mustaq wrote: > > Please remove all text referring to "capturing". > > Done. ping
On 2017/03/29 20:30:14, Navid Zolghadr wrote: > On 2017/03/28 16:16:39, Navid Zolghadr wrote: > > ptal > > > > > https://codereview.chromium.org/2772443006/diff/20001/third_party/WebKit/Layo... > > File > > > third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html > > (right): > > > > > https://codereview.chromium.org/2772443006/diff/20001/third_party/WebKit/Layo... > > > third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html:18: > > WaitingForUp: 4, > > On 2017/03/27 16:18:13, mustaq wrote: > > > Can we easily cover the pointercancel event too? We need to change this to > > > |WaitingForUpOrCancel|, then repeat the instruction twice, and change the > > > touch-action to auto after the first sequence. > > > > Done. > > > > > https://codereview.chromium.org/2772443006/diff/20001/third_party/WebKit/Layo... > > > third_party/WebKit/LayoutTests/external/wpt/pointerevents/extension/pointerevent_coalesced_events_attributes-manual.html:34: > > var test_pointerEvent = setup_pointerevent_test("pointerevent boundary events > in > > capturing", ['mouse']); > > On 2017/03/27 16:18:13, mustaq wrote: > > > Please remove all text referring to "capturing". > > > > Done. > > ping lgtm
nzolghadr@chromium.org changed reviewers: + bokan@chromium.org
rs lgtm
The CQ bit was checked by nzolghadr@chromium.org
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": 60001, "attempt_start_ts": 1490973886087340,
"parent_rev": "3124c82176196788d80554802b7320890f54eecd", "commit_rev":
"779888ee1eba464ede92e539999ebb772afaa0ae"}
Message was sent while issue was closed.
Description was changed from ========== Set trusted flag of coalesced events at creation Set trusted, bubbles, and cancelable flags of the coalesced events to true at the creation time. Note that we didn't set it along with the original event to avoid unnecessary traverse of all the coalesced events for redispatching. This CL also adds a wpt test and its automation for coalesced events. Note that the test doesn't quite pass as we don't run them with compositor thread enabled. BUG=704219 ========== to ========== Set trusted flag of coalesced events at creation Set trusted, bubbles, and cancelable flags of the coalesced events to true at the creation time. Note that we didn't set it along with the original event to avoid unnecessary traverse of all the coalesced events for redispatching. This CL also adds a wpt test and its automation for coalesced events. Note that the test doesn't quite pass as we don't run them with compositor thread enabled. BUG=704219 Review-Url: https://codereview.chromium.org/2772443006 Cr-Commit-Position: refs/heads/master@{#461155} Committed: https://chromium.googlesource.com/chromium/src/+/779888ee1eba464ede92e539999e... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/779888ee1eba464ede92e539999e...
Message was sent while issue was closed.
On 2017/03/31 at 17:10:53, commit-bot wrote: > Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/779888ee1eba464ede92e539999e... GitHub PR created: https://github.com/w3c/web-platform-tests/pull/5322 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
