Chromium Code Reviews
Help | Chromium Project | Sign in
(48)

Issue 1952473002: Fix for 504133 - wandering identity switcher button (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 months, 3 weeks ago by kylix_rd
Modified:
11 months, 1 week ago
Reviewers:
Ilya Kulshin, sky
CC:
chromium-reviews, tfarina, Ilya Kulshin, Bret Sepulveda
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix for wandering identity switcher button when magnified BUG=504133 Committed: https://crrev.com/88d78eca5976e5c58f036aa0b87257653af6c1b7 Cr-Commit-Position: refs/heads/master@{#391595} Committed: https://crrev.com/13715b01cf03787843ff7f78bc002c604942ea36 Cr-Commit-Position: refs/heads/master@{#393977}

Patch Set 1 #

Total comments: 8

Patch Set 2 : Cleaup. #

Total comments: 12

Patch Set 3 : Update based on feedback. #

Total comments: 6

Patch Set 4 : Results of git cl format #

Patch Set 5 : This patch should fix the regressions and ensure the Identity switcher lands in the correct location #

Patch Set 6 : Removed unnecessary 'using' statement. #

Total comments: 7

Patch Set 7 : Changes based on review feedback. #

Total comments: 4

Patch Set 8 : Code style fixes #

Patch Set 9 : Must have relied on an include chain which changed for gfx::Point. Now explicitly included. #

Messages

Total messages: 41 (15 generated)
kylix_rd
This change should take into account the per-monitor DPI settings as handled by display::win::ScreenWin. https://codereview.chromium.org/1952473002/diff/1/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc ...
11 months, 3 weeks ago (2016-05-03 22:13:12 UTC) #3
sky
https://codereview.chromium.org/1952473002/diff/20001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc File chrome/browser/ui/views/frame/minimize_button_metrics_win.cc (right): https://codereview.chromium.org/1952473002/diff/20001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc#newcode19 chrome/browser/ui/views/frame/minimize_button_metrics_win.cc:19: POINT minimize_button_corner = { 0 }; POINT->gfx::Point https://codereview.chromium.org/1952473002/diff/20001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc#newcode28 chrome/browser/ui/views/frame/minimize_button_metrics_win.cc:28: ...
11 months, 3 weeks ago (2016-05-03 23:40:47 UTC) #4
Ilya Kulshin
https://codereview.chromium.org/1952473002/diff/20001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc File chrome/browser/ui/views/frame/minimize_button_metrics_win.cc (right): https://codereview.chromium.org/1952473002/diff/20001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc#newcode7 chrome/browser/ui/views/frame/minimize_button_metrics_win.cc:7: #include "chrome/browser/ui/views/frame/minimize_button_metrics_win.h" Keep this as the first include. In ...
11 months, 3 weeks ago (2016-05-03 23:40:52 UTC) #6
kylix_rd
https://codereview.chromium.org/1952473002/diff/20001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc File chrome/browser/ui/views/frame/minimize_button_metrics_win.cc (right): https://codereview.chromium.org/1952473002/diff/20001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc#newcode7 chrome/browser/ui/views/frame/minimize_button_metrics_win.cc:7: #include "chrome/browser/ui/views/frame/minimize_button_metrics_win.h" On 2016/05/03 23:40:52, Ilya Kulshin wrote: > ...
11 months, 3 weeks ago (2016-05-04 00:05:46 UTC) #7
kylix_rd
https://codereview.chromium.org/1952473002/diff/20001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc File chrome/browser/ui/views/frame/minimize_button_metrics_win.cc (right): https://codereview.chromium.org/1952473002/diff/20001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc#newcode19 chrome/browser/ui/views/frame/minimize_button_metrics_win.cc:19: POINT minimize_button_corner = { 0 }; On 2016/05/03 23:40:47, ...
11 months, 3 weeks ago (2016-05-04 14:25:10 UTC) #8
Ilya Kulshin
lgtm
11 months, 3 weeks ago (2016-05-04 17:20:44 UTC) #9
sky
Run git cl format and LGTM https://codereview.chromium.org/1952473002/diff/20001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc File chrome/browser/ui/views/frame/minimize_button_metrics_win.cc (right): https://codereview.chromium.org/1952473002/diff/20001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc#newcode19 chrome/browser/ui/views/frame/minimize_button_metrics_win.cc:19: POINT minimize_button_corner = ...
11 months, 3 weeks ago (2016-05-04 17:42:03 UTC) #10
kylix_rd
https://codereview.chromium.org/1952473002/diff/40001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc File chrome/browser/ui/views/frame/minimize_button_metrics_win.cc (right): https://codereview.chromium.org/1952473002/diff/40001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc#newcode31 chrome/browser/ui/views/frame/minimize_button_metrics_win.cc:31: { button_bounds.left + window_bounds.left, 0 }; On 2016/05/04 17:42:03, ...
11 months, 3 weeks ago (2016-05-04 17:59:04 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1952473002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1952473002/60001
11 months, 3 weeks ago (2016-05-04 18:01:42 UTC) #14
commit-bot: I haz the power
Committed patchset #4 (id:60001)
11 months, 3 weeks ago (2016-05-04 19:15:38 UTC) #16
commit-bot: I haz the power
Patchset 4 (id:??) landed as https://crrev.com/88d78eca5976e5c58f036aa0b87257653af6c1b7 Cr-Commit-Position: refs/heads/master@{#391595}
11 months, 3 weeks ago (2016-05-04 19:17:52 UTC) #18
kylix_rd
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/1953903002/ by kylixrd@chromium.org. ...
11 months, 3 weeks ago (2016-05-05 21:05:10 UTC) #19
sky
Might it be related to us flooring to 100% in some cases? On Thu, May ...
11 months, 3 weeks ago (2016-05-05 21:21:14 UTC) #20
scottmg
On 2016/05/05 21:21:14, sky wrote: > Might it be related to us flooring to 100% ...
11 months, 3 weeks ago (2016-05-05 21:23:36 UTC) #21
kylix_rd
Hopefully this will fix the regression *and* ensure the identity switcher button is placed in ...
11 months, 2 weeks ago (2016-05-09 18:24:52 UTC) #23
Ilya Kulshin
https://codereview.chromium.org/1952473002/diff/100001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc File chrome/browser/ui/views/frame/minimize_button_metrics_win.cc (right): https://codereview.chromium.org/1952473002/diff/100001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc#newcode27 chrome/browser/ui/views/frame/minimize_button_metrics_win.cc:27: int GetDefaultButtonBoundsOffset() { Add a comment describing how to ...
11 months, 2 weeks ago (2016-05-09 19:53:23 UTC) #24
kylix_rd
https://codereview.chromium.org/1952473002/diff/100001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc File chrome/browser/ui/views/frame/minimize_button_metrics_win.cc (right): https://codereview.chromium.org/1952473002/diff/100001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc#newcode27 chrome/browser/ui/views/frame/minimize_button_metrics_win.cc:27: int GetDefaultButtonBoundsOffset() { On 2016/05/09 19:53:23, Ilya Kulshin wrote: ...
11 months, 2 weeks ago (2016-05-09 21:54:23 UTC) #25
Ilya Kulshin
lgtm, although I'm not a committer, so you should probably also get an lgtm from ...
11 months, 2 weeks ago (2016-05-10 02:35:22 UTC) #26
kylix_rd
Bueller...Bueller? :)
11 months, 2 weeks ago (2016-05-11 17:59:51 UTC) #27
sky
LGTM https://codereview.chromium.org/1952473002/diff/120001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc File chrome/browser/ui/views/frame/minimize_button_metrics_win.cc (right): https://codereview.chromium.org/1952473002/diff/120001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc#newcode24 chrome/browser/ui/views/frame/minimize_button_metrics_win.cc:24: const int kInvalidOffset = (int)0x80000000; static_cast<int> https://codereview.chromium.org/1952473002/diff/120001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc#newcode32 chrome/browser/ui/views/frame/minimize_button_metrics_win.cc:32: ...
11 months, 2 weeks ago (2016-05-12 16:04:10 UTC) #28
kylix_rd
Code style fixes. https://codereview.chromium.org/1952473002/diff/120001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc File chrome/browser/ui/views/frame/minimize_button_metrics_win.cc (right): https://codereview.chromium.org/1952473002/diff/120001/chrome/browser/ui/views/frame/minimize_button_metrics_win.cc#newcode24 chrome/browser/ui/views/frame/minimize_button_metrics_win.cc:24: const int kInvalidOffset = (int)0x80000000; On ...
11 months, 2 weeks ago (2016-05-12 16:17:36 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1952473002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1952473002/140001
11 months, 1 week ago (2016-05-16 20:14:11 UTC) #32
commit-bot: I haz the power
Try jobs failed on following builders: win8_chromium_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/builds/148830)
11 months, 1 week ago (2016-05-16 20:50:07 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1952473002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1952473002/160001
11 months, 1 week ago (2016-05-16 22:29:29 UTC) #37
commit-bot: I haz the power
Committed patchset #9 (id:160001)
11 months, 1 week ago (2016-05-16 23:27:24 UTC) #39
commit-bot: I haz the power
11 months, 1 week ago (2016-05-16 23:29:08 UTC) #41
Message was sent while issue was closed.
Patchset 9 (id:??) landed as
https://crrev.com/13715b01cf03787843ff7f78bc002c604942ea36
Cr-Commit-Position: refs/heads/master@{#393977}
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46