Prepare ScreenWin To Use Per-Monitor DPI

Prepare ScreenWin To Use Per-Monitor DPI

This allows ScreenWin to switch between the global DPI and the per monitor DPI
for platforms where per monitor DPI is availble.

BUG=426656
Committed: https://crrev.com/b479d9f705c35151dc32dde0363a50ef7783f23d
Cr-Commit-Position: refs/heads/master@{#402325}

[robliao, 2016-06-25 00:55:18]

Patchset #1 (id:1) has been deleted

[robliao, 2016-06-25 00:55:28]

The CQ bit was checked by robliao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-25 00:55:42]

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

[commit-bot: I haz the power, 2016-06-25 02:43:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-25 02:43:14]

Dry run: This issue passed the CQ dry run.

[robliao, 2016-06-27 17:01:28]

robliao@chromium.org changed reviewers:
+ scottmg@chromium.org

[robliao, 2016-06-27 17:01:29]

scottmg: Please review the WIndows calls in this CL. Thanks!

[scottmg, 2016-06-27 17:55:04]

https://codereview.chromium.org/2098993002/diff/20001/base/win/win_util.cc
File base/win/win_util.cc (right):

https://codereview.chromium.org/2098993002/diff/20001/base/win/win_util.cc#ne...
base/win/win_util.cc:615: ::GetProcAddress(shcore_dll,
"GetProcessDpiAwareness"));
How about

auto get_process_dpi_awareness =
reinterpret_cast<decltype(::GetProcessDpiAwareness)*>(::GetProcAddress(...));

, rather than redeclaring the function prototype?

https://codereview.chromium.org/2098993002/diff/20001/ui/display/win/screen_w...
File ui/display/win/screen_win.cc (right):

https://codereview.chromium.org/2098993002/diff/20001/ui/display/win/screen_w...
ui/display/win/screen_win.cc:38: HRESULT (WINAPI *)(HMONITOR, MONITOR_DPI_TYPE,
UINT*, UINT*);
Same here.

nit; This will also requery each time if get_dpi_for_monitor_func is null after
lookup. Consider something like

static auto get_dpi_for_monitor = []() {
  return reinterpret_cast<...>(::GetProcAddress(...));
}();

