[Mac] Change to detach the web contents view when the panel is not taller than the titlebar

[Mac] Change to detach the web contents view when the panel is not taller than the titlebar.

We used to hide the web contents view in such case but it seems to trigger some bad effect occasionally.

BUG=265932
TEST=existing tests
R=ccameron@chromium.org, dimich@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=217139

[Dmitry Titov, 2013-08-12 23:04:45]

Here are my comments, however I'm a bit uneasy about the whole thing because
it's unclear why this will fix the crash... Is the plausible theory of the crash
that we want to avoid resizing the RenderVidgetHostViewMac to 0 (or close)
height?

https://codereview.chromium.org/22629009/diff/7001/chrome/browser/ui/cocoa/pa...
File chrome/browser/ui/cocoa/panels/panel_window_controller_cocoa.mm (right):

https://codereview.chromium.org/22629009/diff/7001/chrome/browser/ui/cocoa/pa...
chrome/browser/ui/cocoa/panels/panel_window_controller_cocoa.mm:815: // if the
web contents view is made hidden.
lets put a bug number here, for reference to those crashes.

https://codereview.chromium.org/22629009/diff/7001/chrome/browser/ui/cocoa/pa...
chrome/browser/ui/cocoa/panels/panel_window_controller_cocoa.mm:820: // never
overlap with another view in order to perform optimization. If we do
"in order to perform optimization" is unclear. Do you mean avoiding painting
transparent rect for 'hole punching' by [RenderVidgetHostViewMac drawRect]?

https://codereview.chromium.org/22629009/diff/7001/chrome/browser/ui/cocoa/pa...
chrome/browser/ui/cocoa/panels/panel_window_controller_cocoa.mm:828:
panel::kTitlebarHeight) {
This condition happens when titlebar is all we can see, so the webcontent view
height is 0. Should we do it before this happens?  Will the crash happen
anyways?

https://codereview.chromium.org/22629009/diff/7001/chrome/browser/ui/cocoa/pa...
chrome/browser/ui/cocoa/panels/panel_window_controller_cocoa.mm:835: [[[self
window] contentView] addSubview:contentView];
Perhaps enableWebContentsViewAutosizing should be called here after attaching
the view back to view tree, to set up its size correctly.

[jianli, 2013-08-12 23:43:14]

https://codereview.chromium.org/22629009/diff/7001/chrome/browser/ui/cocoa/pa...
File chrome/browser/ui/cocoa/panels/panel_window_controller_cocoa.mm (right):

https://codereview.chromium.org/22629009/diff/7001/chrome/browser/ui/cocoa/pa...
chrome/browser/ui/cocoa/panels/panel_window_controller_cocoa.mm:815: // if the
web contents view is made hidden.
On 2013/08/12 23:04:45, Dmitry Titov wrote:
> lets put a bug number here, for reference to those crashes.

Done.

https://codereview.chromium.org/22629009/diff/7001/chrome/browser/ui/cocoa/pa...
chrome/browser/ui/cocoa/panels/panel_window_controller_cocoa.mm:820: // never
overlap with another view in order to perform optimization. If we do
On 2013/08/12 23:04:45, Dmitry Titov wrote:
> "in order to perform optimization" is unclear. Do you mean avoiding painting
> transparent rect for 'hole punching' by [RenderVidgetHostViewMac drawRect]?

Updated comment.

https://codereview.chromium.org/22629009/diff/7001/chrome/browser/ui/cocoa/pa...
chrome/browser/ui/cocoa/panels/panel_window_controller_cocoa.mm:828:
panel::kTitlebarHeight) {
On 2013/08/12 23:04:45, Dmitry Titov wrote:
> This condition happens when titlebar is all we can see, so the webcontent view
> height is 0. Should we do it before this happens?  Will the crash happen
> anyways?

When we minimize a panel, the window is being animated from full bounds to the
titlebar bounds. Before the animation starts, we disable autosizing (calling
disableWebContentsViewAutosizing in setPanelFrame). So the web contents view
will indeed remain unchanged. Previously we tried to hide the web contents view
here but RenderWidgetHostViewMac still thinks the view is there and asks the
compositor to do the work.

https://codereview.chromium.org/22629009/diff/7001/chrome/browser/ui/cocoa/pa...
chrome/browser/ui/cocoa/panels/panel_window_controller_cocoa.mm:835: [[[self
window] contentView] addSubview:contentView];
On 2013/08/12 23:04:45, Dmitry Titov wrote:
> Perhaps enableWebContentsViewAutosizing should be called here after attaching
> the view back to view tree, to set up its size correctly.

We can't enable the autosizing for the web contents view now since the animation
has not ended. Otherwise, we could some sort of flickering due to contents view
relayout. The animation cleanup code will reenable the autosizing.

[Ken Russell (switch to Gerrit), 2013-08-13 00:34:16]

ccameron's a better reviewer than me for this workaround but it looks acceptable
given that it's confined to the panel code. The implicit knowledge about the
window hierarchy is poor though and it would be better if this were encoded in
some internal API.

[Dmitry Titov, 2013-08-13 00:39:29]

lgtm

[ccameron, 2013-08-13 00:43:22]

lgtm too (with nit)

https://codereview.chromium.org/22629009/diff/14001/chrome/browser/ui/cocoa/p...
File chrome/browser/ui/cocoa/panels/panel_window_controller_cocoa.mm (right):

https://codereview.chromium.org/22629009/diff/14001/chrome/browser/ui/cocoa/p...
chrome/browser/ui/cocoa/panels/panel_window_controller_cocoa.mm:822: // become
visible and overlapp the bottom area of the titlebar.
nit: s/overlapp/overlap/

[jianli, 2013-08-13 00:59:05]

Committed patchset #3 manually as r217139 (presubmit successful).