Mac: Always layout the Download Shelf, even if it has zero height.

Mac: Always layout the Download Shelf, even if it has zero height.

Even when the download shelf has zero height, Cocoa autolayout can
change its width. It does this when going fullscreen. However, not when
exiting fullscreen.

If the download shelf is visible (i.e. has non-zero height), then
-[BrowserWindowController applyLayout:] does the layout instead.
However, currently that layout is skipped if the download shelf is
hidden. This can leave the download shelf with a stale width that
influences the Cocoa autolayout applied when the browser window is
user-resized. This, in turn, can result in the browser window refusing
to violate a constraint that would break when the browser window is
resized to be small.

To fix, always set the download shelf frame, even if it has zero height.
This ensures the width is valid, even when hidden.

BUG=664477
TEST=On Mac (10.11 or 10.12) Open a new browser window (without a
download shelf). Resize the window to be "large", then make it
fullscreen (e.g. Cmd+Ctrl+f). Exit fullscreen (e.g. Cmd+Ctrl+f), then
resize the window again to try to make it "small". Ensure the width can
be made small as normal.

Committed: https://crrev.com/d8b183ffcfbb18293f623615017bc2e25b023906
Cr-Commit-Position: refs/heads/master@{#432024}

[tapted, 2016-11-14 05:27:37]

Description was changed from

==========
Mac: Always layout the Download Shelf, even if it has zero height.

Even when the download shelf has zero height, Cocoa autolayout can change
its width. It does this when going fullscreen. However, not when exiting
fullscreen.

If the download shelf is visible (i.e. has non-zero height), then
-[BrowserWindowController applyLayout:] does the layout instead. However,
currently that layout is skipped if the download shelf is hidden. This
can leave the download shelf with a stale width that influences the
Cocoa autolayout applied when the browser window is user-resized. This,
in turn, can result in the browser window refusing to violate a constraint
that would break when the browser window is resized to be small.

To fix, always set the download shelf frame, even if it has zero height.
This ensures the width is valid, even when hidden.

BUG=664477
TEST=On Mac (10.11 or 10.12) Open a new browser window (without a download
shelf). Resize the window to be "large", then make it fullscreen (e.g.
Cmd+Ctrl+f). Exit fullscreen (e.g. Cmd+Ctrl+f), then resize the window again
to try to make it "small". Ensure the width can be made small as normal.
==========

to

==========
Mac: Always layout the Download Shelf, even if it has zero height.

Even when the download shelf has zero height, Cocoa autolayout can
change its width. It does this when going fullscreen. However, not when
exiting fullscreen.

If the download shelf is visible (i.e. has non-zero height), then
-[BrowserWindowController applyLayout:] does the layout instead.
However, currently that layout is skipped if the download shelf is
hidden. This can leave the download shelf with a stale width that
influences the Cocoa autolayout applied when the browser window is
user-resized. This, in turn, can result in the browser window refusing
to violate a constraint that would break when the browser window is
resized to be small.

To fix, always set the download shelf frame, even if it has zero height.
This ensures the width is valid, even when hidden.

BUG=664477
TEST=On Mac (10.11 or 10.12) Open a new browser window (without a
download shelf). Resize the window to be "large", then make it
fullscreen (e.g. Cmd+Ctrl+f). Exit fullscreen (e.g. Cmd+Ctrl+f), then
resize the window again to try to make it "small". Ensure the width can
be made small as normal.
==========

[tapted, 2016-11-14 05:27:37]

tapted@chromium.org changed reviewers:
+ erikchen@chromium.org

[tapted, 2016-11-14 05:31:40]

Erik? Could please take a look? (+sdy cc - I think you've been looking at this
lately too).

[erikchen, 2016-11-14 18:29:50]

lgtm

I suspect we want something similar for bookmarkFrame, etc., but those layout
methods will perform non-trivial work even if the input is an empty rect. That
can wait until it proves to be an issue.

[erikchen, 2016-11-14 18:39:14]

On 2016/11/14 18:29:50, erikchen wrote:
> lgtm
> 
> I suspect we want something similar for bookmarkFrame, etc., but those layout
> methods will perform non-trivial work even if the input is an empty rect. That
> can wait until it proves to be an issue.

Woah. When did we opt into autolayout anywhere? Just saw this message,
presumably related:

"""
2016-11-14 10:38:04.759 Chromium[45685:2445315] Unable to simultaneously satisfy
constraints:
(
    "<NSAutoresizingMaskLayoutConstraint:0x7fc52ab3f1d0 h=-&- v=&--
V:[BookmarkBarToolbarView:0x7fc5295606d0(0)]>",
    "<NSAutoresizingMaskLayoutConstraint:0x7fc52ab3ec80 h=-&- v=-&-
V:|-(6)-[BookmarkBarView:0x7fc52955ba20]   (Names:
'|':BookmarkBarToolbarView:0x7fc5295606d0 )>",
    "<NSAutoresizingMaskLayoutConstraint:0x7fc52ab3ec30 h=-&- v=-&-
V:[BookmarkBarView:0x7fc52955ba20]-(6)-|   (Names:
'|':BookmarkBarToolbarView:0x7fc5295606d0 )>"
)

Will attempt to recover by breaking constraint 
<NSAutoresizingMaskLayoutConstraint:0x7fc52ab3ec80 h=-&- v=-&-
V:|-(6)-[BookmarkBarView:0x7fc52955ba20]   (Names:
'|':BookmarkBarToolbarView:0x7fc5295606d0 )>

Set the NSUserDefault
NSConstraintBasedLayoutVisualizeMutuallyExclusiveConstraints to YES to have
-[NSWindow visualizeConstraints:] automatically called when this happens. 
And/or, break on objc_exception_throw to catch this in the debugger.
"""

[Sidney San Martín, 2016-11-14 18:43:42]

On 2016/11/14 at 18:39:14, erikchen wrote:
> Woah. When did we opt into autolayout anywhere?

I think lgrey@ might be trying it out for their RTL work, I remember hearing
something about the bookmarks bar. My MD download shelf also uses Auto Layout,
though.

[lgrey, 2016-11-14 19:27:13]

On 2016/11/14 18:43:42, Sidney San Martín wrote:
> On 2016/11/14 at 18:39:14, erikchen wrote:
> > Woah. When did we opt into autolayout anywhere?
> 
> I think lgrey@ might be trying it out for their RTL work, I remember hearing
> something about the bookmarks bar. My MD download shelf also uses Auto Layout,
> though.

For RTL, I ended up deciding against it

[tapted, 2016-11-14 23:27:41]

On 2016/11/14 18:39:14, erikchen wrote:
> On 2016/11/14 18:29:50, erikchen wrote:
> > lgtm
> > 
> > I suspect we want something similar for bookmarkFrame, etc., but those
layout
> > methods will perform non-trivial work even if the input is an empty rect.
That
> > can wait until it proves to be an issue.
> 
> Woah. When did we opt into autolayout anywhere? Just saw this message,
> presumably related:
> 
> """
> 2016-11-14 10:38:04.759 Chromium[45685:2445315] Unable to simultaneously
satisfy
> constraints:
> (
>     "<NSAutoresizingMaskLayoutConstraint:0x7fc52ab3f1d0 h=-&- v=&--
> V:[BookmarkBarToolbarView:0x7fc5295606d0(0)]>",
>     "<NSAutoresizingMaskLayoutConstraint:0x7fc52ab3ec80 h=-&- v=-&-
> V:|-(6)-[BookmarkBarView:0x7fc52955ba20]   (Names:
> '|':BookmarkBarToolbarView:0x7fc5295606d0 )>",
>     "<NSAutoresizingMaskLayoutConstraint:0x7fc52ab3ec30 h=-&- v=-&-
> V:[BookmarkBarView:0x7fc52955ba20]-(6)-|   (Names:
> '|':BookmarkBarToolbarView:0x7fc5295606d0 )>"
> )
> 
> Will attempt to recover by breaking constraint 
> <NSAutoresizingMaskLayoutConstraint:0x7fc52ab3ec80 h=-&- v=-&-
> V:|-(6)-[BookmarkBarView:0x7fc52955ba20]   (Names:
> '|':BookmarkBarToolbarView:0x7fc5295606d0 )>
> 
> Set the NSUserDefault
> NSConstraintBasedLayoutVisualizeMutuallyExclusiveConstraints to YES to have
> -[NSWindow visualizeConstraints:] automatically called when this happens. 
> And/or, break on objc_exception_throw to catch this in the debugger.
> """

Followed up on chrome-mac-dev, but for archival sake:

My working theory is that we've always been configured for auto-layout. However,
after reconfiguring our window frame view hierarchy into something that AppKit
now supports (i.e. by not adding an unknown subview to the frame) in r424609 ,
AppKit is now able to actually apply auto-layout constraints where it would
previously either give up, or get lost.

Then the BookmarkBarView stuff is hopefully covered by a fix landed in r427290

[tapted, 2016-11-14 23:28:21]

The CQ bit was checked by tapted@chromium.org

[commit-bot: I haz the power, 2016-11-14 23:30:12]

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-11-15 00:42:27]

