|
|
Created:
3 years, 7 months ago by Fady Samuel Modified:
3 years, 6 months ago Reviewers:
sadrul CC:
chromium-reviews, kalyank, sadrul Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionAura-Mus: Fix high-DPI gutter
Previously, gutter did not take into account high-DPIness and didn't
convert from DIP to pixels and back correctly. This CL corrects this.
BUG=672962
Review-Url: https://codereview.chromium.org/2873473002
Cr-Commit-Position: refs/heads/master@{#475712}
Committed: https://chromium.googlesource.com/chromium/src/+/8b7c94c6b2a2cd8e80580162da291248bdd04bf9
Patch Set 1 #
Total comments: 2
Patch Set 2 : Addressed sadrul's comment #
Total comments: 4
Patch Set 3 : Addressed sadrul's comments #
Messages
Total messages: 31 (20 generated)
fsamuel@chromium.org changed reviewers: + sadrul@chromium.org
PTAL Sadrul!
The CQ bit was checked by fsamuel@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: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by fsamuel@chromium.org
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
No L-G-T-M from a valid reviewer yet. CQ run can only be started once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a full super star committer. Committers are members of the group "project-chromium-committers". Note that this has nothing to do with OWNERS files.
The CQ bit was checked by fsamuel@chromium.org to run a CQ dry run
PTAL Sadrul!
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/2873473002/diff/1/ui/aura/mus/window_tree_cli... File ui/aura/mus/window_tree_client_unittest.cc (right): https://codereview.chromium.org/2873473002/diff/1/ui/aura/mus/window_tree_cli... ui/aura/mus/window_tree_client_unittest.cc:118: switches::kForceDeviceScaleFactor, "2"); This makes all tests run in high-dpi. Can you create a subclass from here for the gutter test that adds the DSF 2? Or make all these tests parameterized on DSF ... although that may be going a bit overboard.
PTAL Sadrul! https://codereview.chromium.org/2873473002/diff/1/ui/aura/mus/window_tree_cli... File ui/aura/mus/window_tree_client_unittest.cc (right): https://codereview.chromium.org/2873473002/diff/1/ui/aura/mus/window_tree_cli... ui/aura/mus/window_tree_client_unittest.cc:118: switches::kForceDeviceScaleFactor, "2"); On 2017/05/09 15:04:59, sadrul wrote: > This makes all tests run in high-dpi. Can you create a subclass from here for > the gutter test that adds the DSF 2? > > Or make all these tests parameterized on DSF ... although that may be going a > bit overboard. Done.
The CQ bit was checked by fsamuel@chromium.org to run a CQ dry run
https://codereview.chromium.org/2873473002/diff/20001/ui/aura/mus/window_tree... File ui/aura/mus/window_tree_client_unittest.cc (right): https://codereview.chromium.org/2873473002/diff/20001/ui/aura/mus/window_tree... ui/aura/mus/window_tree_client_unittest.cc:111: public ::testing::WithParamInterface<UseHighDPI> { Just use WithParamInterface<bool>? https://codereview.chromium.org/2873473002/diff/20001/ui/aura/mus/window_tree... ui/aura/mus/window_tree_client_unittest.cc:214: ::testing::Values(UseHighDPI::YES, UseHighDPI::NO)); ::testing::Bool()
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 fsamuel@chromium.org to run a CQ dry run
PTAL https://codereview.chromium.org/2873473002/diff/20001/ui/aura/mus/window_tree... File ui/aura/mus/window_tree_client_unittest.cc (right): https://codereview.chromium.org/2873473002/diff/20001/ui/aura/mus/window_tree... ui/aura/mus/window_tree_client_unittest.cc:111: public ::testing::WithParamInterface<UseHighDPI> { On 2017/05/30 21:33:26, sadrul wrote: > Just use WithParamInterface<bool>? Done. https://codereview.chromium.org/2873473002/diff/20001/ui/aura/mus/window_tree... ui/aura/mus/window_tree_client_unittest.cc:214: ::testing::Values(UseHighDPI::YES, UseHighDPI::NO)); On 2017/05/30 21:33:26, sadrul wrote: > ::testing::Bool() 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: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by fsamuel@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": 40001, "attempt_start_ts": 1496191660485760, "parent_rev": "ca7526effd33b9d60760f5fc1d361830243a71c3", "commit_rev": "8b7c94c6b2a2cd8e80580162da291248bdd04bf9"}
Message was sent while issue was closed.
Description was changed from ========== Aura-Mus: Fix high-DPI gutter Previously, gutter did not take into account high-DPIness and didn't convert from DIP to pixels and back correctly. This CL corrects this. BUG=672962 ========== to ========== Aura-Mus: Fix high-DPI gutter Previously, gutter did not take into account high-DPIness and didn't convert from DIP to pixels and back correctly. This CL corrects this. BUG=672962 Review-Url: https://codereview.chromium.org/2873473002 Cr-Commit-Position: refs/heads/master@{#475712} Committed: https://chromium.googlesource.com/chromium/src/+/8b7c94c6b2a2cd8e80580162da29... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/8b7c94c6b2a2cd8e80580162da29... |