win10: Fix various ui glitches

win10: Fix various ui glitches

This addresses the most egregious problems so that we look more
or less like a reasonable app on Win10.

In particular, fixes a too-wide border, removing the rounded corners
on the toolbar that don't work when there's no border (see 504044 for
agreement from UX), and an ugly overhang in the non-client area in
the top-left and top-right of the window.

In case you're not familiar with Win10 UI style (I wasn't!) I put
a composite of what standard apps plus before/after this patch here
http://i.imgur.com/JxNJvBv.png . We aren't easily able to switch to
a more "grey"-style title bar at the moment as that's reserved for
Windows-store applications (plain Win32 apps like notepad and chrome
always get the white title bar style). We might want to look into
that in the future as it's pretty harsh, but at least we match the
expected system behaviour this way.

R=pkasting@chromium.org
BUG=504044, 502001
Committed: https://crrev.com/48ddea097cd6bd2ef0fc4d133834b55d3b6d99fc
Cr-Commit-Position: refs/heads/master@{#336438}

[scottmg, 2015-06-25 00:56:05]

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

[scottmg, 2015-06-25 01:03:08]

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

[commit-bot: I haz the power, 2015-06-25 01:05:14]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1209723004/20001

[commit-bot: I haz the power, 2015-06-25 01:58:55]

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

[commit-bot: I haz the power, 2015-06-25 01:58:56]

Dry run: This issue passed the CQ dry run.

[scottmg, 2015-06-25 23:22:10]

scottmg@chromium.org changed reviewers:
+ sky@chromium.org
- pkasting@chromium.org

[scottmg, 2015-06-25 23:22:11]

->sky would you mind taking a look?

[Peter Kasting, 2015-06-25 23:34:11]

pkasting@chromium.org changed reviewers:
+ pkasting@chromium.org

[Peter Kasting, 2015-06-25 23:34:12]

LGTM

https://codereview.chromium.org/1209723004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc
(right):

https://codereview.chromium.org/1209723004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc:347: if
(base::win::GetVersion() >= base::win::VERSION_WIN10) {
Nit: I think this might be clearer if instead of doing this here, you added a
conditional to where we set these nonzero above:

    if (!IsMaximized() && !GetWidget()->IsFullscreen()) {
      margins.cyTopHeight = kClientEdgeThickness + 1;
      // On Windows 10, we don't draw our own window border, so don't extend the
      // nonclient area in for it.
      if (base::win::GetVersion() < base::win::VERSION_WIN10) {
        ...

https://codereview.chromium.org/1209723004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/glass_browser_frame_view.cc (right):

https://codereview.chromium.org/1209723004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:42: const int
kNonClientBorderThicknessWin81AndBelow = 3;
Nit: Use "PreWin10" as "Win81" looks like Windows 81 instead of 8.1.

https://codereview.chromium.org/1209723004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:336: return
kNonClientBorderThicknessWin10;
Nit: Accordingly, you'd also want to change this to:

  return (base::win::GetVersion() < base::win::VERSION_WIN10) ?
      kNonClientBorderThicknessPreWin10 : kNonClientBorderThicknessWin10;

https://codereview.chromium.org/1209723004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:355: // Windows system
browser on Win10+).
Nit: Maybe:

On Windows 10, we don't draw our own window border but rather go right to the
system border, so we don't need to draw the toolbar edges.

https://codereview.chromium.org/1209723004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:356: bool
rounded_corners = base::win::GetVersion() < base::win::VERSION_WIN10;
Nit: Just inline this into the one usage below.

[scottmg, 2015-06-26 18:29:32]

Thanks!

https://codereview.chromium.org/1209723004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc
(right):

https://codereview.chromium.org/1209723004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc:347: if
(base::win::GetVersion() >= base::win::VERSION_WIN10) {
On 2015/06/25 23:34:12, Peter Kasting wrote:
> Nit: I think this might be clearer if instead of doing this here, you added a
> conditional to where we set these nonzero above:
> 
>     if (!IsMaximized() && !GetWidget()->IsFullscreen()) {
>       margins.cyTopHeight = kClientEdgeThickness + 1;
>       // On Windows 10, we don't draw our own window border, so don't extend
the
>       // nonclient area in for it.
>       if (base::win::GetVersion() < base::win::VERSION_WIN10) {
>         ...

Done.

https://codereview.chromium.org/1209723004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/glass_browser_frame_view.cc (right):

https://codereview.chromium.org/1209723004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:42: const int
kNonClientBorderThicknessWin81AndBelow = 3;
On 2015/06/25 23:34:12, Peter Kasting wrote:
> Nit: Use "PreWin10" as "Win81" looks like Windows 81 instead of 8.1.

Done.

https://codereview.chromium.org/1209723004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:336: return
kNonClientBorderThicknessWin10;
On 2015/06/25 23:34:12, Peter Kasting wrote:
> Nit: Accordingly, you'd also want to change this to:
> 
>   return (base::win::GetVersion() < base::win::VERSION_WIN10) ?
>       kNonClientBorderThicknessPreWin10 : kNonClientBorderThicknessWin10;

Done.

https://codereview.chromium.org/1209723004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:355: // Windows system
browser on Win10+).
On 2015/06/25 23:34:12, Peter Kasting wrote:
> Nit: Maybe:
> 
> On Windows 10, we don't draw our own window border but rather go right to the
> system border, so we don't need to draw the toolbar edges.

Done.

https://codereview.chromium.org/1209723004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:356: bool
rounded_corners = base::win::GetVersion() < base::win::VERSION_WIN10;
On 2015/06/25 23:34:12, Peter Kasting wrote:
> Nit: Just inline this into the one usage below.

Done.

[scottmg, 2015-06-26 18:31:55]

The CQ bit was checked by scottmg@chromium.org

[scottmg, 2015-06-26 18:31:56]

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/1209723004/#ps40001
(title: "fixes")

[commit-bot: I haz the power, 2015-06-26 18:34:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1209723004/40001

[commit-bot: I haz the power, 2015-06-26 19:16:27]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-06-26 19:17:22]

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