Description was changed from

==========
Mac: Always layout the Download Shelf, even if it has zero height.

Even when the download shelf has zero height, Cocoa autolayout can
change its width. It does this when going fullscreen. However, not when
exiting fullscreen.

If the download shelf is visible (i.e. has non-zero height), then
-[BrowserWindowController applyLayout:] does the layout instead.
However, currently that layout is skipped if the download shelf is
hidden. This can leave the download shelf with a stale width that
influences the Cocoa autolayout applied when the browser window is
user-resized. This, in turn, can result in the browser window refusing
to violate a constraint that would break when the browser window is
resized to be small.

To fix, always set the download shelf frame, even if it has zero height.
This ensures the width is valid, even when hidden.

BUG=664477
TEST=On Mac (10.11 or 10.12) Open a new browser window (without a
download shelf). Resize the window to be "large", then make it
fullscreen (e.g. Cmd+Ctrl+f). Exit fullscreen (e.g. Cmd+Ctrl+f), then
resize the window again to try to make it "small". Ensure the width can
be made small as normal.
==========

to

==========
Mac: Always layout the Download Shelf, even if it has zero height.

Even when the download shelf has zero height, Cocoa autolayout can
change its width. It does this when going fullscreen. However, not when
exiting fullscreen.

If the download shelf is visible (i.e. has non-zero height), then
-[BrowserWindowController applyLayout:] does the layout instead.
However, currently that layout is skipped if the download shelf is
hidden. This can leave the download shelf with a stale width that
influences the Cocoa autolayout applied when the browser window is
user-resized. This, in turn, can result in the browser window refusing
to violate a constraint that would break when the browser window is
resized to be small.

