New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 737206 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug

Blocked on:
issue 605219



Sign in to add a comment

HighSierra: [Incognito] Browser 'close/min/max' control is invisible.

Project Member Reported by manoranj...@chromium.org, Jun 27 2017

Issue description

Chrome: 61.0.3142.0
OS X: Mac OS X 10.13 Beta (17A291j)

For chrome incognito, 'close/min/max' control is invisible.

Looks like similar root cause of https://bugs.chromium.org/p/chromium/issues/detail?id=730174 ?

Thank you!
 
Screen Shot 2017-06-27 at 12.07.00 PM.png
86.2 KB View Download
This is not a regression, observing the similar behavior for M59# 59.0.3071.115 chrome as well.

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

> Looks like similar root cause of https://bugs.chromium.org/p/chromium/issues/detail?id=730174 ?

I believe so. Changing ShouldUseFullSizeContentView() to return true seems to fix it.

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

Blockedon: 605219
Status: Available (was: Untriaged)

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

Owner: sdy@chromium.org
Status: Started (was: Available)
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/+/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 7 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 8 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 9 by gov...@chromium.org, Jul 11 2017

A friendly reminder that M61 branch is coming soon on 07/20! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix ASAP to trunk. This way we branch M61 from a high quality trunk. Thank you.

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

Status: Fixed (was: Started)
Cc: linds...@chromium.org manoranj...@chromium.org
Mano, can you please verify this fix?
Labels: TE-Verified-62.0.3165.0
Status: Verified (was: Fixed)
Working as intended on Latest Canary#62.0.3165.0 (OS X: 10.13 Beta (17A306f)).

Sign in to add a comment