Enable Per-Monitor Support for Windows 10

Enable Per-Monitor Support for Windows 10

This allows Chrome to render at the native resolution on each display on
Windows 10.

BUG=426656, 624672
Committed: https://crrev.com/c110f1855d378172bcb74e0c26640bfc0d35d02d
Cr-Commit-Position: refs/heads/master@{#403329}

[robliao, 2016-06-30 01:17:24]

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

[commit-bot: I haz the power, 2016-06-30 01:17:59]

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-30 01:21:07]

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

[robliao, 2016-06-30 01:21:07]

thestig: Please review this changelist. Thanks!

[Lei Zhang, 2016-06-30 01:55:39]

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

[Lei Zhang, 2016-06-30 01:55:40]

scottmg: Can you chime in on whether chrome_initial can depend on chrome/common?
Would it drag in too much code?

https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn#newcode138
chrome/BUILD.gn:138: "//chrome/common",
Has Windows switched to GN yet? If not, don't you need to change GYP to match?

[commit-bot: I haz the power, 2016-06-30 02:14:15]

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

[commit-bot: I haz the power, 2016-06-30 02:14:16]

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_...)

[scottmg, 2016-06-30 05:05:52]

On 2016/06/30 01:55:40, Lei Zhang wrote:
> scottmg: Can you chime in on whether chrome_initial can depend on
chrome/common?
> Would it drag in too much code?

Yes, we don't want all of chrome in chrome.exe as well as chrome.dll, so that is
not a good idea.

(we should probably add some more assert_no_deps in gn to help make that clearer
I guess)

> 
> https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn
> File chrome/BUILD.gn (right):
> 
> https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn#newcode138
> chrome/BUILD.gn:138: "//chrome/common",
> Has Windows switched to GN yet? If not, don't you need to change GYP to match?

[robliao, 2016-06-30 05:10:07]

On 2016/06/30 05:05:52, scottmg wrote:
> On 2016/06/30 01:55:40, Lei Zhang wrote:
> > scottmg: Can you chime in on whether chrome_initial can depend on
> chrome/common?
> > Would it drag in too much code?
> 
> Yes, we don't want all of chrome in chrome.exe as well as chrome.dll, so that
is
> not a good idea.
> 
> (we should probably add some more assert_no_deps in gn to help make that
clearer
> I guess)
> 
> > 
> > https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn
> > File chrome/BUILD.gn (right):
> > 
> > https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn#newcode138
> > chrome/BUILD.gn:138: "//chrome/common",
> > Has Windows switched to GN yet? If not, don't you need to change GYP to
match?

What's the best way to pull in channel_info these days? Explicit include?

[scottmg, 2016-06-30 05:18:17]

On 2016/06/30 05:10:07, robliao wrote:
> On 2016/06/30 05:05:52, scottmg wrote:
> > On 2016/06/30 01:55:40, Lei Zhang wrote:
> > > scottmg: Can you chime in on whether chrome_initial can depend on
> > chrome/common?
> > > Would it drag in too much code?
> > 
> > Yes, we don't want all of chrome in chrome.exe as well as chrome.dll, so
that
> is
> > not a good idea.
> > 
> > (we should probably add some more assert_no_deps in gn to help make that
> clearer
> > I guess)
> > 
> > > 
> > > https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn
> > > File chrome/BUILD.gn (right):
> > > 
> > >
https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn#newcode138
> > > chrome/BUILD.gn:138: "//chrome/common",
> > > Has Windows switched to GN yet? If not, don't you need to change GYP to
> match?
> 
> What's the best way to pull in channel_info these days? Explicit include?

Why do you need channel_info for the < STABLE check? My understanding is that we
try pretty strongly to avoid having different behaviour on different channels.

[robliao, 2016-06-30 05:28:04]

On 2016/06/30 05:18:17, scottmg wrote:
> On 2016/06/30 05:10:07, robliao wrote:
> > On 2016/06/30 05:05:52, scottmg wrote:
> > > On 2016/06/30 01:55:40, Lei Zhang wrote:
> > > > scottmg: Can you chime in on whether chrome_initial can depend on
> > > chrome/common?
> > > > Would it drag in too much code?
> > > 
> > > Yes, we don't want all of chrome in chrome.exe as well as chrome.dll, so
> that
> > is
> > > not a good idea.
> > > 
> > > (we should probably add some more assert_no_deps in gn to help make that
> > clearer
> > > I guess)
> > > 
> > > > 
> > > > https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn
> > > > File chrome/BUILD.gn (right):
> > > > 
> > > >
> https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn#newcode138
> > > > chrome/BUILD.gn:138: "//chrome/common",
> > > > Has Windows switched to GN yet? If not, don't you need to change GYP to
> > match?
> > 
> > What's the best way to pull in channel_info these days? Explicit include?
> 
> Why do you need channel_info for the < STABLE check? My understanding is that
we
> try pretty strongly to avoid having different behaviour on different channels.

