Fix the segfault on Linux when closing a tab that is in tab fullscreen mode.

Fix the segfault on Linux when closing a tab that is in tab fullscreen mode.

It is caused by using a TabContentsWrapper pointer after it has been freed.

BUG=100680
TEST=BrowserTest.TestFullscreenBubbleMouseLockState should work on
Linux.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=106948

[yzshen1, 2011-10-21 21:45:28]

Please take a look. Thanks!

http://codereview.chromium.org/8369013/diff/1/chrome/browser/ui/views/frame/b...
File chrome/browser/ui/views/frame/browser_view.cc (left):

http://codereview.chromium.org/8369013/diff/1/chrome/browser/ui/views/frame/b...
chrome/browser/ui/views/frame/browser_view.cc:819:
DCHECK(fullscreen_request_.pending);
I observed fullscreen state change notification that is not caused by
Enter/ExitFullscreen() during browser shutdown. So I removed the DCHECK here.

[scheib, 2011-10-21 23:28:13]

One Q in the _gtk code.

http://codereview.chromium.org/8369013/diff/1/chrome/browser/ui/gtk/fullscree...
File chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc (right):

http://codereview.chromium.org/8369013/diff/1/chrome/browser/ui/gtk/fullscree...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc:86: hbox_ =
gtk_hbox_new(false, ui::kControlSpacing);
What happens if you click on any other part of the bubble? E.g. the message
text, or white space? Does that take focus? Do we need to call
...set_can_focus(..., FALSE) on absolutely everything here?

[yzshen1, 2011-10-21 23:29:25]

http://codereview.chromium.org/8369013/diff/1/chrome/browser/ui/gtk/fullscree...
File chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc (right):

http://codereview.chromium.org/8369013/diff/1/chrome/browser/ui/gtk/fullscree...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc:86: hbox_ =
gtk_hbox_new(false, ui::kControlSpacing);
On 2011/10/21 23:28:13, scheib wrote:
> What happens if you click on any other part of the bubble? E.g. the message
> text, or white space? Does that take focus? Do we need to call
> ...set_can_focus(..., FALSE) on absolutely everything here?

I tried and they didn't take focus.

[scheib, 2011-10-22 00:02:42]

lgtm

[koz (OOO until 15th September), 2011-10-24 00:57:05]

lgtm

http://codereview.chromium.org/8369013/diff/1/chrome/browser/ui/views/frame/b...
File chrome/browser/ui/views/frame/browser_view.cc (left):

http://codereview.chromium.org/8369013/diff/1/chrome/browser/ui/views/frame/b...
chrome/browser/ui/views/frame/browser_view.cc:819:
DCHECK(fullscreen_request_.pending);
On 2011/10/21 21:45:29, yzshen1 wrote:
> I observed fullscreen state change notification that is not caused by
> Enter/ExitFullscreen() during browser shutdown. So I removed the DCHECK here.

SGTM.

[sky, 2011-10-24 15:14:44]

LGTM

[commit-bot: I haz the power, 2011-10-24 17:00:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yzshen@chromium.org/8369013/1

[commit-bot: I haz the power, 2011-10-24 18:14:08]

Change committed as 106948