Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2796303006:
Add gesture support in views::ResizeArea (Closed)
|
Created:
3 years, 8 months ago by tdanderson Modified:
3 years, 8 months ago Reviewers:
sadrul CC:
chromium-reviews, tfarina Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd gesture support in views::ResizeArea
Add gesture (touch) support in views::ResizeArea, which
is used for resizing the location bar. This area is located
directly to the right of the location bar.
BUG=320544
TEST=manual
Review-Url: https://codereview.chromium.org/2796303006
Cr-Commit-Position: refs/heads/master@{#463733}
Committed: https://chromium.googlesource.com/chromium/src/+/6db93d5b7ac95549bea2a1ec2d82cf6c1e8ed58d
Patch Set 1 #
Total comments: 2
Patch Set 2 : tests #
Total comments: 2
Patch Set 3 : use callback #Patch Set 4 : more OSX compilation issues #Patch Set 5 : skip OSX #
Messages
Total messages: 34 (25 generated)
The CQ bit was checked by tdanderson@chromium.org to run a CQ dry run
tdanderson@chromium.org changed reviewers: + sadrul@chromium.org
Sadrul, can you please take a look?
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.
lgtm Is it possible to write tests for this control? (looks like we currently don't have any, which is unfortunate) https://codereview.chromium.org/2796303006/diff/1/ui/views/controls/resize_ar... File ui/views/controls/resize_area_delegate.h (right): https://codereview.chromium.org/2796303006/diff/1/ui/views/controls/resize_ar... ui/views/controls/resize_area_delegate.h:19: // the user has released the mouse/finger. I would change this to ".. has released the pointer (e.g. mouse, stylus, touch etc.)"
The CQ bit was checked by tdanderson@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...
Sadrul, please have another look. https://codereview.chromium.org/2796303006/diff/1/ui/views/controls/resize_ar... File ui/views/controls/resize_area_delegate.h (right): https://codereview.chromium.org/2796303006/diff/1/ui/views/controls/resize_ar... ui/views/controls/resize_area_delegate.h:19: // the user has released the mouse/finger. On 2017/04/07 03:43:05, sadrul wrote: > I would change this to ".. has released the pointer (e.g. mouse, stylus, touch > etc.)" Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
lgtm For mac, I would try using widget()->GetRootView() with MoveMouse*, but I am OK with excluding the tests on mac in this CL (but please do a file bug for that) https://codereview.chromium.org/2796303006/diff/20001/ui/views/controls/resiz... File ui/views/controls/resize_area_unittest.cc (right): https://codereview.chromium.org/2796303006/diff/20001/ui/views/controls/resiz... ui/views/controls/resize_area_unittest.cc:166: EXPECT_TRUE(done_resizing()); Consider using EventGenerator::GestureScrollSequence[WithCallback] if it's not too much extra work.
The CQ bit was checked by tdanderson@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: 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 tdanderson@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: 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 tdanderson@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...
Thanks. I tried without success to get the tests working on OSX, but ended up skipping them. crbug.com/710475 tracks that work. https://codereview.chromium.org/2796303006/diff/20001/ui/views/controls/resiz... File ui/views/controls/resize_area_unittest.cc (right): https://codereview.chromium.org/2796303006/diff/20001/ui/views/controls/resiz... ui/views/controls/resize_area_unittest.cc:166: EXPECT_TRUE(done_resizing()); On 2017/04/10 18:02:51, sadrul wrote: > Consider using EventGenerator::GestureScrollSequence[WithCallback] if it's not > too much extra work. Done.
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 tdanderson@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org Link to the patchset: https://codereview.chromium.org/2796303006/#ps60001 (title: "skip OSX")
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": 1491939400735380,
"parent_rev": "1956acf3a49b71307e53b33129dad628b64326e8", "commit_rev":
"6db93d5b7ac95549bea2a1ec2d82cf6c1e8ed58d"}
Message was sent while issue was closed.
Description was changed from ========== Add gesture support in views::ResizeArea Add gesture (touch) support in views::ResizeArea, which is used for resizing the location bar. This area is located directly to the right of the location bar. BUG=320544 TEST=manual ========== to ========== Add gesture support in views::ResizeArea Add gesture (touch) support in views::ResizeArea, which is used for resizing the location bar. This area is located directly to the right of the location bar. BUG=320544 TEST=manual Review-Url: https://codereview.chromium.org/2796303006 Cr-Commit-Position: refs/heads/master@{#463733} Committed: https://chromium.googlesource.com/chromium/src/+/6db93d5b7ac95549bea2a1ec2d82... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:60001) as https://chromium.googlesource.com/chromium/src/+/6db93d5b7ac95549bea2a1ec2d82...
Message was sent while issue was closed.
Findit identified this CL at revision 463733 as the culprit for failures in the build cycles as shown on: https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb...
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:60001) has been created in https://codereview.chromium.org/2811143004/ by jdoerrie@chromium.org. The reason for reverting is: Reverting due to consistent failure on Linux Chromium OS ASan LSan Tests: https://luci-milo.appspot.com/buildbot/chromium.memory/Linux%20Chromium%20OS%... delegate_ = new TestResizeAreaDelegate; in ui/views/controls/resize_area_unittest.cc:118 is the likely culprit. BUG=710771. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
