Mac: Fix window raising for multiple desktops

When raising a set of windows in response to a dock icon click, only raise the
ones on the currently active space. We currently raise all of them and switch
spaces haphazardly.

Also tidy up the applicationShouldHandleReopen:hasVisibileWindows: callback. We
should return NO since we've reacted and don't need AppKit to deminiaturize for
us. (Although it seems to not make much difference, probably because we've
already picked a window to deminiaturize, so AppKit won't pick a second.) Also
remove the tabbed/popup check. As of r192264, those are the only browser window
types.

BUG=281674
TEST=Open a browser window in Desktop 1 and one in Desktop 2.
Switch to Desktop 2.
Open another application in Desktop 2 and make it foreground.
Click the dock icon for Chrome. Should remain on Desktop 2.

Review URL: https://chromiumcodereview.appspot.com/23737003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@221495 0039d316-1c4b-4281-b951-d872f2087c98

apps/app_shim/extension_app_shim_handler_mac.cc

@@ -297,7 +297,11 @@ void ExtensionAppShimHandler::OnShimFocus(Host* host,
     native_windows.insert((*it)->GetNativeWindow());
   }
   if (!native_windows.empty()) {
-    ui::FocusWindowSet(native_windows);
+    // Allow workspace switching. For the browser process, we can
+    // reasonably rely on OS X to switch spaces for us and honor
+    // relevant user settings. But shims don't have windows, so we
+    // have to do it ourselves.
+    ui::FocusWindowSet(native_windows, true);
     return;
   }
 

chrome/browser/app_controller_mac.mm

@@ -1071,20 +1071,20 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
   // Bring all browser windows to the front. Specifically, this brings them in
   // front of any app windows. FocusWindowSet will also unminimize the most
   // recently minimized window if no windows in the set are visible.
-  // If there are tabbed or popup windows, return here. Otherwise, the windows
-  // are panels or notifications so we still need to open a new window.
+  // If there are any, return here. Otherwise, the windows are panels or
+  // notifications so we still need to open a new window.
   if (hasVisibleWindows) {
-    BOOL foundBrowser = NO;
     std::set<NSWindow*> browserWindows;
     for (chrome::BrowserIterator iter; !iter.done(); iter.Next()) {
       Browser* browser = *iter;
       browserWindows.insert(browser->window()->GetNativeWindow());
-      if (browser->is_type_tabbed() || browser->is_type_popup())
-        foundBrowser = YES;
     }
-    ui::FocusWindowSet(browserWindows);
-    if (foundBrowser)
-      return YES;
+    if (!browserWindows.empty()) {
+      ui::FocusWindowSet(browserWindows, false);
+      // Return NO; we've done the unminimize, so AppKit shouldn't do
+      // anything.
+      return NO;
+    }
   }
 
   // If launched as a hidden login item (due to installation of a persistent app

chrome/browser/app_controller_mac_browsertest.mm

@@ -48,9 +48,11 @@ IN_PROC_BROWSER_TEST_F(AppControllerPlatformAppBrowserTest,
   base::scoped_nsobject<AppController> ac([[AppController alloc] init]);
   NSUInteger old_window_count = [[NSApp windows] count];
   EXPECT_EQ(1u, active_browser_list_->size());
-  BOOL result = [ac applicationShouldHandleReopen:NSApp hasVisibleWindows:YES];
+  [ac applicationShouldHandleReopen:NSApp hasVisibleWindows:YES];
+  // We do not EXPECT_TRUE the result here because the method
+  // deminiaturizes windows manually rather than return YES and have
+  // AppKit do it.
 
-  EXPECT_TRUE(result);
   EXPECT_EQ(old_window_count, [[NSApp windows] count]);
   EXPECT_EQ(1u, active_browser_list_->size());
 }

ui/base/cocoa/focus_window_set.h

@@ -15,7 +15,8 @@ namespace ui {
 // Brings a group of windows to the front without changing their order, and
 // makes the frontmost one key and main. If none are visible, the frontmost
 // miniaturized window is deminiaturized.
-UI_EXPORT void FocusWindowSet(const std::set<gfx::NativeWindow>& windows);
+UI_EXPORT void FocusWindowSet(const std::set<gfx::NativeWindow>& windows,
+                              bool allow_workspace_switch);
 
 }  // namespace ui
 

ui/base/cocoa/focus_window_set.mm

@@ -8,25 +8,56 @@
 
 namespace ui {
 
-void FocusWindowSet(const std::set<NSWindow*>& windows) {
+// This attempts to match OS X's native behavior, namely that a window
+// is only ever deminiaturized if ALL windows on ALL workspaces are
+// miniaturized. (This callback runs before AppKit picks its own
+// window to deminiaturize, so we get to pick one from the right set.)
+//
+// In addition, limit to the windows on the current
+// workspace. Otherwise we jump spaces haphazardly.
+//
+// NOTE: This is not perfect. If clicking the dock icon resulted in
+// switching spaces, isOnActiveSpace gives the answer for the PREVIOUS
+// space. This means that we actually raise the wrong space's
+// windows. This seems to still work okay.
+//
+// However, if we decide to deminiaturize a window instead, that
+// results in switching spaces and switching back. Fortunately, this
+// only happens if, say, space 1 contains an app, space 2 contains a
+// miniaturized browser. We click the icon, OS X switches to space 1,
+// we deminiaturize the browser, and that triggers switching back.
+void FocusWindowSet(const std::set<NSWindow*>& windows,
+                    bool allow_workspace_switch) {
   NSArray* ordered_windows = [NSApp orderedWindows];
   NSWindow* frontmost_window = nil;
+  NSWindow* frontmost_window_all_spaces = nil;
   NSWindow* frontmost_miniaturized_window = nil;
+  bool all_miniaturized = true;
   for (int i = [ordered_windows count] - 1; i >= 0; i--) {
     NSWindow* win = [ordered_windows objectAtIndex:i];
     if (windows.find(win) != windows.end()) {
-      if ([win isVisible]) {
-        [win orderFront:nil];
-        frontmost_window = win;
-      } else if ([win isMiniaturized]) {
+      if ([win isMiniaturized]) {
         frontmost_miniaturized_window = win;
+      } else if ([win isVisible]) {
+        all_miniaturized = false;
+        frontmost_window_all_spaces = win;
+        if ([win isOnActiveSpace]) {
+          [win orderFront:nil];
+          frontmost_window = win;
+        }
       }
     }
   }
-  if (!frontmost_window && frontmost_miniaturized_window) {
+  if (all_miniaturized && frontmost_miniaturized_window) {
     [frontmost_miniaturized_window deminiaturize:nil];
     frontmost_window = frontmost_miniaturized_window;
   }
+  // If we couldn't find one on this window, consider all spaces.
+  if (allow_workspace_switch &&
+      !frontmost_window && frontmost_window_all_spaces) {
+    frontmost_window = frontmost_window_all_spaces;
+    [frontmost_window orderFront:nil];
+  }
   if (frontmost_window) {
     [NSApp activateIgnoringOtherApps:YES];
     [frontmost_window makeMainWindow];