Fix a crash in FullscreenExitBubbleController when the user clicks the "Exit full screen" button.

Fix a crash in FullscreenExitBubbleController when the user clicks the "Exit full screen" button.

The crash was due to the fact that Browser::OnDenyFullscreenPermission causes BrowserWindow::ExitFullscreen to be called, which causes the FullscreenExitBubbleController to be destroyed. [FullscreenExitBubbleController deny:] was sending a -hideSoon message to its zombie self following the call to Browser::OnDenyFullscreenPermission.

BUG=101835
TEST=The browser doesn't crash when you click "Exit full screen" on a fullscreened page.


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

[Nico, 2011-10-26 23:48:28]

Please add a test, and an explanation why it crashed previously.

[jeremya, 2011-10-28 04:18:37]

On 2011/10/26 23:48:28, Nico wrote:
> Please add a test, and an explanation why it crashed previously.

Done and done.

[(unused - use chromium), 2011-10-28 04:46:34]

http://codereview.chromium.org/8399017/diff/3001/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller.h (right):

http://codereview.chromium.org/8399017/diff/3001/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller.h:465: -
(FullscreenExitBubbleController*)fullscreenExitBubbleController;
Please put this in a TestingAPI category, see
http://codesearch.google.com/codesearch#search/&exact_package=chromium&q=%5C(...

http://codereview.chromium.org/8399017/diff/3001/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/8399017/diff/3001/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller.mm:1854: return
fullscreenExitBubbleController_.get();
ditto

http://codereview.chromium.org/8399017/diff/3001/chrome/browser/ui/cocoa/full...
File chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller_unittest.mm
(right):

http://codereview.chromium.org/8399017/diff/3001/chrome/browser/ui/cocoa/full...
chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller_unittest.mm:72:
TEST_F(FullscreenExitBubbleControllerTest, DenyExitsFullscreen) {
Nice test!

http://codereview.chromium.org/8399017/diff/3001/chrome/browser/ui/cocoa/full...
chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller_unittest.mm:89:
FullscreenExitBubbleController* fseb = [bwc fullscreenExitBubbleController];
no abbreviations; maybe just `bubble`. (`bwc` above is okish since we use it in
a few places already)

[jeremya, 2011-10-28 06:43:37]

http://codereview.chromium.org/8399017/diff/3001/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller.h (right):

http://codereview.chromium.org/8399017/diff/3001/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller.h:465: -
(FullscreenExitBubbleController*)fullscreenExitBubbleController;
On 2011/10/28 04:46:34, thakis wrote:
> Please put this in a TestingAPI category, see
>
http://codesearch.google.com/codesearch#search/&exact_package=chromium&q=%255...

Is it not already? (see the line below)

http://codereview.chromium.org/8399017/diff/3001/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/8399017/diff/3001/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller.mm:1854: return
fullscreenExitBubbleController_.get();
On 2011/10/28 04:46:34, thakis wrote:
> ditto

Other methods in this class are just commented as Private/TestingAPI, and
declared in @interface BrowserWindowController (TestingAPI). Should this method
be different?

http://codereview.chromium.org/8399017/diff/3001/chrome/browser/ui/cocoa/full...
File chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller_unittest.mm
(right):

http://codereview.chromium.org/8399017/diff/3001/chrome/browser/ui/cocoa/full...
chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller_unittest.mm:89:
FullscreenExitBubbleController* fseb = [bwc fullscreenExitBubbleController];
On 2011/10/28 04:46:34, thakis wrote:
> no abbreviations; maybe just `bubble`. (`bwc` above is okish since we use it
in
> a few places already)

Done.

[(unused - use chromium), 2011-10-28 15:13:20]

lgtm

[commit-bot: I haz the power, 2011-10-28 15:13:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeremya@chromium.org/8399017/5006

[Nico, 2011-10-28 15:14:12]

lgtm

[commit-bot: I haz the power, 2011-10-28 15:14:42]

Try job failure for 8399017-5006 on linux_rel for step "update".
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2011-10-28 23:14:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeremya@chromium.org/8399017/5006

[commit-bot: I haz the power, 2011-10-29 00:55:04]

Change committed as 107841