Mac: Fix window raising for multiple desktops

ui/base/cocoa/focus_window_set.mm

Index: ui/base/cocoa/focus_window_set.mm
diff --git a/ui/base/cocoa/focus_window_set.mm b/ui/base/cocoa/focus_window_set.mm
index 553db6fe3be597fe14f157d8623852e1f714d14e..e260627d3cab39d8bccbc671a35578828c35398f 100644
--- a/ui/base/cocoa/focus_window_set.mm
+++ b/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.

[Nico, 2013/09/04 15:50:09]

Would using the private api CGSGetWindowWorkspace() allow fixing this problem?

[davidben, 2013/09/04 16:05:58]

Probably not? Is there a way to figure out where OS X decided to switch desktops
to, so we have something to compare against? The problem is that we don't know
at the time FocusWindowSet is currently called whether OS X triggered its
desktop switching logic. (And that logic is dependent on a user setting, so
trying to fake it ourselves seems a bad plan.) I'm not familiar enough with OS X
to know if there's another hook we should be using instead.

Fortunately, raising the wrong desktop's windows *while another workspace switch
is already happening* seems to not trigger an additional workspace switch. I'm
not too happy to be relying on that, but that's what I mean by "This seems to
still work okay."

What does trigger a second switch is deminiaturizing during a workspace switch.
But at that point, we've already lost. We want that second switch. The real
trouble is that OS X's desktop switching logic doesn't actually know that the
app windows are logically a different app from the browser. It sees that we're
on both spaces 1 and 2 and decides to go into "cycle between workspaces with
this app" mode and switches to space 1. But space 1 only contains an app window,
not a browser window, so we should never have switched to it in the first place.

I suspect we can't actually behave completely properly unless the app shims
actually host the app's NSWindow. Otherwise we're always cleaning up after the
default window server behavior, and sometimes we confuse it. But I imagine
separating the browser process from the NSWindow host would require quite a bit
of work.

[davidben, 2013/09/04 16:19:16]

Thinking about that some more, another explanation is that it's not that a
workspace switch is in progress but that, at the time, it is actually still the
current workspace (by whatever relevant metric) and doesn't trigger a second
switch. I'm slightly happier relying on that one. But that doesn't explain why
deminitiarizing a window on the (previously) current workspace does trigger a
switch back. Maybe terrible races with the fact that some things are animated
and some things aren't...

*shrug* Secretly, I have no idea how anything on Mac works. I just spent a bunch
of time experimenting with TextEdit's and our behavior here. :-)

+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];