https://codereview.chromium.org/2098993002/diff/20001/ui/display/win/screen_w...
ui/display/win/screen_win.cc:52: SUCCEEDED(get_dpi_for_monitor_func(monitor,
MDT_EFFECTIVE_DPI,
Maybe DCHECK(monitor)?

[robliao, 2016-06-27 18:39:30]

Feedback applied. Thanks!

https://codereview.chromium.org/2098993002/diff/20001/base/win/win_util.cc
File base/win/win_util.cc (right):

https://codereview.chromium.org/2098993002/diff/20001/base/win/win_util.cc#ne...
base/win/win_util.cc:615: ::GetProcAddress(shcore_dll,
"GetProcessDpiAwareness"));
On 2016/06/27 17:55:04, scottmg wrote:
> How about
> 
> auto get_process_dpi_awareness =
> reinterpret_cast<decltype(::GetProcessDpiAwareness)*>(::GetProcAddress(...));
> 
> , rather than redeclaring the function prototype?

This is much nicer. Done!

https://codereview.chromium.org/2098993002/diff/20001/ui/display/win/screen_w...
File ui/display/win/screen_win.cc (right):

https://codereview.chromium.org/2098993002/diff/20001/ui/display/win/screen_w...
ui/display/win/screen_win.cc:38: HRESULT (WINAPI *)(HMONITOR, MONITOR_DPI_TYPE,
UINT*, UINT*);
On 2016/06/27 17:55:04, scottmg wrote:
> Same here.
> 
> nit; This will also requery each time if get_dpi_for_monitor_func is null
after
> lookup. Consider something like
> 
> static auto get_dpi_for_monitor = []() {
>   return reinterpret_cast<...>(::GetProcAddress(...));
> }();

Done.

https://codereview.chromium.org/2098993002/diff/20001/ui/display/win/screen_w...
ui/display/win/screen_win.cc:52: SUCCEEDED(get_dpi_for_monitor_func(monitor,
MDT_EFFECTIVE_DPI,
On 2016/06/27 17:55:04, scottmg wrote:
> Maybe DCHECK(monitor)?

Done at the top now.

[scottmg, 2016-06-27 18:41:52]

lgtm

https://codereview.chromium.org/2098993002/diff/40001/base/win/win_util.cc
File base/win/win_util.cc (right):

https://codereview.chromium.org/2098993002/diff/40001/base/win/win_util.cc#ne...
base/win/win_util.cc:611: auto process_dpi_awareness_func =
nit; process_dpi_awareness_func -> get_process_dpi_awareness[_func]

[robliao, 2016-06-27 18:52:37]

Patchset #3 (id:60001) has been deleted

[robliao, 2016-06-27 18:54:41]

robliao@chromium.org changed reviewers:
+ oshima@chromium.org

[robliao, 2016-06-27 18:54:42]

oshima: Please review ui/display/*. Thanks!

https://codereview.chromium.org/2098993002/diff/40001/base/win/win_util.cc
File base/win/win_util.cc (right):

https://codereview.chromium.org/2098993002/diff/40001/base/win/win_util.cc#ne...
base/win/win_util.cc:611: auto process_dpi_awareness_func =
On 2016/06/27 18:41:52, scottmg wrote:
> nit; process_dpi_awareness_func -> get_process_dpi_awareness[_func]

Done. I keep the _func since we seem to do that in quite a few places.

[oshima, 2016-06-27 18:58:52]

lgtm

https://codereview.chromium.org/2098993002/diff/80001/base/win/win_util.cc
File base/win/win_util.cc (right):

https://codereview.chromium.org/2098993002/diff/80001/base/win/win_util.cc#ne...
base/win/win_util.cc:606: } per_monitor_dpi_aware = PerMonitorDpiAware::UNKNOWN;
nit: new line would make it easier to read.

[robliao, 2016-06-27 19:07:32]

https://codereview.chromium.org/2098993002/diff/80001/base/win/win_util.cc
File base/win/win_util.cc (right):

https://codereview.chromium.org/2098993002/diff/80001/base/win/win_util.cc#ne...
base/win/win_util.cc:606: } per_monitor_dpi_aware = PerMonitorDpiAware::UNKNOWN;
On 2016/06/27 18:58:52, oshima wrote:
> nit: new line would make it easier to read.

sgtm. Done.

[robliao, 2016-06-27 19:07:37]

The CQ bit was checked by robliao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-27 19:08:24]

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

[commit-bot: I haz the power, 2016-06-27 21:05:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-27 21:05:01]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[robliao, 2016-06-27 21:17:44]

The CQ bit was checked by robliao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-27 21:18:51]

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

[robliao, 2016-06-27 22:29:13]

Description was changed from

==========
Prepare ScreenWin To Use Per Monitor DPI

This allows ScreenWin to switch between the global DPI and the per monitor DPI
for platforms where per monitor DPI is availble.

BUG=426656
==========

to

==========
Prepare ScreenWin To Use Per-Monitor DPI

This allows ScreenWin to switch between the global DPI and the per monitor DPI
for platforms where per monitor DPI is availble.

BUG=426656
==========

[commit-bot: I haz the power, 2016-06-27 22:51:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-27 22:51:28]

Dry run: This issue passed the CQ dry run.

[robliao, 2016-06-27 22:52:13]

The CQ bit was checked by robliao@chromium.org

[robliao, 2016-06-27 22:52:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
scottmg@chromium.org, oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/2098993002/#ps120001
(title: "Clang Build Fix")

[commit-bot: I haz the power, 2016-06-27 22:53:26]

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

[commit-bot: I haz the power, 2016-06-27 23:06:08]

Description was changed from

==========
Prepare ScreenWin To Use Per-Monitor DPI

This allows ScreenWin to switch between the global DPI and the per monitor DPI
for platforms where per monitor DPI is availble.

BUG=426656
==========

to

==========
Prepare ScreenWin To Use Per-Monitor DPI

This allows ScreenWin to switch between the global DPI and the per monitor DPI
for platforms where per monitor DPI is availble.

BUG=426656
==========

[commit-bot: I haz the power, 2016-06-27 23:06:10]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2016-06-27 23:09:37]

Description was changed from

==========
Prepare ScreenWin To Use Per-Monitor DPI

This allows ScreenWin to switch between the global DPI and the per monitor DPI
for platforms where per monitor DPI is availble.

BUG=426656
==========

to

==========
Prepare ScreenWin To Use Per-Monitor DPI

This allows ScreenWin to switch between the global DPI and the per monitor DPI
for platforms where per monitor DPI is availble.

BUG=426656

Committed: https://crrev.com/b479d9f705c35151dc32dde0363a50ef7783f23d
Cr-Commit-Position: refs/heads/master@{#402325}
==========

[commit-bot: I haz the power, 2016-06-27 23:09:39]

Patchset 5 (id:??) landed as
https://crrev.com/b479d9f705c35151dc32dde0363a50ef7783f23d
Cr-Commit-Position: refs/heads/master@{#402325}