Commit f19bb479 authored by Avi Drissman's avatar Avi Drissman Committed by Commit Bot

Move the quit confirmation dialog earlier.

This avoids it getting triggered by URL requests to quit/restart.

BUG=909592, 142944

Change-Id: I353c1334decd556554399349bb207e63fca1352b
Reviewed-on: https://chromium-review.googlesource.com/c/1354399Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612323}
parent 69c21290
...@@ -123,6 +123,10 @@ class ScopedKeepAlive; ...@@ -123,6 +123,10 @@ class ScopedKeepAlive;
// window closure from causing the application to quit. // window closure from causing the application to quit.
- (void)stopTryingToTerminateApplication:(NSApplication*)app; - (void)stopTryingToTerminateApplication:(NSApplication*)app;
// Run the quit confirmation panel and return whether or not to continue
// quitting.
- (BOOL)runConfirmQuitPanel;
// Indicate that the system is powering off or logging out. // Indicate that the system is powering off or logging out.
- (void)willPowerOff:(NSNotification*)inNotification; - (void)willPowerOff:(NSNotification*)inNotification;
......
...@@ -463,12 +463,6 @@ static base::mac::ScopedObjCClassSwizzler* g_swizzle_imk_input_session; ...@@ -463,12 +463,6 @@ static base::mac::ScopedObjCClassSwizzler* g_swizzle_imk_input_session;
// them in, but I'm not sure about UX; we'd also want to disable other things // them in, but I'm not sure about UX; we'd also want to disable other things
// though.) http://crbug.com/40861 // though.) http://crbug.com/40861
// Check if the user really wants to quit by employing the confirm-to-quit
// mechanism.
if (!browser_shutdown::IsTryingToQuit() &&
[self applicationShouldTerminate:app] != NSTerminateNow)
return NO;
// Check for active apps. If quitting is prevented, only close browsers and // Check for active apps. If quitting is prevented, only close browsers and
// sessions. // sessions.
if (!browser_shutdown::IsTryingToQuit() && !isPoweringOff && if (!browser_shutdown::IsTryingToQuit() && !isPoweringOff &&
...@@ -519,30 +513,26 @@ static base::mac::ScopedObjCClassSwizzler* g_swizzle_imk_input_session; ...@@ -519,30 +513,26 @@ static base::mac::ScopedObjCClassSwizzler* g_swizzle_imk_input_session;
} }
} }
- (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication*)app { - (BOOL)runConfirmQuitPanel {
// If there are no windows, quit immediately. // If there are no windows, quit immediately.
if (BrowserList::GetInstance()->empty() && if (BrowserList::GetInstance()->empty() &&
!AppWindowRegistryUtil::IsAppWindowVisibleInAnyProfile(0)) { !AppWindowRegistryUtil::IsAppWindowVisibleInAnyProfile(0)) {
return NSTerminateNow; return YES;
} }
// Check if the preference is turned on. // Check if the preference is turned on.
const PrefService* prefs = g_browser_process->local_state(); const PrefService* prefs = g_browser_process->local_state();
if (!prefs->GetBoolean(prefs::kConfirmToQuitEnabled)) { if (!prefs->GetBoolean(prefs::kConfirmToQuitEnabled)) {
confirm_quit::RecordHistogram(confirm_quit::kNoConfirm); confirm_quit::RecordHistogram(confirm_quit::kNoConfirm);
return NSTerminateNow; return YES;
} }
// If the application is going to terminate as the result of a Cmd+Q // Run only for keyboard-initiated quits.
// invocation, use the special sauce to prevent accidental quitting. if ([[NSApp currentEvent] type] != NSKeyDown)
// http://dev.chromium.org/developers/design-documents/confirm-to-quit-experiment
// This logic is only for keyboard-initiated quits.
if ([[app currentEvent] type] != NSKeyDown)
return NSTerminateNow; return NSTerminateNow;
return [[ConfirmQuitPanelController sharedController] return [[ConfirmQuitPanelController sharedController]
runModalLoopForApplication:app]; runModalLoopForApplication:NSApp];
} }
// Called when the app is shutting down. Clean-up as appropriate. // Called when the app is shutting down. Clean-up as appropriate.
......
...@@ -15,7 +15,7 @@ BrowserProcessPlatformPart::BrowserProcessPlatformPart() { ...@@ -15,7 +15,7 @@ BrowserProcessPlatformPart::BrowserProcessPlatformPart() {
BrowserProcessPlatformPart::~BrowserProcessPlatformPart() { BrowserProcessPlatformPart::~BrowserProcessPlatformPart() {
} }
void BrowserProcessPlatformPart::AttemptExit() { void BrowserProcessPlatformPart::AttemptExit(bool try_to_quit_application) {
// Tell the Java code to finish() the Activity. // Tell the Java code to finish() the Activity.
chrome::TerminateAndroid(); chrome::TerminateAndroid();
} }
...@@ -15,7 +15,7 @@ class BrowserProcessPlatformPart : public BrowserProcessPlatformPartBase { ...@@ -15,7 +15,7 @@ class BrowserProcessPlatformPart : public BrowserProcessPlatformPartBase {
~BrowserProcessPlatformPart() override; ~BrowserProcessPlatformPart() override;
// Overridden from BrowserProcessPlatformPartBase: // Overridden from BrowserProcessPlatformPartBase:
void AttemptExit() override; void AttemptExit(bool try_to_quit_application) override;
private: private:
DISALLOW_COPY_AND_ASSIGN(BrowserProcessPlatformPart); DISALLOW_COPY_AND_ASSIGN(BrowserProcessPlatformPart);
......
...@@ -23,7 +23,7 @@ void BrowserProcessPlatformPartBase::PlatformSpecificCommandLineProcessing( ...@@ -23,7 +23,7 @@ void BrowserProcessPlatformPartBase::PlatformSpecificCommandLineProcessing(
void BrowserProcessPlatformPartBase::StartTearDown() { void BrowserProcessPlatformPartBase::StartTearDown() {
} }
void BrowserProcessPlatformPartBase::AttemptExit() { void BrowserProcessPlatformPartBase::AttemptExit(bool try_to_quit_application) {
// chrome::CloseAllBrowsers() doesn't link on OS_ANDROID, but it overrides this // chrome::CloseAllBrowsers() doesn't link on OS_ANDROID, but it overrides this
// method already. // method already.
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
......
...@@ -34,7 +34,7 @@ class BrowserProcessPlatformPartBase { ...@@ -34,7 +34,7 @@ class BrowserProcessPlatformPartBase {
virtual void StartTearDown(); virtual void StartTearDown();
// Called from AttemptExitInternal(). // Called from AttemptExitInternal().
virtual void AttemptExit(); virtual void AttemptExit(bool try_to_quit_application);
// Called at the end of BrowserProcessImpl::PreMainMessageLoopRun(). // Called at the end of BrowserProcessImpl::PreMainMessageLoopRun().
virtual void PreMainMessageLoopRun(); virtual void PreMainMessageLoopRun();
......
...@@ -19,7 +19,7 @@ class BrowserProcessPlatformPart : public BrowserProcessPlatformPartBase { ...@@ -19,7 +19,7 @@ class BrowserProcessPlatformPart : public BrowserProcessPlatformPartBase {
// Overridden from BrowserProcessPlatformPartBase: // Overridden from BrowserProcessPlatformPartBase:
void StartTearDown() override; void StartTearDown() override;
void AttemptExit() override; void AttemptExit(bool try_to_quit_application) override;
void PreMainMessageLoopRun() override; void PreMainMessageLoopRun() override;
AppShimHostManager* app_shim_host_manager(); AppShimHostManager* app_shim_host_manager();
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
#include "chrome/browser/browser_process_platform_part_mac.h" #include "chrome/browser/browser_process_platform_part_mac.h"
#include "base/mac/foundation_util.h"
#import "chrome/browser/app_controller_mac.h"
#include "chrome/browser/chrome_browser_application_mac.h" #include "chrome/browser/chrome_browser_application_mac.h"
BrowserProcessPlatformPart::BrowserProcessPlatformPart() { BrowserProcessPlatformPart::BrowserProcessPlatformPart() {
...@@ -16,10 +18,26 @@ void BrowserProcessPlatformPart::StartTearDown() { ...@@ -16,10 +18,26 @@ void BrowserProcessPlatformPart::StartTearDown() {
app_shim_host_manager_ = NULL; app_shim_host_manager_ = NULL;
} }
void BrowserProcessPlatformPart::AttemptExit() { void BrowserProcessPlatformPart::AttemptExit(bool try_to_quit_application) {
// On the Mac, the application continues to run once all windows are closed. // On the Mac, the application continues to run once all windows are closed.
// Terminate will result in a CloseAllBrowsers() call, and once (and if) // Terminate will result in a CloseAllBrowsers() call, and once (and if)
// that is done, will cause the application to exit cleanly. // that is done, will cause the application to exit cleanly.
//
// This function is called for two types of attempted exits: URL requests
// (chrome://quit or chrome://restart), and a keyboard menu invocations of
// command-Q. (Interestingly, selecting the Quit command with the mouse don't
// come down this code path at all.) URL requests to exit have
// |try_to_quit_application| set to true; keyboard menu invocations have it
// set to false.
if (!try_to_quit_application) {
// A keyboard menu invocation.
AppController* app_controller =
base::mac::ObjCCastStrict<AppController>([NSApp delegate]);
if (![app_controller runConfirmQuitPanel])
return;
}
chrome_browser_application_mac::Terminate(); chrome_browser_application_mac::Terminate();
} }
......
...@@ -149,7 +149,7 @@ void AttemptExitInternal(bool try_to_quit_application) { ...@@ -149,7 +149,7 @@ void AttemptExitInternal(bool try_to_quit_application) {
content::NotificationService::AllSources(), content::NotificationService::AllSources(),
content::NotificationService::NoDetails()); content::NotificationService::NoDetails());
g_browser_process->platform_part()->AttemptExit(); g_browser_process->platform_part()->AttemptExit(try_to_quit_application);
} }
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
......
...@@ -22,9 +22,8 @@ ...@@ -22,9 +22,8 @@
+ (ConfirmQuitPanelController*)sharedController; + (ConfirmQuitPanelController*)sharedController;
// Runs a modal loop that brings up the panel and handles the logic for if and // Runs a modal loop that brings up the panel and handles the logic for if and
// when to terminate. Returns NSApplicationTerminateReply for use in // when to terminate. Returns YES if the quit should continue.
// -[NSApplicationDelegate applicationShouldTerminate:]. - (BOOL)runModalLoopForApplication:(NSApplication*)app;
- (NSApplicationTerminateReply)runModalLoopForApplication:(NSApplication*)app;
// Shows the window. // Shows the window.
- (void)showWindow:(id)sender; - (void)showWindow:(id)sender;
......
...@@ -198,7 +198,7 @@ ConfirmQuitPanelController* g_confirmQuitPanelController = nil; ...@@ -198,7 +198,7 @@ ConfirmQuitPanelController* g_confirmQuitPanelController = nil;
return self; return self;
} }
- (NSApplicationTerminateReply)runModalLoopForApplication:(NSApplication*)app { - (BOOL)runModalLoopForApplication:(NSApplication*)app {
base::scoped_nsobject<ConfirmQuitPanelController> keepAlive([self retain]); base::scoped_nsobject<ConfirmQuitPanelController> keepAlive([self retain]);
// If this is the second of two such attempts to quit within a certain time // If this is the second of two such attempts to quit within a certain time
...@@ -227,7 +227,7 @@ ConfirmQuitPanelController* g_confirmQuitPanelController = nil; ...@@ -227,7 +227,7 @@ ConfirmQuitPanelController* g_confirmQuitPanelController = nil;
confirm_quit::RecordHistogram(confirm_quit::kDoubleTap); confirm_quit::RecordHistogram(confirm_quit::kDoubleTap);
else else
confirm_quit::RecordHistogram(confirm_quit::kTapHold); confirm_quit::RecordHistogram(confirm_quit::kTapHold);
return NSTerminateNow; return YES;
} else { } else {
[lastQuitAttempt release]; // Harmless if already nil. [lastQuitAttempt release]; // Harmless if already nil.
lastQuitAttempt = [timeNow retain]; // Record this attempt for next time. lastQuitAttempt = [timeNow retain]; // Record this attempt for next time.
...@@ -277,16 +277,16 @@ ConfirmQuitPanelController* g_confirmQuitPanelController = nil; ...@@ -277,16 +277,16 @@ ConfirmQuitPanelController* g_confirmQuitPanelController = nil;
// The user held down the combination long enough that quitting should // The user held down the combination long enough that quitting should
// happen. // happen.
confirm_quit::RecordHistogram(confirm_quit::kHoldDuration); confirm_quit::RecordHistogram(confirm_quit::kHoldDuration);
return NSTerminateNow; return YES;
} else { } else {
// Slowly fade the confirm window out in case the user doesn't // Slowly fade the confirm window out in case the user doesn't
// understand what they have to do to quit. // understand what they have to do to quit.
[self dismissPanel]; [self dismissPanel];
return NSTerminateCancel; return NO;
} }
// Default case: terminate. // Default case: terminate.
return NSTerminateNow; return YES;
} }
- (void)windowWillClose:(NSNotification*)notif { - (void)windowWillClose:(NSNotification*)notif {
......
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