Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2627463002:
Replace touch actions in pointer event tests with pointerActionSequence (Closed)
|
Created:
3 years, 11 months ago by lanwei Modified:
3 years, 11 months ago CC:
blink-reviews, blink-reviews-w3ctests_chromium.org, chromium-apps-reviews_chromium.org, chromium-reviews, darin-cc_chromium.org, dtapuska+chromiumwatch_chromium.org, extensions-reviews_chromium.org, jam, mlamouri+watch-content_chromium.org, piman+watch_chromium.org, tfarina Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionReplace touch actions in pointer event tests with pointerActionSequence
Replace touchScrollInTarget and pinchZoomInTarget in pointerevent_common_input.js
with pointerActionSequence.
BUG=680330
Review-Url: https://codereview.chromium.org/2627463002
Cr-Commit-Position: refs/heads/master@{#444096}
Committed: https://chromium.googlesource.com/chromium/src/+/11b24cf16bcfe02c4d881af965330ddfe72e7a64
Patch Set 1 : touch test #
Total comments: 8
Patch Set 2 : touch test #
Total comments: 1
Patch Set 3 : touch tests #
Messages
Total messages: 39 (30 generated)
Description was changed from ========== pointer event tests pointer event tests pointer event tests Add PointerActionSequence to GpuBenchmarking Add the generic synthetic pointer action sequence API to GpuBenchmarking class to generate a sequence of actions of multi-pointers. Currently we only support single source input type, and only one mouse device. BUG=525187 patch from issue 2591303002 at patchset 40001 (http://crrev.com/2591303002#ps40001) ========== to ========== Replace touch actions for pointer event tests with pointerActionSequence BUG=525187 ==========
Description was changed from ========== Replace touch actions for pointer event tests with pointerActionSequence BUG=525187 ========== to ========== Replace touch actions in pointer event tests with pointerActionSequence BUG=525187 ==========
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Patchset #1 (id:1) has been deleted
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 checked by lanwei@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...
Description was changed from ========== Replace touch actions in pointer event tests with pointerActionSequence BUG=525187 ========== to ========== Replace touch actions in pointer event tests with pointerActionSequence Replace touchScrollInTarget and pinchZoomInTarget in pointerevent_common_input.js with pointerActionSequence. BUG=525187 ==========
Description was changed from ========== Replace touch actions in pointer event tests with pointerActionSequence Replace touchScrollInTarget and pinchZoomInTarget in pointerevent_common_input.js with pointerActionSequence. BUG=525187 ========== to ========== Replace touch actions in pointer event tests with pointerActionSequence Replace touchScrollInTarget and pinchZoomInTarget in pointerevent_common_input.js with pointerActionSequence. BUG=525187 ==========
lanwei@chromium.org changed reviewers: + nzolghadr@chromium.org
lanwei@chromium.org changed reviewers: + tdresser@chromium.org
Patchset #1 (id:20001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Replace touch actions in pointer event tests with pointerActionSequence Replace touchScrollInTarget and pinchZoomInTarget in pointerevent_common_input.js with pointerActionSequence. BUG=525187 ========== to ========== Replace touch actions in pointer event tests with pointerActionSequence Replace touchScrollInTarget and pinchZoomInTarget in pointerevent_common_input.js with pointerActionSequence. BUG=680330 ==========
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Patchset #2 (id:50004) has been deleted
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2627463002/diff/40001/content/renderer/gpu/ac... File content/renderer/gpu/actions_parser.cc (right): https://codereview.chromium.org/2627463002/diff/40001/content/renderer/gpu/ac... content/renderer/gpu/actions_parser.cc:224: num_idle == 1) { nit: I believe this is okay to remove the "|| num_idle == 1" part in this condition.
https://codereview.chromium.org/2627463002/diff/40001/content/renderer/gpu/ac... File content/renderer/gpu/actions_parser.cc (right): https://codereview.chromium.org/2627463002/diff/40001/content/renderer/gpu/ac... content/renderer/gpu/actions_parser.cc:221: Add a comment explaining what's happening here. https://codereview.chromium.org/2627463002/diff/40001/content/renderer/gpu/ac... content/renderer/gpu/actions_parser.cc:225: return true; Do we need this return? https://codereview.chromium.org/2627463002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js (right): https://codereview.chromium.org/2627463002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js:176: return false; Should this throw?
The CQ bit was checked by lanwei@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.
https://codereview.chromium.org/2627463002/diff/40001/content/renderer/gpu/ac... File content/renderer/gpu/actions_parser.cc (right): https://codereview.chromium.org/2627463002/diff/40001/content/renderer/gpu/ac... content/renderer/gpu/actions_parser.cc:221: On 2017/01/16 15:45:30, tdresser wrote: > Add a comment explaining what's happening here. Done. https://codereview.chromium.org/2627463002/diff/40001/content/renderer/gpu/ac... content/renderer/gpu/actions_parser.cc:224: num_idle == 1) { On 2017/01/16 15:30:08, Navid Zolghadr wrote: > nit: I believe this is okay to remove the "|| num_idle == 1" part in this > condition. Done. https://codereview.chromium.org/2627463002/diff/40001/content/renderer/gpu/ac... content/renderer/gpu/actions_parser.cc:225: return true; On 2017/01/16 15:45:30, tdresser wrote: > Do we need this return? Done. https://codereview.chromium.org/2627463002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js (right): https://codereview.chromium.org/2627463002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js:176: return false; On 2017/01/16 15:45:30, tdresser wrote: > Should this throw? Done.
LGTM https://codereview.chromium.org/2627463002/diff/70001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js (right): https://codereview.chromium.org/2627463002/diff/70001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js:176: throw 'Scroll direction "' + direction + '" is not expected, we expecte "down", "up", "left" or "right"'; We should probably use () around the arguments to throw.
lanwei@chromium.org changed reviewers: + jochen@chromium.org
jochen@chromium.org: Please review changes in content/renderer/gpu/actions_parser.cc, thank you!
lgtm
The CQ bit was checked by lanwei@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nzolghadr@chromium.org, jochen@chromium.org, tdresser@chromium.org Link to the patchset: https://codereview.chromium.org/2627463002/#ps90001 (title: "touch tests")
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": 90001, "attempt_start_ts": 1484672735891650,
"parent_rev": "8f838fb2fa95ad16bada8d5a54336a9c3df656e2", "commit_rev":
"11b24cf16bcfe02c4d881af965330ddfe72e7a64"}
Message was sent while issue was closed.
Description was changed from ========== Replace touch actions in pointer event tests with pointerActionSequence Replace touchScrollInTarget and pinchZoomInTarget in pointerevent_common_input.js with pointerActionSequence. BUG=680330 ========== to ========== Replace touch actions in pointer event tests with pointerActionSequence Replace touchScrollInTarget and pinchZoomInTarget in pointerevent_common_input.js with pointerActionSequence. BUG=680330 Review-Url: https://codereview.chromium.org/2627463002 Cr-Commit-Position: refs/heads/master@{#444096} Committed: https://chromium.googlesource.com/chromium/src/+/11b24cf16bcfe02c4d881af96533... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:90001) as https://chromium.googlesource.com/chromium/src/+/11b24cf16bcfe02c4d881af96533... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