To fix, always set the download shelf frame, even if it has zero height.
This ensures the width is valid, even when hidden.

BUG=664477
TEST=On Mac (10.11 or 10.12) Open a new browser window (without a
download shelf). Resize the window to be "large", then make it
fullscreen (e.g. Cmd+Ctrl+f). Exit fullscreen (e.g. Cmd+Ctrl+f), then
resize the window again to try to make it "small". Ensure the width can
be made small as normal.
==========

[commit-bot: I haz the power, 2016-11-15 00:42:27]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-15 00:50:36]

Description was changed from

==========
Mac: Always layout the Download Shelf, even if it has zero height.

Even when the download shelf has zero height, Cocoa autolayout can
change its width. It does this when going fullscreen. However, not when
exiting fullscreen.

If the download shelf is visible (i.e. has non-zero height), then
-[BrowserWindowController applyLayout:] does the layout instead.
However, currently that layout is skipped if the download shelf is
hidden. This can leave the download shelf with a stale width that
influences the Cocoa autolayout applied when the browser window is
user-resized. This, in turn, can result in the browser window refusing
to violate a constraint that would break when the browser window is
resized to be small.

To fix, always set the download shelf frame, even if it has zero height.
This ensures the width is valid, even when hidden.

BUG=664477
TEST=On Mac (10.11 or 10.12) Open a new browser window (without a
download shelf). Resize the window to be "large", then make it
fullscreen (e.g. Cmd+Ctrl+f). Exit fullscreen (e.g. Cmd+Ctrl+f), then
resize the window again to try to make it "small". Ensure the width can
be made small as normal.
==========

to

==========
Mac: Always layout the Download Shelf, even if it has zero height.

Even when the download shelf has zero height, Cocoa autolayout can
change its width. It does this when going fullscreen. However, not when
exiting fullscreen.

If the download shelf is visible (i.e. has non-zero height), then
-[BrowserWindowController applyLayout:] does the layout instead.
However, currently that layout is skipped if the download shelf is
hidden. This can leave the download shelf with a stale width that
influences the Cocoa autolayout applied when the browser window is
user-resized. This, in turn, can result in the browser window refusing
to violate a constraint that would break when the browser window is
resized to be small.

To fix, always set the download shelf frame, even if it has zero height.
This ensures the width is valid, even when hidden.

BUG=664477
TEST=On Mac (10.11 or 10.12) Open a new browser window (without a
download shelf). Resize the window to be "large", then make it
fullscreen (e.g. Cmd+Ctrl+f). Exit fullscreen (e.g. Cmd+Ctrl+f), then
resize the window again to try to make it "small". Ensure the width can
be made small as normal.

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

[commit-bot: I haz the power, 2016-11-15 00:50:37]

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