Mac: delay window raising until space transitions.

chrome/browser/app_controller_mac.mm

Index: chrome/browser/app_controller_mac.mm
diff --git a/chrome/browser/app_controller_mac.mm b/chrome/browser/app_controller_mac.mm
index 34736e6251be73809fade4cc03caf67b8cdb177b..f1e93828cd8bbd23164ed0dbe09139e54ae19070 100644
--- a/chrome/browser/app_controller_mac.mm
+++ b/chrome/browser/app_controller_mac.mm
@@ -106,6 +106,11 @@ NSString* NSPopoverDidShowNotification = @"NSPopoverDidShowNotification";
 NSString* NSPopoverDidCloseNotification = @"NSPopoverDidCloseNotification";
 #endif
 
+// How long we allow a workspace change notification to wait to be
+// associated with a dock activation. The animation lasts 250ms. See
+// applicationShouldHandleReopen:hasVisibleWindows:.
+static const int kWorkspaceChangeTimeoutMs = 500;
+
 // True while AppController is calling chrome::NewEmptyWindow(). We need a
 // global flag here, analogue to StartupBrowserCreator::InProcessStartup()
 // because otherwise the SessionService will try to restore sessions when we
@@ -196,6 +201,7 @@ void RecordLastRunAppBundlePath() {
      withReply:(NSAppleEventDescriptor*)reply;
 - (void)submitCloudPrintJob:(NSAppleEventDescriptor*)event;
 - (void)windowLayeringDidChange:(NSNotification*)inNotification;
+- (void)activeSpaceDidChange:(NSNotification*)inNotification;
 - (void)windowChangedToProfile:(Profile*)profile;
 - (void)checkForAnyKeyWindows;
 - (BOOL)userWillWaitForInProgressDownloads:(int)downloadCount;
@@ -314,6 +320,13 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
              object:nil];
   }
 
+  // Register for space change notifications.
+  [[[NSWorkspace sharedWorkspace] notificationCenter]
+    addObserver:self
+       selector:@selector(activeSpaceDidChange:)
+           name:NSWorkspaceActiveSpaceDidChangeNotification
+         object:nil];
+
   // Set up the command updater for when there are no windows open
   [self initMenuState];
 
@@ -556,6 +569,28 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
   }
 }
 
+- (void)activeSpaceDidChange:(NSNotification*)notify {
+  if (reopenTime_.is_null() ||
+      ![NSApp isActive] ||
+      (base::TimeTicks::Now() - reopenTime_).InMilliseconds() >
+      kWorkspaceChangeTimeoutMs) {
+    return;
+  }
+
+  // The last applicationShouldHandleReopen:hasVisibleWindows: call
+  // happened during a space change. Now that the change has
+  // completed, raise browser windows.
+  reopenTime_ = base::TimeTicks();
+  std::set<NSWindow*> browserWindows;
+  for (chrome::BrowserIterator iter; !iter.done(); iter.Next()) {
+    Browser* browser = *iter;
+    browserWindows.insert(browser->window()->GetNativeWindow());
+  }
+  if (!browserWindows.empty()) {
+    ui::FocusWindowSet(browserWindows, false);
+  }
+}
+
 // Called on Lion and later when a popover (e.g. dictionary) is shown.
 - (void)popoverDidShow:(NSNotification*)notify {
   hasPopover_ = YES;
@@ -1082,9 +1117,18 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
       browserWindows.insert(browser->window()->GetNativeWindow());
     }
     if (!browserWindows.empty()) {
-      ui::FocusWindowSet(browserWindows, false);
-      // Return NO; we've done the unminimize, so AppKit shouldn't do
-      // anything.
+      NSWindow* keyWindow = [NSApp keyWindow];
+      if (keyWindow && ![keyWindow isOnActiveSpace]) {
+        // The key window is not on the active space. We must be
+        // mid-animation for a space change triggered by the
+        // dock. Delay the call to ui::FocusWindowSet until the change
+        // completes.
+        reopenTime_ = base::TimeTicks::Now();

[davidben, 2013/10/23 16:04:45]

Not calling FocusWindowSet here is apparently necessary. If it's called both
here and in activeSpaceDidChange, the focus gets really confused. If the omnibox
is focused, first keypress goes to the wrong window and switches space. Future
keypresses go to the right window which is now hidden and don't trigger a space
change.

+      } else {
+        ui::FocusWindowSet(browserWindows, false);
+      }
+      // Return NO; we've done (or soon will do) the deminiaturize, so
+      // AppKit shouldn't do anything.
       return NO;
     }
   }