Commit 464fae69 authored by Christopher Cameron's avatar Christopher Cameron Committed by Commit Bot

RemoteMacViews: Fix hang at command-Q

When we call -[NSApp terminate] in the app shim process, this does
not immediately terminate. Rather, it
 0. Sends the message appShimController_->host()->QuitApp() and waits
    for a reply before actually terminating.
 1. In the browser, this call hits ExtensionAppShimHandler::
    CloseBrowsersForApp
 2. Which will end up calling NativeWidgetMac::Close
 3. Which will then politely request that the app shim process close
    the corresponding window via
    views_bridge_mac::mojom::BridgedNativeWidget::CloseWindow
 4. Which will send an ack back to the browser via
    views_bridge_mac::mojom::BridgedNativeWidgetHost::OnWindowHasClosed
 5. Whereupon the browser window will be registered as closed, and a
    close message will be sent to the app shim, and the wait for
    termination will break

This ends up not working at step 3. Our suspended terminate ends up
running a nested message loop inside a mojo message handler, and so
it will never execute the CloseWindow message, and therefore never
close.

Change the logic at [0] to send the QuitApp message, and then
immediately terminate the application.

Bug: 902583
Change-Id: I60fd7b5fdf9f145369d06219e8ebef36cdcd0f44
Reviewed-on: https://chromium-review.googlesource.com/c/1364220Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614922}
parent 3977f0b2
...@@ -201,7 +201,7 @@ void AppShimController::SetUserAttention( ...@@ -201,7 +201,7 @@ void AppShimController::SetUserAttention(
} }
void AppShimController::Close() { void AppShimController::Close() {
[delegate_ terminateNow]; [NSApp terminate:nil];
} }
bool AppShimController::SendFocusApp(apps::AppShimFocusType focus_type, bool AppShimController::SendFocusApp(apps::AppShimFocusType focus_type,
......
...@@ -18,8 +18,6 @@ class AppShimController; ...@@ -18,8 +18,6 @@ class AppShimController;
: NSObject<NSApplicationDelegate, NSUserInterfaceValidations> { : NSObject<NSApplicationDelegate, NSUserInterfaceValidations> {
@private @private
AppShimController* appShimController_; // Weak, initially NULL. AppShimController* appShimController_; // Weak, initially NULL.
BOOL terminateNow_;
BOOL terminateRequested_;
std::vector<base::FilePath> filesToOpenAtStartup_; std::vector<base::FilePath> filesToOpenAtStartup_;
} }
...@@ -36,10 +34,6 @@ class AppShimController; ...@@ -36,10 +34,6 @@ class AppShimController;
// Takes an array of NSString*. // Takes an array of NSString*.
- (void)openFiles:(NSArray*)filename; - (void)openFiles:(NSArray*)filename;
// Terminate immediately. This is necessary as we override terminate: to send
// a QuitApp message.
- (void)terminateNow;
@end @end
#endif // CHROME_APP_SHIM_APP_SHIM_DELEGATE_H_ #endif // CHROME_APP_SHIM_APP_SHIM_DELEGATE_H_
...@@ -67,13 +67,11 @@ ...@@ -67,13 +67,11 @@
- (NSApplicationTerminateReply)applicationShouldTerminate: - (NSApplicationTerminateReply)applicationShouldTerminate:
(NSApplication*)sender { (NSApplication*)sender {
if (terminateNow_ || !appShimController_) // Send a last message to the host indicating that the host should close all
return NSTerminateNow; // associated browser windows.
if (appShimController_)
appShimController_->host()->QuitApp(); appShimController_->host()->QuitApp();
// Wait for the channel to close before terminating. return NSTerminateNow;
terminateRequested_ = YES;
return NSTerminateLater;
} }
- (void)applicationWillHide:(NSNotification*)notification { - (void)applicationWillHide:(NSNotification*)notification {
...@@ -86,16 +84,6 @@ ...@@ -86,16 +84,6 @@
appShimController_->host()->SetAppHidden(false); appShimController_->host()->SetAppHidden(false);
} }
- (void)terminateNow {
if (terminateRequested_) {
[NSApp replyToApplicationShouldTerminate:NSTerminateNow];
return;
}
terminateNow_ = YES;
[NSApp terminate:nil];
}
- (BOOL)validateUserInterfaceItem:(id<NSValidatedUserInterfaceItem>)item { - (BOOL)validateUserInterfaceItem:(id<NSValidatedUserInterfaceItem>)item {
return NO; return NO;
} }
......
...@@ -1037,12 +1037,7 @@ bool BridgedNativeWidgetImpl::ShouldRunCustomAnimationFor( ...@@ -1037,12 +1037,7 @@ bool BridgedNativeWidgetImpl::ShouldRunCustomAnimationFor(
} }
bool BridgedNativeWidgetImpl::RedispatchKeyEvent(NSEvent* event) { bool BridgedNativeWidgetImpl::RedispatchKeyEvent(NSEvent* event) {
NSWindow* window = ns_window(); return [[window_ commandDispatcher] redispatchKeyEvent:event];
DCHECK([window.class conformsToProtocol:@protocol(CommandDispatchingWindow)]);
NSObject<CommandDispatchingWindow>* command_dispatching_window =
base::mac::ObjCCastStrict<NSObject<CommandDispatchingWindow>>(window);
return
[[command_dispatching_window commandDispatcher] redispatchKeyEvent:event];
} }
NSWindow* BridgedNativeWidgetImpl::ns_window() { NSWindow* BridgedNativeWidgetImpl::ns_window() {
......
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