Commit dd63d249 authored by erikchen's avatar erikchen Committed by Commit bot

Mac: Fix accidental changes to fullscreen logic from refactor. (reland)

This CL was reverted due to test failures. Under inspection, the test proved to
already be flaky. The test has been disabled. See http://crbug.com/415422 for
more details.

--------------Original CL Description-----------------

The menu item "Enter Presentation Mode" has different effects in 10.6 and
10.7+. In 10.6, the menu item invokes Immersive Fullscreen. In 10.7+, the menu
item invokes AppKit Fullscreen. Similar logic applies to fullscreen invoked by
an extension.

I accidentally changed this logic during the fullscreen refactor by removing
the 10.6 vs 10.7+ conditional. This CL adds back the conditional. I also
updated the code to explicitly indicate the mechanism that is invoking
fullscreen to prevent similar future mistakes.

Original CL: https://codereview.chromium.org/575653003/

BUG=NONE
TBR=rsesek@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#295598}
parent 5b88d999
...@@ -43,6 +43,7 @@ ...@@ -43,6 +43,7 @@
#import "chrome/browser/ui/cocoa/toolbar/toolbar_controller.h" #import "chrome/browser/ui/cocoa/toolbar/toolbar_controller.h"
#import "chrome/browser/ui/cocoa/web_dialog_window_controller.h" #import "chrome/browser/ui/cocoa/web_dialog_window_controller.h"
#import "chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h" #import "chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.h"
#include "chrome/browser/ui/fullscreen/fullscreen_controller.h"
#include "chrome/browser/ui/search/search_model.h" #include "chrome/browser/ui/search/search_model.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/web_applications/web_app.h" #include "chrome/browser/web_applications/web_app.h"
...@@ -350,9 +351,20 @@ void BrowserWindowCocoa::Restore() { ...@@ -350,9 +351,20 @@ void BrowserWindowCocoa::Restore() {
[window() deminiaturize:controller_]; [window() deminiaturize:controller_];
} }
void BrowserWindowCocoa::EnterFullscreen( // See browser_window_controller.h for a detailed explanation of the logic in
const GURL& url, FullscreenExitBubbleType bubble_type) { // this method.
[controller_ enterHTML5FullscreenForURL:url bubbleType:bubble_type]; void BrowserWindowCocoa::EnterFullscreen(const GURL& url,
FullscreenExitBubbleType bubble_type) {
if (browser_->fullscreen_controller()->IsWindowFullscreenForTabOrPending()) {
[controller_ enterWebContentFullscreenForURL:url bubbleType:bubble_type];
return;
}
if (url.is_empty()) {
[controller_ enterPresentationMode];
} else {
[controller_ enterExtensionFullscreenForURL:url bubbleType:bubble_type];
}
} }
void BrowserWindowCocoa::ExitFullscreen() { void BrowserWindowCocoa::ExitFullscreen() {
......
...@@ -463,6 +463,31 @@ class Command; ...@@ -463,6 +463,31 @@ class Command;
// //
// + HTML5 fullscreen. <-- Currently uses AppKitFullscreen API. This should // + HTML5 fullscreen. <-- Currently uses AppKitFullscreen API. This should
// eventually migrate to the Immersive Fullscreen API. // eventually migrate to the Immersive Fullscreen API.
//
// There are more fullscreen styles on OSX than other OSes. However, all OSes
// share the same cross-platform code for entering fullscreen
// (FullscreenController). It is important for OSX fullscreen logic to track
// how the user triggered fullscreen mode.
// There are currently 5 possible mechanisms:
// - User clicks the AppKit Fullscreen button.
// -- This invokes -[BrowserWindowController windowWillEnterFullscreen:]
// - User selects the menu item "Enter Full Screen".
// -- This invokes FullscreenController::ToggleFullscreenModeInternal(
// BROWSER_WITH_CHROME)
// - User selects the menu item "Enter Presentation Mode".
// -- This invokes FullscreenController::ToggleFullscreenModeInternal(
// BROWSER)
// -- The corresponding URL will be empty.
// - User requests fullscreen via an extension.
// -- This invokes FullscreenController::ToggleFullscreenModeInternal(
// BROWSER)
// -- The corresponding URL will be the url of the extension.
// - User requests fullscreen via Flash or JavaScript apis.
// -- This invokes FullscreenController::ToggleFullscreenModeInternal(
// BROWSER)
// -- browser_->fullscreen_controller()->
// IsWindowFullscreenForTabOrPending() returns true.
// -- The corresponding URL will be the url of the web page.
// Methods having to do with fullscreen and presentation mode. // Methods having to do with fullscreen and presentation mode.
@interface BrowserWindowController(Fullscreen) @interface BrowserWindowController(Fullscreen)
...@@ -494,13 +519,16 @@ class Command; ...@@ -494,13 +519,16 @@ class Command;
// and other UI is expected to slide. // and other UI is expected to slide.
- (BOOL)isInFullscreenWithOmniboxSliding; - (BOOL)isInFullscreenWithOmniboxSliding;
// Enters (or exits) presentation mode. // Enters presentation mode.
- (void)enterPresentationModeForURL:(const GURL&)url - (void)enterPresentationMode;
bubbleType:(FullscreenExitBubbleType)bubbleType;
// Enter fullscreen for an extension.
- (void)enterExtensionFullscreenForURL:(const GURL&)url
bubbleType:(FullscreenExitBubbleType)bubbleType;
// Enters Immersive Fullscreen for the given URL. // Enters Immersive Fullscreen for the given URL.
- (void)enterHTML5FullscreenForURL:(const GURL&)url - (void)enterWebContentFullscreenForURL:(const GURL&)url
bubbleType:(FullscreenExitBubbleType)bubbleType; bubbleType:(FullscreenExitBubbleType)bubbleType;
// Exits the current fullscreen mode. // Exits the current fullscreen mode.
- (void)exitAnyFullscreen; - (void)exitAnyFullscreen;
......
...@@ -2101,11 +2101,11 @@ willAnimateFromState:(BookmarkBar::State)oldState ...@@ -2101,11 +2101,11 @@ willAnimateFromState:(BookmarkBar::State)oldState
return presentationModeController_.get() != nil; return presentationModeController_.get() != nil;
} }
- (void)enterPresentationModeForURL:(const GURL&)url - (void)enterPresentationMode {
bubbleType:(FullscreenExitBubbleType)bubbleType { if (!chrome::mac::SupportsSystemFullscreen()) {
DCHECK(chrome::mac::SupportsSystemFullscreen()); [self enterImmersiveFullscreen];
fullscreenUrl_ = url; return;
fullscreenBubbleType_ = bubbleType; }
if ([self isInAppKitFullscreen]) { if ([self isInAppKitFullscreen]) {
// Already in AppKit Fullscreen. Adjust the UI to use Presentation Mode. // Already in AppKit Fullscreen. Adjust the UI to use Presentation Mode.
...@@ -2119,8 +2119,21 @@ willAnimateFromState:(BookmarkBar::State)oldState ...@@ -2119,8 +2119,21 @@ willAnimateFromState:(BookmarkBar::State)oldState
} }
} }
- (void)enterHTML5FullscreenForURL:(const GURL&)url - (void)enterExtensionFullscreenForURL:(const GURL&)url
bubbleType:(FullscreenExitBubbleType)bubbleType { bubbleType:(FullscreenExitBubbleType)bubbleType {
if (chrome::mac::SupportsSystemFullscreen()) {
fullscreenUrl_ = url;
fullscreenBubbleType_ = bubbleType;
[self enterPresentationMode];
} else {
[self enterImmersiveFullscreen];
DCHECK(!url.is_empty());
[self updateFullscreenExitBubbleURL:url bubbleType:bubbleType];
}
}
- (void)enterWebContentFullscreenForURL:(const GURL&)url
bubbleType:(FullscreenExitBubbleType)bubbleType {
[self enterImmersiveFullscreen]; [self enterImmersiveFullscreen];
if (!url.is_empty()) if (!url.is_empty())
[self updateFullscreenExitBubbleURL:url bubbleType:bubbleType]; [self updateFullscreenExitBubbleURL:url bubbleType:bubbleType];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment