New issue
Advanced search Search tips

Issue 730679 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocked on:
issue 605219

Blocking:
issue 748727



Sign in to add a comment

High Siera: Browser window frame lacks rounded corners

Project Member Reported by rsesek@chromium.org, Jun 7 2017

Issue description

Chrome Version: 61.0.3123.0
OS: macOS 10.13 17A264c

What steps will reproduce the problem?
(1) Open Chrome
(2) Look at four corners of a browser window

What is the expected result?
The corners of the browser window should be rounded.

What happens instead?
They are square.

This may also be related to  issue 730174 .

Please use labels and text to provide additional information.


For graphics-related bugs, please copy/paste the contents of the about:gpu
page at the end of this report.

 
Screen Shot 2017-06-07 at 12.36.24 PM.png
166 KB View Download

Comment 1 by lgrey@google.com, Jun 9 2017

Status: Available (was: Untriaged)

Comment 2 by shrike@chromium.org, Jun 20 2017

Running M59 as well as 61.0.3136.0 Canary on 10.13 17A264c on my Touch Bar Mac, the top window corners are rounded (but the bottom ones are still squared off).

Changing ShouldUseFullSizeContentView() to return true (so that we're no longer a peer of the window contentview) fixes the rounded corners problem.

Comment 3 by shrike@chromium.org, Jun 30 2017

Blockedon: 605219

Comment 4 by sdy@chromium.org, Jul 6 2017

Owner: sdy@chromium.org
Status: Started (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/45f7994fbcf0e40e02e4c4823a07b9b6ef28a325

commit 45f7994fbcf0e40e02e4c4823a07b9b6ef28a325
Author: Sidney San Martín <sdy@chromium.org>
Date: Tue Jul 11 07:05:57 2017

Enable ShouldUseFullSizeContentView().

Also fixes the known issues with enabling it:

- crbug/665787: The tab background view was added in front of the content and
  blocked interaction with the top of the scroll bar when the toolbar was
  hidden. It's now inserted behind the content, like before.

- crbug/655112: FramedBrowserWindowTest.DoesHideTitle took screenshots of the
  frame view, which gets drawn using a different (layer-backed?) path when
  NSFullSizeContentViewWindowMask is on. The test now calls -[CATransaction
  flush] before taking each screenshot, and uses a different API to screenshot
  the whole window instead of trying to grab just the frame view by accessing
  the content view's superview.

Bug:  605219 ,  655112 ,  665787 ,  730679 ,  734713 ,  737206 
Change-Id: Id78c21aff6748e02fd4b3e461c77ef9f7454d744
Reviewed-on: https://chromium-review.googlesource.com/566030
Commit-Queue: Sidney San Martin <sdy@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485544}
[modify] https://crrev.com/45f7994fbcf0e40e02e4c4823a07b9b6ef28a325/chrome/browser/ui/cocoa/browser_window_layout.mm
[modify] https://crrev.com/45f7994fbcf0e40e02e4c4823a07b9b6ef28a325/chrome/browser/ui/cocoa/framed_browser_window_unittest.mm
[modify] https://crrev.com/45f7994fbcf0e40e02e4c4823a07b9b6ef28a325/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/685e99063e6bac1d4417829ec3b6abdcb918caf6

commit 685e99063e6bac1d4417829ec3b6abdcb918caf6
Author: Keishi Hattori <keishi@chromium.org>
Date: Tue Jul 11 08:55:01 2017

Revert "Enable ShouldUseFullSizeContentView()."

This reverts commit 45f7994fbcf0e40e02e4c4823a07b9b6ef28a325.

Reason for revert: Broke BrowserWindowControllerTest.UsesAutoLayout

Original change's description:
> Enable ShouldUseFullSizeContentView().
> 
> Also fixes the known issues with enabling it:
> 
> - crbug/665787: The tab background view was added in front of the content and
>   blocked interaction with the top of the scroll bar when the toolbar was
>   hidden. It's now inserted behind the content, like before.
> 
> - crbug/655112: FramedBrowserWindowTest.DoesHideTitle took screenshots of the
>   frame view, which gets drawn using a different (layer-backed?) path when
>   NSFullSizeContentViewWindowMask is on. The test now calls -[CATransaction
>   flush] before taking each screenshot, and uses a different API to screenshot
>   the whole window instead of trying to grab just the frame view by accessing
>   the content view's superview.
> 
> Bug:  605219 ,  655112 ,  665787 ,  730679 ,  734713 ,  737206 
> Change-Id: Id78c21aff6748e02fd4b3e461c77ef9f7454d744
> Reviewed-on: https://chromium-review.googlesource.com/566030
> Commit-Queue: Sidney San Martin <sdy@chromium.org>
> Reviewed-by: Trent Apted <tapted@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#485544}

TBR=tapted@chromium.org,shrike@chromium.org,sdy@chromium.org

Change-Id: I00b73bc5d2d5607736ddc81b9ae81ec65527ebad
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  605219 ,  655112 ,  665787 ,  730679 ,  734713 ,  737206 
Reviewed-on: https://chromium-review.googlesource.com/566898
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485564}
[modify] https://crrev.com/685e99063e6bac1d4417829ec3b6abdcb918caf6/chrome/browser/ui/cocoa/browser_window_layout.mm
[modify] https://crrev.com/685e99063e6bac1d4417829ec3b6abdcb918caf6/chrome/browser/ui/cocoa/framed_browser_window_unittest.mm
[modify] https://crrev.com/685e99063e6bac1d4417829ec3b6abdcb918caf6/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a20a025fb10592190b5b3bc90fd627c45c76676a

commit a20a025fb10592190b5b3bc90fd627c45c76676a
Author: Sidney San Martín <sdy@chromium.org>
Date: Tue Jul 11 18:39:06 2017

Enable ShouldUseFullSizeContentView().

Also fixes the known issues with enabling it:

- crbug/665787: The tab background view was added in front of the content and
  blocked interaction with the top of the scroll bar when the toolbar was
  hidden. It's now inserted behind the content, like before.

- crbug/655112: FramedBrowserWindowTest.DoesHideTitle took screenshots of the
  frame view, which gets drawn using a different (layer-backed?) path when
  NSFullSizeContentViewWindowMask is on. The test now calls -[CATransaction
  flush] before taking each screenshot, and uses a different API to screenshot
  the whole window instead of trying to grab just the frame view by accessing
  the content view's superview.

This is a re-land of r485544 (reverted in r485564) with a test fix for
BrowserWindowControllerTest.UsesAutoLayout.

Bug:  605219 ,  655112 ,  665787 ,  730679 ,  734713 ,  737206 
Change-Id: I59c03e90b30613285df89a1b3694f36de3f1a7d8
Reviewed-on: https://chromium-review.googlesource.com/567205
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Sidney San Martin <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485682}
[modify] https://crrev.com/a20a025fb10592190b5b3bc90fd627c45c76676a/chrome/browser/ui/cocoa/browser_window_controller_unittest.mm
[modify] https://crrev.com/a20a025fb10592190b5b3bc90fd627c45c76676a/chrome/browser/ui/cocoa/browser_window_layout.mm
[modify] https://crrev.com/a20a025fb10592190b5b3bc90fd627c45c76676a/chrome/browser/ui/cocoa/framed_browser_window_unittest.mm
[modify] https://crrev.com/a20a025fb10592190b5b3bc90fd627c45c76676a/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm

Comment 8 by sdy@chromium.org, Jul 12 2017

Status: Fixed (was: Started)

Comment 9 by sdy@chromium.org, Aug 4 2017

Blocking: 748727

Sign in to add a comment