Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2909213003:
Handle input events for ARC++ popup windows. (Closed)
DescriptionHandle input events for ARC++ popup windows.
TEST=Tested manually with apps with popups out of task bounds.
BUG=b/62088045
Review-Url: https://codereview.chromium.org/2909213003
Cr-Commit-Position: refs/heads/master@{#475751}
Committed: https://chromium.googlesource.com/chromium/src/+/cf4017d480a1ac88a04e34efacd2891a2a99586d
Patch Set 1 #Patch Set 2 : Self review. #
Total comments: 5
Patch Set 3 : Addressed comments. #Messages
Total messages: 17 (11 generated)
The CQ bit was checked by mtomasz@chromium.org to run a CQ dry run
Description was changed from ========== Handle input events for ARC++ popup windows. TEST=Tested manually with apps with popups out of task bounds. BUG=b/62088045 ========== to ========== Handle input events for ARC++ popup windows. TEST=Tested manually with apps with popups out of task bounds. BUG=b/62088045 ==========
mtomasz@chromium.org changed reviewers: + skuhne@chromium.org
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
mtomasz@chromium.org changed reviewers: + reveman@chromium.org
@skuhne, @reveman: PTAL. Thanks.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm with nits https://codereview.chromium.org/2909213003/diff/20001/components/exo/shell_su... File components/exo/shell_surface.cc (right): https://codereview.chromium.org/2909213003/diff/20001/components/exo/shell_su... components/exo/shell_surface.cc:113: gfx::Point local_point_in_shadow = local_point; nit: local_point_in_shadow_underlay as we might remove the shadow_ prefix in the near future https://codereview.chromium.org/2909213003/diff/20001/components/exo/shell_su... components/exo/shell_surface.cc:117: .Contains(local_point_in_shadow)) nit: use {} when statement and/or conditional code is more than one line https://codereview.chromium.org/2909213003/diff/20001/components/exo/shell_su... components/exo/shell_surface.cc:144: component == HTNOWHERE) { nit: please do this "component == HTNOWHERE" check before ConvertPointToTarget and HitTestRect as they are much more expensive. keeping the if statement above on line 134 as before might be the cleanest way to do this
lgtm
https://codereview.chromium.org/2909213003/diff/20001/components/exo/shell_su... File components/exo/shell_surface.cc (right): https://codereview.chromium.org/2909213003/diff/20001/components/exo/shell_su... components/exo/shell_surface.cc:113: gfx::Point local_point_in_shadow = local_point; On 2017/05/30 13:33:55, reveman wrote: > nit: local_point_in_shadow_underlay as we might remove the shadow_ prefix in the > near future Done. https://codereview.chromium.org/2909213003/diff/20001/components/exo/shell_su... components/exo/shell_surface.cc:117: .Contains(local_point_in_shadow)) On 2017/05/30 13:33:55, reveman wrote: > nit: use {} when statement and/or conditional code is more than one line Done.
The CQ bit was checked by mtomasz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from reveman@chromium.org, skuhne@chromium.org Link to the patchset: https://codereview.chromium.org/2909213003/#ps40001 (title: "Addressed comments.")
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": 40001, "attempt_start_ts": 1496194706950290,
"parent_rev": "ca5a950a9da72f942db32e10751f459584998c22", "commit_rev":
"cf4017d480a1ac88a04e34efacd2891a2a99586d"}
Message was sent while issue was closed.
Description was changed from ========== Handle input events for ARC++ popup windows. TEST=Tested manually with apps with popups out of task bounds. BUG=b/62088045 ========== to ========== Handle input events for ARC++ popup windows. TEST=Tested manually with apps with popups out of task bounds. BUG=b/62088045 Review-Url: https://codereview.chromium.org/2909213003 Cr-Commit-Position: refs/heads/master@{#475751} Committed: https://chromium.googlesource.com/chromium/src/+/cf4017d480a1ac88a04e34efacd2... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/cf4017d480a1ac88a04e34efacd2... |
