[Win] OBFV::GetIconSize() should return the value in dip unit

[Win] OBFV::GetIconSize() should return the value in dip unit

OpaqueBrowserFrameView::GetIconSize() returns the size of the window icon. On
Windows, the value is obtained from GetSystemMetrics(SM_CYSMICON) call, but the
problem is that the value returned is in pixel, not in dip. This leads to a
wrong sized window icon if an user is using a higher scale ratio. This CL
replaces GetSystemMetrics() with display::win::GetSystemMetricsInDIP() to fix
this.

BUG=631488
TEST=See the bug page for the reproduce steps

Committed: https://crrev.com/9502bad0f8dd68836373a6dfc1eae65f85a6e98a
Cr-Commit-Position: refs/heads/master@{#408017}

[Sungmann Cho, 2016-07-26 15:55:03]

Description was changed from

==========
[Win] OBFV::GetIconSize() should return the value in dip unit

OpaqueBrowserFrameView::GetIconSize() returns the size of the window icon. On
Windows, the value is obtained from GetSystemMetrics(SM_CYSMICON) call, but it
is in pixel, not in dip. This may lead to a wrong sized window icon if an user
is using a higher scale ratio.

BUG=631488
==========

to

==========
[Win] OBFV::GetIconSize() should return the value in dip unit

OpaqueBrowserFrameView::GetIconSize() returns the size of the window icon.
On Windows, the value is obtained from GetSystemMetrics(SM_CYSMICON) call,
but the problem is that the value returned is in pixel, not in dip. This leads
to a
wrong sized window icon if an user is using a higher scale ratio. This CL
replaces
GetSystemMetrics() with display::win::GetSystemMetricsInDIP() to fix this.

BUG=631488
==========

[Sungmann Cho, 2016-07-26 15:55:03]

sungmann.cho@navercorp.com changed reviewers:
+ pkasting@chromium.org

[Sungmann Cho, 2016-07-26 15:57:22]

Description was changed from

==========
[Win] OBFV::GetIconSize() should return the value in dip unit

OpaqueBrowserFrameView::GetIconSize() returns the size of the window icon.
On Windows, the value is obtained from GetSystemMetrics(SM_CYSMICON) call,
but the problem is that the value returned is in pixel, not in dip. This leads
to a
wrong sized window icon if an user is using a higher scale ratio. This CL
replaces
GetSystemMetrics() with display::win::GetSystemMetricsInDIP() to fix this.

BUG=631488
==========

to

==========
[Win] OBFV::GetIconSize() should return the value in dip unit

OpaqueBrowserFrameView::GetIconSize() returns the size of the window icon.
On Windows, the value is obtained from GetSystemMetrics(SM_CYSMICON) call,
but the problem is that the value returned is in pixel, not in dip.
This leads to a wrong sized window icon if an user is using a higher scale
ratio.
This CL replaces GetSystemMetrics() with display::win::GetSystemMetricsInDIP()
to fix this.

BUG=631488
==========

[Sungmann Cho, 2016-07-26 15:57:50]

Description was changed from

==========
[Win] OBFV::GetIconSize() should return the value in dip unit

OpaqueBrowserFrameView::GetIconSize() returns the size of the window icon.
On Windows, the value is obtained from GetSystemMetrics(SM_CYSMICON) call,
but the problem is that the value returned is in pixel, not in dip.
This leads to a wrong sized window icon if an user is using a higher scale
ratio.
This CL replaces GetSystemMetrics() with display::win::GetSystemMetricsInDIP()
to fix this.

BUG=631488
==========

to

==========
[Win] OBFV::GetIconSize() should return the value in dip unit

OpaqueBrowserFrameView::GetIconSize() returns the size of the window icon.
On Windows, the value is obtained from GetSystemMetrics(SM_CYSMICON) call,
but the problem is that the value returned is in pixel, not in dip.
This leads to a wrong sized window icon if an user is using a higher scale
ratio.
This CL replaces GetSystemMetrics() with display::win::GetSystemMetricsInDIP()
to fix this.

BUG=631488
==========

[Sungmann Cho, 2016-07-26 15:58:16]

Please take a look. Thanks!

[Peter Kasting, 2016-07-26 22:31:09]

LGTM; if possible, provide a TEST= line in your CL description for instructions
on how to test that this is fixed.

https://codereview.chromium.org/2183103002/diff/1/chrome/browser/ui/views/fra...
File chrome/browser/ui/views/frame/opaque_browser_frame_view.cc (right):

https://codereview.chromium.org/2183103002/diff/1/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:59: const int
kIconMinimumSize = 16;
Nit: While here, can you move this into the block that uses it (in
GetIconSize()) so we can get rid of this set of ifdefs?

[Sungmann Cho, 2016-07-26 23:05:25]

https://codereview.chromium.org/2183103002/diff/1/chrome/browser/ui/views/fra...
File chrome/browser/ui/views/frame/opaque_browser_frame_view.cc (right):

https://codereview.chromium.org/2183103002/diff/1/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:59: const int
kIconMinimumSize = 16;
On 2016/07/26 22:31:08, Peter Kasting wrote:
> Nit: While here, can you move this into the block that uses it (in
> GetIconSize()) so we can get rid of this set of ifdefs?

Done.

[Sungmann Cho, 2016-07-27 01:11:15]

Description was changed from

==========
[Win] OBFV::GetIconSize() should return the value in dip unit

OpaqueBrowserFrameView::GetIconSize() returns the size of the window icon.
On Windows, the value is obtained from GetSystemMetrics(SM_CYSMICON) call,
but the problem is that the value returned is in pixel, not in dip.
This leads to a wrong sized window icon if an user is using a higher scale
ratio.
This CL replaces GetSystemMetrics() with display::win::GetSystemMetricsInDIP()
to fix this.

BUG=631488
==========

to

==========
[Win] OBFV::GetIconSize() should return the value in dip unit

OpaqueBrowserFrameView::GetIconSize() returns the size of the window icon. On
Windows, the value is obtained from GetSystemMetrics(SM_CYSMICON) call, but the
problem is that the value returned is in pixel, not in dip. This leads to a
wrong sized window icon if an user is using a higher scale ratio. This CL
replaces GetSystemMetrics() with display::win::GetSystemMetricsInDIP() to fix
this.

BUG=631488
TEST=See the bug page for the reproduce steps
==========

[Sungmann Cho, 2016-07-27 01:12:58]

The CQ bit was checked by sungmann.cho@navercorp.com

[Sungmann Cho, 2016-07-27 01:12:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2183103002/#ps20001
(title: "Address comments")

[commit-bot: I haz the power, 2016-07-27 01:13:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sungmann Cho, 2016-07-27 01:29:37]

By the way, how can I be assigned the
bug(https://bugs.chromium.org/p/chromium/issues/detail?id=631488)? Can
committers only be assigned to it?

[Peter Kasting, 2016-07-27 01:53:20]

Bugs can only be assigned to project members, which is a larger group than
committers.

If you want to be able to mark bugs fixed and such, you may need bug edit
privileges, which are generally granted upon request.

[Sungmann Cho, 2016-07-27 02:13:08]

On 2016/07/27 01:53:20, Peter Kasting wrote:
> Bugs can only be assigned to project members, which is a larger group than
> committers.
> 
> If you want to be able to mark bugs fixed and such, you may need bug edit
> privileges, which are generally granted upon request.

Could you grant me the permission?

[Peter Kasting, 2016-07-27 02:16:47]

On 2016/07/27 02:13:08, Sungmann Cho wrote:
> On 2016/07/27 01:53:20, Peter Kasting wrote:
> > Bugs can only be assigned to project members, which is a larger group than
> > committers.
> > 
> > If you want to be able to mark bugs fixed and such, you may need bug edit
> > privileges, which are generally granted upon request.
> 
> Could you grant me the permission?

I sent mail to the relevant authorities asking that you be granted bug edit
permissions as well as the ability to schedule tryjobs for your CLs.

[Sungmann Cho, 2016-07-27 02:19:53]

On 2016/07/27 02:16:47, Peter Kasting wrote:
> On 2016/07/27 02:13:08, Sungmann Cho wrote:
> > On 2016/07/27 01:53:20, Peter Kasting wrote:
> > > Bugs can only be assigned to project members, which is a larger group than
> > > committers.
> > > 
> > > If you want to be able to mark bugs fixed and such, you may need bug edit
> > > privileges, which are generally granted upon request.
> > 
> > Could you grant me the permission?
> 
> I sent mail to the relevant authorities asking that you be granted bug edit
> permissions as well as the ability to schedule tryjobs for your CLs.

Great! Thanks for your help! :)

[commit-bot: I haz the power, 2016-07-27 02:21:00]

Description was changed from

==========
[Win] OBFV::GetIconSize() should return the value in dip unit

OpaqueBrowserFrameView::GetIconSize() returns the size of the window icon. On
Windows, the value is obtained from GetSystemMetrics(SM_CYSMICON) call, but the
problem is that the value returned is in pixel, not in dip. This leads to a
wrong sized window icon if an user is using a higher scale ratio. This CL
replaces GetSystemMetrics() with display::win::GetSystemMetricsInDIP() to fix
this.

BUG=631488
TEST=See the bug page for the reproduce steps
==========

to

==========
[Win] OBFV::GetIconSize() should return the value in dip unit

OpaqueBrowserFrameView::GetIconSize() returns the size of the window icon. On
Windows, the value is obtained from GetSystemMetrics(SM_CYSMICON) call, but the
problem is that the value returned is in pixel, not in dip. This leads to a
wrong sized window icon if an user is using a higher scale ratio. This CL
replaces GetSystemMetrics() with display::win::GetSystemMetricsInDIP() to fix
this.

BUG=631488
TEST=See the bug page for the reproduce steps
==========

[commit-bot: I haz the power, 2016-07-27 02:21:01]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-27 02:22:45]

Description was changed from

==========
[Win] OBFV::GetIconSize() should return the value in dip unit

OpaqueBrowserFrameView::GetIconSize() returns the size of the window icon. On
Windows, the value is obtained from GetSystemMetrics(SM_CYSMICON) call, but the
problem is that the value returned is in pixel, not in dip. This leads to a
wrong sized window icon if an user is using a higher scale ratio. This CL
replaces GetSystemMetrics() with display::win::GetSystemMetricsInDIP() to fix
this.

BUG=631488
TEST=See the bug page for the reproduce steps
==========

to

==========
[Win] OBFV::GetIconSize() should return the value in dip unit

OpaqueBrowserFrameView::GetIconSize() returns the size of the window icon. On
Windows, the value is obtained from GetSystemMetrics(SM_CYSMICON) call, but the
problem is that the value returned is in pixel, not in dip. This leads to a
wrong sized window icon if an user is using a higher scale ratio. This CL
replaces GetSystemMetrics() with display::win::GetSystemMetricsInDIP() to fix
this.

BUG=631488
TEST=See the bug page for the reproduce steps

Committed: https://crrev.com/9502bad0f8dd68836373a6dfc1eae65f85a6e98a
Cr-Commit-Position: refs/heads/master@{#408017}
==========

[commit-bot: I haz the power, 2016-07-27 02:22:46]

Patchset 2 (id:??) landed as
https://crrev.com/9502bad0f8dd68836373a6dfc1eae65f85a6e98a
Cr-Commit-Position: refs/heads/master@{#408017}