I want to get some more testing coverage for per-monitor DPI. There are quite a
few existing quirks today. The change fixes some quirks, but introduces some new
quirks; some due to Windows's handling of per-monitor DPI, some that I'll need
to fix in Chrome. It would be good to get a feel of what quirks are acceptable
for stable before actually going to stable.

[scottmg, 2016-06-30 05:30:18]

On 2016/06/30 05:28:04, robliao wrote:
> On 2016/06/30 05:18:17, scottmg wrote:
> > On 2016/06/30 05:10:07, robliao wrote:
> > > On 2016/06/30 05:05:52, scottmg wrote:
> > > > On 2016/06/30 01:55:40, Lei Zhang wrote:
> > > > > scottmg: Can you chime in on whether chrome_initial can depend on
> > > > chrome/common?
> > > > > Would it drag in too much code?
> > > > 
> > > > Yes, we don't want all of chrome in chrome.exe as well as chrome.dll, so
> > that
> > > is
> > > > not a good idea.
> > > > 
> > > > (we should probably add some more assert_no_deps in gn to help make that
> > > clearer
> > > > I guess)
> > > > 
> > > > > 
> > > > > https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn
> > > > > File chrome/BUILD.gn (right):
> > > > > 
> > > > >
> > https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn#newcode138
> > > > > chrome/BUILD.gn:138: "//chrome/common",
> > > > > Has Windows switched to GN yet? If not, don't you need to change GYP
to
> > > match?
> > > 
> > > What's the best way to pull in channel_info these days? Explicit include?
> > 
> > Why do you need channel_info for the < STABLE check? My understanding is
that
> we
> > try pretty strongly to avoid having different behaviour on different
channels.
> 
> I want to get some more testing coverage for per-monitor DPI. There are quite
a
> few existing quirks today. The change fixes some quirks, but introduces some
new
> quirks; some due to Windows's handling of per-monitor DPI, some that I'll need
> to fix in Chrome. It would be good to get a feel of what quirks are acceptable
> for stable before actually going to stable.

I think we ought to gate on a flag, or an experiment, or something else for that
then, but not on channel.

[robliao, 2016-06-30 05:36:04]

On 2016/06/30 05:30:18, scottmg wrote:
> On 2016/06/30 05:28:04, robliao wrote:
> > On 2016/06/30 05:18:17, scottmg wrote:
> > > On 2016/06/30 05:10:07, robliao wrote:
> > > > On 2016/06/30 05:05:52, scottmg wrote:
> > > > > On 2016/06/30 01:55:40, Lei Zhang wrote:
> > > > > > scottmg: Can you chime in on whether chrome_initial can depend on
> > > > > chrome/common?
> > > > > > Would it drag in too much code?
> > > > > 
> > > > > Yes, we don't want all of chrome in chrome.exe as well as chrome.dll,
so
> > > that
> > > > is
> > > > > not a good idea.
> > > > > 
> > > > > (we should probably add some more assert_no_deps in gn to help make
that
> > > > clearer
> > > > > I guess)
> > > > > 
> > > > > > 
> > > > > > https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn
> > > > > > File chrome/BUILD.gn (right):
> > > > > > 
> > > > > >
> > >
https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn#newcode138
> > > > > > chrome/BUILD.gn:138: "//chrome/common",
> > > > > > Has Windows switched to GN yet? If not, don't you need to change GYP
> to
> > > > match?
> > > > 
> > > > What's the best way to pull in channel_info these days? Explicit
include?
> > > 
> > > Why do you need channel_info for the < STABLE check? My understanding is
> that
> > we
> > > try pretty strongly to avoid having different behaviour on different
> channels.
> > 
> > I want to get some more testing coverage for per-monitor DPI. There are
quite
> a
> > few existing quirks today. The change fixes some quirks, but introduces some
> new
> > quirks; some due to Windows's handling of per-monitor DPI, some that I'll
need
> > to fix in Chrome. It would be good to get a feel of what quirks are
acceptable
> > for stable before actually going to stable.
> 
> I think we ought to gate on a flag, or an experiment, or something else for
that
> then, but not on channel.
IIRC Chrome Variations aren't available at this stage of Chrome yet, especially
since we haven't loaded chrome.dll, making an experiment a non-starter.
Doesn't the flag infrastructure also live in chrome.dll?

[Lei Zhang, 2016-06-30 05:43:10]

On 2016/06/30 05:18:17, scottmg wrote:
> Why do you need channel_info for the < STABLE check? My understanding is that
we
> try pretty strongly to avoid having different behaviour on different channels.

Yes, thanks for reminding us of this. You could just turn it on without any
channel checks, let it go to canary -> dev -> beta, and turn it off on the beta
branch before beta goes to stable / once you get sufficient feedback.

[robliao, 2016-06-30 05:49:03]

Description was changed from

==========
Enable Per-Monitor Support for Windows 10 and All Channels Except Stable

