Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
mac: Fix bug where exiting tab fullscreen would cause Chrome menubar …
Browse files Browse the repository at this point in the history
…to disappear.

Merge into branch 2171.

Prior to my major fullscreen refactor, Presentation Mode and Canonical
Fullscreen had significant implementation differences. As a result,
fullscreen_controller treated the two modes very differently. If a user tried
to enter tab fullscreen after entering browser fullscreen (Presentation Mode),
enterImmersiveFullscreen: would not be invoked. If a user tried to do the same
after Canonical Fullscreen, enterImmersiveFullscreen: would be invoked.

After my major fullscreen refactor, Presentation Mode and Canonical Fullscreen
are almost identical. However, I failed to update the logic in
fullscreen_controller, which still treated the two modes differently.

This CL updates the logic in fullscreen_controller to treat both modes like
Presentation Mode. If a user tries to enter tab fullscreen after entering
browser fullscreen, enterImmersiveFullscreen: is not invoked.

Original Review URL: https://codereview.chromium.org/652983005
Original Cr-Commit-Position: refs/heads/master@{#299838}

Conflicts:
	chrome/browser/ui/cocoa/browser_window_cocoa.h
	chrome/browser/ui/fullscreen/fullscreen_controller_state_unittest.cc
	chrome/test/base/test_browser_window.h

TBR=rsesek@chromium.org
BUG=422191

Review URL: https://codereview.chromium.org/669603005

Cr-Commit-Position: refs/branch-heads/2171@{#198}
Cr-Branched-From: 267aeeb-refs/heads/master@{#297060}
  • Loading branch information
erikchen committed Oct 20, 2014
1 parent a6a2198 commit 7fed17e
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 20 deletions.
11 changes: 6 additions & 5 deletions chrome/browser/ui/browser_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -313,12 +313,13 @@ class BrowserWindow : public ui::BaseWindow {
virtual void Paste() = 0;

#if defined(OS_MACOSX)
// Enters Mac specific fullscreen mode with chrome displayed (e.g. omnibox)
// on OSX 10.7+, a.k.a. Lion Fullscreen mode.
// Invalid to call on OSX earlier than 10.7.
// Enters either from non fullscreen, or from fullscreen without chrome.
// Exit to normal fullscreen with EnterFullscreen().
// The following two methods cause the browser window to enter AppKit
// Fullscreen. The methods are idempotent. The methods are invalid to call on
// OSX 10.6. One method displays chrome (e.g. omnibox, tabstrip), whereas the
// other method hides it.
virtual void EnterFullscreenWithChrome() = 0;
virtual void EnterFullscreenWithoutChrome() = 0;

virtual bool IsFullscreenWithChrome() = 0;
virtual bool IsFullscreenWithoutChrome() = 0;
#endif
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ui/cocoa/browser_window_cocoa.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ class BrowserWindowCocoa :
virtual void Copy() OVERRIDE;
virtual void Paste() OVERRIDE;
virtual void EnterFullscreenWithChrome() OVERRIDE;
virtual void EnterFullscreenWithoutChrome() OVERRIDE;
virtual bool IsFullscreenWithChrome() OVERRIDE;
virtual bool IsFullscreenWithoutChrome() OVERRIDE;
virtual WindowOpenDisposition GetDispositionForPopupBounds(
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/ui/cocoa/browser_window_cocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,11 @@ new OneClickSigninDialogController(
[controller_ enterFullscreenWithChrome];
}

void BrowserWindowCocoa::EnterFullscreenWithoutChrome() {
CHECK(chrome::mac::SupportsSystemFullscreen());
[controller_ enterPresentationMode];
}

bool BrowserWindowCocoa::IsFullscreenWithChrome() {
return IsFullscreen() && ![controller_ inPresentationMode];
}
Expand Down
33 changes: 18 additions & 15 deletions chrome/browser/ui/fullscreen/fullscreen_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,25 +122,25 @@ void FullscreenController::ToggleFullscreenModeForTab(WebContents* web_contents,
#endif

bool in_browser_or_tab_fullscreen_mode = window_->IsFullscreen();
bool window_is_fullscreen_with_chrome = false;
#if defined(OS_MACOSX)
window_is_fullscreen_with_chrome = window_->IsFullscreenWithChrome();
#endif

if (enter_fullscreen) {
SetFullscreenedTab(web_contents);
if (!in_browser_or_tab_fullscreen_mode) {
state_prior_to_tab_fullscreen_ = STATE_NORMAL;
ToggleFullscreenModeInternal(TAB);
} else if (window_is_fullscreen_with_chrome) {
} else {
#if defined(OS_MACOSX)
state_prior_to_tab_fullscreen_ = STATE_BROWSER_FULLSCREEN_WITH_CHROME;
EnterFullscreenModeInternal(TAB);
state_prior_to_tab_fullscreen_ =
window_->IsFullscreenWithChrome()
? STATE_BROWSER_FULLSCREEN_WITH_CHROME
: STATE_BROWSER_FULLSCREEN_NO_CHROME;

// The browser is in AppKit fullscreen. Remove the chrome, if it's
// present.
window_->EnterFullscreenWithoutChrome();
#else
NOTREACHED();
#endif
} else {
state_prior_to_tab_fullscreen_ = STATE_BROWSER_FULLSCREEN_NO_CHROME;
#endif // defined(OS_MACOSX)

// We need to update the fullscreen exit bubble, e.g., going from browser
// fullscreen to tab fullscreen will need to show different content.
Expand All @@ -163,12 +163,15 @@ void FullscreenController::ToggleFullscreenModeForTab(WebContents* web_contents,
#if defined(OS_MACOSX)
if (state_prior_to_tab_fullscreen_ ==
STATE_BROWSER_FULLSCREEN_WITH_CHROME) {
EnterFullscreenModeInternal(BROWSER_WITH_CHROME);
} else {
// Clear the bubble URL, which forces the Mac UI to redraw.
UpdateFullscreenExitBubbleContent();
// The browser is still in AppKit Fullscreen. This just adds back the
// chrome.
window_->EnterFullscreenWithChrome();
}
#endif

// Clear the bubble URL, which forces the Mac UI to redraw.
UpdateFullscreenExitBubbleContent();
#endif // defined(OS_MACOSX)

// If currently there is a tab in "tab fullscreen" mode and fullscreen
// was not caused by it (i.e., previously it was in "browser fullscreen"
// mode), we need to switch back to "browser fullscreen" mode. In this
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class FullscreenControllerTestWindow : public TestBrowserWindow {
#endif
#if defined(OS_MACOSX)
virtual void EnterFullscreenWithChrome() OVERRIDE;
virtual void EnterFullscreenWithoutChrome() OVERRIDE;
virtual bool IsFullscreenWithChrome() OVERRIDE;
virtual bool IsFullscreenWithoutChrome() OVERRIDE;
#endif
Expand Down Expand Up @@ -127,6 +128,10 @@ void FullscreenControllerTestWindow::EnterFullscreenWithChrome() {
EnterFullscreen(true);
}

void FullscreenControllerTestWindow::EnterFullscreenWithoutChrome() {
EnterFullscreen(false);
}

bool FullscreenControllerTestWindow::IsFullscreenWithChrome() {
return IsFullscreen() && mac_with_chrome_mode_;
}
Expand Down
1 change: 1 addition & 0 deletions chrome/test/base/test_browser_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ class TestBrowserWindow : public BrowserWindow {
virtual void Paste() OVERRIDE {}
#if defined(OS_MACOSX)
virtual void EnterFullscreenWithChrome() OVERRIDE {}
virtual void EnterFullscreenWithoutChrome() OVERRIDE {}
virtual bool IsFullscreenWithChrome() OVERRIDE;
virtual bool IsFullscreenWithoutChrome() OVERRIDE;
#endif
Expand Down

0 comments on commit 7fed17e

Please sign in to comment.