Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2700433002:
cros: Fix minimized snapped window may not restore to the snapped bounds (Closed)
|
Created:
3 years, 10 months ago by Qiang(Joe) Xu Modified:
3 years, 10 months ago Reviewers:
oshima CC:
chromium-reviews, kalyank, sadrul Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptioncros: Fix minimized snapped window may not restore to the snapped bounds
Changes:
When new window is created, and snapped by shortcut, the minimizing/restoring will still be affected by window_positioner. This CL is to fix that.
BUG=692175
BUG=689764
TEST=test that the case(a) in crbug.com/692175 is fixed, test coverage is also added.
Review-Url: https://codereview.chromium.org/2700433002
Cr-Commit-Position: refs/heads/master@{#451690}
Committed: https://chromium.googlesource.com/chromium/src/+/0402e8c9dd194fd74dce5eccf6b65cae62ae7847
Patch Set 1 #Patch Set 2 #Patch Set 3 : rebase #Patch Set 4 : TEST_P to TEST_F #
Messages
Total messages: 29 (23 generated)
Description was changed from ========== cros: Fix minimized snapped window may not restore to the snapped bounds BUG=692175 BUG=689764 ========== to ========== cros: Fix minimized snapped window may not restore to the snapped bounds BUG=692175 BUG=689764 ==========
warx@chromium.org changed reviewers: + oshima@chromium.org
Description was changed from ========== cros: Fix minimized snapped window may not restore to the snapped bounds BUG=692175 BUG=689764 ========== to ========== cros: Fix minimized snapped window may not restore to the snapped bounds Changes: When new window is created, and snapped by shortcut, the minimizing/restoring will still be affected by window_positioner. This CL is to fix that. BUG=692175 BUG=689764 TEST=test that the case(a) in crbug.com/692175 is fixed, test coverage is also added. ==========
Hi Oshima, ptal, thanks!
The CQ bit was checked by warx@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.
Patchset #3 (id:40001) has been deleted
Patchset #3 (id:60001) has been deleted
The CQ bit was checked by warx@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: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by warx@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/2700433002/diff/120001/ash/common/accelerator... File ash/common/accelerators/accelerator_controller.cc (right): https://codereview.chromium.org/2700433002/diff/120001/ash/common/accelerator... ash/common/accelerators/accelerator_controller.cc:312: window_state->set_bounds_changed_by_user(true); can you do this in a default state? A window may ignore/reject this request (like in Maximize mdoe), in which case, we shouldn't set this.
Patchset #5 (id:120001) has been deleted
Patchset #4 (id:100001) has been deleted
Patchset #3 (id:80001) has been deleted
On 2017/02/17 17:21:57, oshima wrote: > https://codereview.chromium.org/2700433002/diff/120001/ash/common/accelerator... > File ash/common/accelerators/accelerator_controller.cc (right): > > https://codereview.chromium.org/2700433002/diff/120001/ash/common/accelerator... > ash/common/accelerators/accelerator_controller.cc:312: > window_state->set_bounds_changed_by_user(true); > can you do this in a default state? A window may ignore/reject this request > (like in Maximize mdoe), in which case, we shouldn't set this. Sure, reasonable. Patch set 2 did this in default_state.cc. I restored to that. Ptal, Thanks!
lgtm thanks!
The CQ bit was checked by warx@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": 160001, "attempt_start_ts": 1487642091555310,
"parent_rev": "e082e1d1591776aff10d468bc7057dfa3e49abea", "commit_rev":
"0402e8c9dd194fd74dce5eccf6b65cae62ae7847"}
Message was sent while issue was closed.
Description was changed from ========== cros: Fix minimized snapped window may not restore to the snapped bounds Changes: When new window is created, and snapped by shortcut, the minimizing/restoring will still be affected by window_positioner. This CL is to fix that. BUG=692175 BUG=689764 TEST=test that the case(a) in crbug.com/692175 is fixed, test coverage is also added. ========== to ========== cros: Fix minimized snapped window may not restore to the snapped bounds Changes: When new window is created, and snapped by shortcut, the minimizing/restoring will still be affected by window_positioner. This CL is to fix that. BUG=692175 BUG=689764 TEST=test that the case(a) in crbug.com/692175 is fixed, test coverage is also added. Review-Url: https://codereview.chromium.org/2700433002 Cr-Commit-Position: refs/heads/master@{#451690} Committed: https://chromium.googlesource.com/chromium/src/+/0402e8c9dd194fd74dce5eccf6b6... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:160001) as https://chromium.googlesource.com/chromium/src/+/0402e8c9dd194fd74dce5eccf6b6... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