This allows Chrome to render at the native resolution on each display on
Windows 10 and on all channels except stable.

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

to

==========
Enable Per-Monitor Support for Windows 10 and All Channels Except Stable

This allows Chrome to render at the native resolution on each display on
Windows 10 and on all channels except stable.

BUG=426656,624672
==========

[robliao, 2016-06-30 05:53:28]

Description was changed from

==========
Enable Per-Monitor Support for Windows 10 and All Channels Except Stable

This allows Chrome to render at the native resolution on each display on
Windows 10 and on all channels except stable.

BUG=426656,624672
==========

to

==========
Enable Per-Monitor Support for Windows 10

This allows Chrome to render at the native resolution on each display on
Windows 10.

BUG=426656,624672
==========

[robliao, 2016-06-30 06:30:09]

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

[commit-bot: I haz the power, 2016-06-30 06:30:22]

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-30 07:57:04]

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

[commit-bot: I haz the power, 2016-06-30 07:57:04]

Dry run: This issue passed the CQ dry run.

[robliao, 2016-06-30 16:58:08]

sgtm. That makes the patch easier. I've opened an M-53 Release Blocking Bug to
track this then. Thanks!

https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2114503002/diff/1/chrome/BUILD.gn#newcode138
chrome/BUILD.gn:138: "//chrome/common",
On 2016/06/30 01:55:40, Lei Zhang wrote:
> Has Windows switched to GN yet? If not, don't you need to change GYP to match?

Obsoleted with latest patchset.

[Lei Zhang, 2016-06-30 17:05:17]

lgtm

[scottmg, 2016-06-30 17:06:29]

https://codereview.chromium.org/2114503002/diff/20001/chrome/app/chrome_exe_m...
File chrome/app/chrome_exe_main_win.cc (right):

https://codereview.chromium.org/2114503002/diff/20001/chrome/app/chrome_exe_m...
chrome/app/chrome_exe_main_win.cc:131: base::win::GetVersion() >=
base::win::VERSION_WIN10
Why not >= VERSION_WIN8_1? If there's some good reason, a comment here.

[robliao, 2016-06-30 19:53:48]

https://codereview.chromium.org/2114503002/diff/20001/chrome/app/chrome_exe_m...
File chrome/app/chrome_exe_main_win.cc (right):

https://codereview.chromium.org/2114503002/diff/20001/chrome/app/chrome_exe_m...
chrome/app/chrome_exe_main_win.cc:131: base::win::GetVersion() >=
base::win::VERSION_WIN10
On 2016/06/30 17:06:29, scottmg wrote:
> Why not >= VERSION_WIN8_1? If there's some good reason, a comment here.

Done. Add this comment:
  // Enable per-monitor DPI for Win10 or above instead of Win8.1 since Win8.1
  // does not have EnableChildWindowDpiMessage, necessary for correct non-client
  // area scaling across monitors.

[robliao, 2016-06-30 19:53:58]

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

[commit-bot: I haz the power, 2016-06-30 19:54:28]

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

[scottmg, 2016-06-30 19:57:12]

lgtm (Exciting! Nice work. :)

[robliao, 2016-06-30 20:12:45]

On 2016/06/30 19:57:12, scottmg wrote:
> lgtm (Exciting! Nice work. :)

Thanks! But, I'm still not done yet! While Chrome is functional, there's
definitely some fit-and-finish to apply, but at least we'll get sharp text on
platforms with displays of differing DPIs.

[commit-bot: I haz the power, 2016-06-30 21:49:46]

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

[commit-bot: I haz the power, 2016-06-30 21:49:47]

Dry run: This issue passed the CQ dry run.

[robliao, 2016-06-30 23:10:18]

The CQ bit was checked by robliao@chromium.org

[robliao, 2016-06-30 23:10:19]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/2114503002/#ps40001
(title: "Add Comment")

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

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-30 23:16:59]

Description was changed from

==========
Enable Per-Monitor Support for Windows 10

This allows Chrome to render at the native resolution on each display on
Windows 10.

BUG=426656,624672
==========

to

==========
Enable Per-Monitor Support for Windows 10

This allows Chrome to render at the native resolution on each display on
Windows 10.

BUG=426656,624672
==========

[commit-bot: I haz the power, 2016-06-30 23:17:01]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-30 23:17:03]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-06-30 23:19:40]

Description was changed from

==========
Enable Per-Monitor Support for Windows 10

This allows Chrome to render at the native resolution on each display on
Windows 10.

BUG=426656,624672
==========

to

==========
Enable Per-Monitor Support for Windows 10

This allows Chrome to render at the native resolution on each display on
Windows 10.

BUG=426656,624672

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

[commit-bot: I haz the power, 2016-06-30 23:19:41]

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

[robliao, 2016-07-01 17:03:13]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2115963002/ by robliao@chromium.org.

The reason for reverting is: Causes maximized incognito windows to render
blank..