Mac: Give packaged app windows their own window cycle list.

For now it's behind chrome://flags#app-window-cycling.

This is to hopefully give a better UX for App Shims. It also solves a
tricky problem with Chrome Remote Desktop: Since CRD swallows Cmd+`, it
breaks browser window cycling since the cycling will stop as soon as a
CRD window is encountered.

This works by hooking in to the same mechanism that changes the menu
bar. Switching to or away from a packaged app window will now also update
the window cycle list. The CL intentionally does *not*:
 - try to distinguish between the same app running in different profiles,
 - update the `Window` menu, nor
 - try to do anything for Cmd+Tab switching.

The hard part: AppKit does not immediately apply changes to
NSWindowCollectionBehaviorIgnoresCycle after a window is already shown.
An effective workaround seems to be call to [[NSApp keyWindow]
makeKeyAndOrderFront:nil]. However, this may have other consequences.
Lots of other things were tried (see code comments).

Adds an interactive UI test. Tests on mac using
ui_test_utils::ShowAndFocusNativeWindow(..) have a history of flakiness,
so this CL attempts to address that by combining with a notification
observer. This approach has successfully addressed flakiness around
key-window transitions elsewhere. It would commonly occur when multiple
windows are in the process -- a single RunLoop().RunUntilIdle() couldn't
guarantee that the window server had completed the switch.

BUG=484737, 278408

Review URL: https://codereview.chromium.org/1186803003

Cr-Commit-Position: refs/heads/master@{#336063}

12 files changed, 369 insertions, 55 deletions

diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 4e9f95b..365fdd3 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -6963,6 +6963,12 @@ Keep your key file in a safe place. You will need it to create new versions of y
         <message name="IDS_FLAGS_MAC_VIEWS_NATIVE_APP_WINDOWS_DESCRIPTION" desc="Description of flag to enable or disable toolkit-views Chrome App windows on Mac.">
           Controls whether to use Toolkit-Views based Chrome App windows.
         </message>
+        <message name="IDS_FLAGS_APP_WINDOW_CYCLING_NAME" desc="Name of the flag to enable or disable custom Cmd+` App window cycling on Mac.">
+          Custom Window Cycling for Chrome Apps.
+        </message>
+        <message name="IDS_FLAGS_APP_WINDOW_CYCLING_DESCRIPTION" desc="Description of flag to enable or disable custom Cmd+` App window cycling on Mac.">
+          Changes the behavior of Cmd+` when a Chrome App becomes active. When enabled, Chrome Apps will not be cycled when Cmd+` is pressed from a browser window, and browser windows will not be cycled when a Chrome App is active.
+        </message>
       </if>
 
       <!-- Crashes -->
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index 8ec1ffb..4a20747 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -2002,6 +2002,12 @@ const Experiment kExperiments[] = {
      kOsMac,
      ENABLE_DISABLE_VALUE_TYPE(switches::kEnableMacViewsNativeAppWindows,
                                switches::kDisableMacViewsNativeAppWindows)},
+    {"app-window-cycling",
+     IDS_FLAGS_APP_WINDOW_CYCLING_NAME,
+     IDS_FLAGS_APP_WINDOW_CYCLING_DESCRIPTION,
+     kOsMac,
+     ENABLE_DISABLE_VALUE_TYPE(switches::kEnableAppWindowCycling,
+                               switches::kDisableAppWindowCycling)},
 #endif
 #if defined(ENABLE_WEBVR)
     {"enable-webvr",
diff --git a/chrome/browser/ui/cocoa/apps/app_shim_menu_controller_mac.h b/chrome/browser/ui/cocoa/apps/app_shim_menu_controller_mac.h
index f7428c4..4a07794 100644
--- a/chrome/browser/ui/cocoa/apps/app_shim_menu_controller_mac.h
+++ b/chrome/browser/ui/cocoa/apps/app_shim_menu_controller_mac.h
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_UI_COCOA_APPS_APP_SHIM_MENU_CONTROLLER_MAC_H_
 
 #import <Cocoa/Cocoa.h>
+#include <string>
 
 #include "base/mac/scoped_nsobject.h"
 
@@ -20,7 +21,7 @@
 @interface AppShimMenuController : NSObject {
  @private
   // The extension id of the currently focused packaged app.
-  base::scoped_nsobject<NSString> appId_;
+  std::string appId_;
   // Items that need a doppelganger.
   base::scoped_nsobject<DoppelgangerMenuItem> aboutDoppelganger_;
   base::scoped_nsobject<DoppelgangerMenuItem> hideDoppelganger_;
diff --git a/chrome/browser/ui/cocoa/apps/app_shim_menu_controller_mac.mm b/chrome/browser/ui/cocoa/apps/app_shim_menu_controller_mac.mm
index 1ff2ec2..efb3485 100644
--- a/chrome/browser/ui/cocoa/apps/app_shim_menu_controller_mac.mm
+++ b/chrome/browser/ui/cocoa/apps/app_shim_menu_controller_mac.mm
@@ -4,6 +4,7 @@
 
 #import "chrome/browser/ui/cocoa/apps/app_shim_menu_controller_mac.h"
 
+#include "base/command_line.h"
 #include "base/mac/scoped_nsautorelease_pool.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
@@ -14,14 +15,33 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #import "chrome/browser/ui/cocoa/apps/native_app_window_cocoa.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/grit/generated_resources.h"
 #include "extensions/browser/app_window/app_window.h"
 #include "extensions/common/extension.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/l10n/l10n_util_mac.h"
 
+using extensions::Extension;
+
 namespace {
 
+// When an app window loses main status, AppKit may make another app window main
+// instead. Rather than trying to predict what AppKit will do (which is hard),
+// just protect against changes in the event queue that will clobber each other.
+int g_window_cycle_sequence_number = 0;
+
+// Whether Custom Cmd+` window cycling is enabled for apps.
+bool IsAppWindowCyclingEnabled() {
+  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(switches::kDisableAppWindowCycling))
+    return false;
+  if (command_line->HasSwitch(switches::kEnableAppWindowCycling))
+    return true;
+
+  return false;  // Current default.
+}
+
 // Gets an item from the main menu given the tag of the top level item
 // |menu_tag| and the tag of the item |item_tag|.
 NSMenuItem* GetItemByTag(NSInteger menu_tag, NSInteger item_tag) {
@@ -113,6 +133,100 @@ void SetItemWithTagVisible(NSMenuItem* top_level_item,
   [menu_item setHidden:!visible];
 }
 
+// Return the Extension (if any) associated with the given window. If it is not
+// a platform app nor hosted app, but it is a browser, |is_browser| will be set
+// to true (otherwise false).
+const Extension* GetExtensionForNSWindow(NSWindow* window, bool* is_browser) {
+  const Extension* extension = nullptr;
+  Browser* browser = nullptr;
+
+  extensions::AppWindow* app_window =
+      AppWindowRegistryUtil::GetAppWindowForNativeWindowAnyProfile(window);
+  if (app_window) {
+    extension = app_window->GetExtension();
+  } else {
+    // If there is no corresponding AppWindow, this could be a hosted app, so
+    // check for a browser.
+    browser = chrome::FindBrowserWithWindow(window);
+    extension = apps::ExtensionAppShimHandler::MaybeGetAppForBrowser(browser);
+  }
+
+  *is_browser = extension == nullptr && browser != nullptr;
+  return extension;
+}
+
+// Sets or clears NSWindowCollectionBehaviorIgnoresCycle for |window|. Does not
+// change NSWindowCollectionBehaviorParticipatesInCycle. That exists, e.g, for
+// an NSPanel to override its default behavior, but this should only ever be
+// called for Browser windows and App windows (which are not panels).
+bool SetWindowParticipatesInCycle(NSWindow* window, bool participates) {
+  const NSWindowCollectionBehavior past_behavior = [window collectionBehavior];
+  NSWindowCollectionBehavior behavior = past_behavior;
+  if (participates)
+    behavior &= ~NSWindowCollectionBehaviorIgnoresCycle;
+  else
+    behavior |= NSWindowCollectionBehaviorIgnoresCycle;
+
+  // Often, there is no change. AppKit has no early exit since the value is
+  // derived partially from styleMask and other things, so do our own.
+  if (behavior == past_behavior)
+    return false;
+
+  [window setCollectionBehavior:behavior];
+  return true;
+}
+
+// Sets the window cycle list to |app_id|'s windows only.
+void SetAppCyclesWindows(const std::string& app_id, int sequence_number) {
+  if (g_window_cycle_sequence_number != sequence_number)
+    return;
+
+  bool any_change = false;
+  for (NSWindow* window : [NSApp windows]) {
+    bool is_browser;
+    const Extension* extension = GetExtensionForNSWindow(window, &is_browser);
+    if (extension && extension->id() == app_id)
+      any_change |= SetWindowParticipatesInCycle(window, true);
+    else if (extension || is_browser)
+      any_change |= SetWindowParticipatesInCycle(window, false);
+  }
+
+  // Without the following, -[NSApplication _getLockedWindowListForCycle] will
+  // happily return windows that were just set to ignore window cycling. Doing
+  // this seems to trick AppKit into updating the window cycle list. But it is a
+  // bit scary, so avoid it when there is no change. These attempts were based
+  // on the observation that clicking a window twice to switch focus would
+  // always work. Also tried (without luck):
+  //  - [NSApp setWindowsNeedUpdate:YES],
+  //  - Creating a deferred NSWindow and immediately releasing it,
+  //  - Calling private methods like [NSApp _unlockWindowListForCycle],
+  //  - [NSApp postEvent:[NSEvent otherEventWithType:NSApplicationDefined...
+  //      (an attempt to tickle AppKit into an update of some kind),
+  //  - Calling synchronously (i.e. not via PostTask) <- this was actually the
+  //      initial attempt. Then, switching to PostTask didn't help with this
+  //      quirk, but was useful for the sequence number stuff, and
+  //  - Re-ordering collection behavior changes to ensure one window was always
+  //      participating (i.e. all 'adds' before any 'removes').
+  if (any_change)
+    [[NSApp keyWindow] makeKeyAndOrderFront:nil];
+}
+
+// Sets the window cycle list to Chrome browser windows only.
+void SetChromeCyclesWindows(int sequence_number) {
+  if (g_window_cycle_sequence_number != sequence_number)
+    return;
+
+  bool any_change = false;
+  for (NSWindow* window : [NSApp windows]) {
+    bool is_browser;
+    const Extension* extension = GetExtensionForNSWindow(window, &is_browser);
+    if (extension || is_browser)
+      any_change |= SetWindowParticipatesInCycle(window, is_browser);
+  }
+  if (any_change)
+    [[NSApp keyWindow] makeKeyAndOrderFront:nil];
+}
+
 }  // namespace
 
 // Used by AppShimMenuController to manage menu items that are a copy of a
@@ -149,7 +263,7 @@ void SetItemWithTagVisible(NSMenuItem* top_level_item,
         targetItemTag:(NSInteger)targetItemTag
         keyEquivalent:(NSString*)keyEquivalent;
 // Set the title using |resourceId_| and unset the source item's key equivalent.
-- (void)enableForApp:(const extensions::Extension*)app;
+- (void)enableForApp:(const Extension*)app;
 // Restore the source item's key equivalent.
 - (void)disable;
 @end
@@ -195,7 +309,7 @@ void SetItemWithTagVisible(NSMenuItem* top_level_item,
   return self;
 }
 
-- (void)enableForApp:(const extensions::Extension*)app {
+- (void)enableForApp:(const Extension*)app {
   // It seems that two menu items that have the same key equivalent must also
   // have the same action for the keyboard shortcut to work. (This refers to the
   // original keyboard shortcut, regardless of any overrides set in OSX).
@@ -225,8 +339,12 @@ void SetItemWithTagVisible(NSMenuItem* top_level_item,
 - (void)registerEventHandlers;
 // If the window is an app window, add or remove menu items.
 - (void)windowMainStatusChanged:(NSNotification*)notification;
+// Called when |app| becomes the main window in the Chrome process.
+- (void)appBecameMain:(const Extension*)app;
+// Called when there is no main window, or if the main window is not an app.
+- (void)chromeBecameMain;
 // Add menu items for an app and hide Chrome menu items.
-- (void)addMenuItems:(const extensions::Extension*)app;
+- (void)addMenuItems:(const Extension*)app;
 // If the window belongs to the currently focused app, remove the menu items and
 // unhide Chrome menu items.
 - (void)removeMenuItems;
@@ -376,23 +494,14 @@ void SetItemWithTagVisible(NSMenuItem* top_level_item,
   NSString* name = [notification name];
   if ([name isEqualToString:NSWindowDidBecomeMainNotification]) {
     id window = [notification object];
-    extensions::AppWindow* appWindow =
-        AppWindowRegistryUtil::GetAppWindowForNativeWindowAnyProfile(
-            window);
-
-    const extensions::Extension* extension = NULL;
-    // If there is no corresponding AppWindow, this could be a hosted app, so
-    // check for a browser.
-    if (appWindow)
-      extension = appWindow->GetExtension();
-    else
-      extension = apps::ExtensionAppShimHandler::MaybeGetAppForBrowser(
-          chrome::FindBrowserWithWindow(window));
-
+    bool is_browser;
+    const Extension* extension = GetExtensionForNSWindow(window, &is_browser);
+    // Ignore is_browser: if a window becomes main that does not belong to an
+    // extension or browser, treat it the same as switching to a browser.
     if (extension)
-      [self addMenuItems:extension];
+      [self appBecameMain:extension];
     else
-      [self removeMenuItems];
+      [self chromeBecameMain];
   } else if ([name isEqualToString:NSWindowDidResignMainNotification]) {
     // When a window resigns main status, reset back to the Chrome menu.
     // In the past we've tried:
@@ -410,21 +519,44 @@ void SetItemWithTagVisible(NSMenuItem* top_level_item,
     // unconditionally means that if another packaged app window becomes key,
     // the menu will flicker. TODO(tapted): Investigate restoring the logic when
     // the panel code is removed.
-    [self removeMenuItems];
+    [self chromeBecameMain];
   } else {
     NOTREACHED();
   }
 }
 
-- (void)addMenuItems:(const extensions::Extension*)app {
-  NSString* appId = base::SysUTF8ToNSString(app->id());
-  NSString* title = base::SysUTF8ToNSString(app->name());
+- (void)appBecameMain:(const Extension*)app {
+  if (appId_ == app->id())
+    return;
+
+  if (!appId_.empty())
+    [self removeMenuItems];
+
+  appId_ = app->id();
+  [self addMenuItems:app];
+  if (IsAppWindowCyclingEnabled()) {
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE, base::Bind(&SetAppCyclesWindows, appId_,
+                              ++g_window_cycle_sequence_number));
+  }
+}
 
-  if ([appId_ isEqualToString:appId])
+- (void)chromeBecameMain {
+  if (appId_.empty())
     return;
 
+  appId_.clear();
   [self removeMenuItems];
-  appId_.reset([appId copy]);
+  if (IsAppWindowCyclingEnabled()) {
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&SetChromeCyclesWindows, ++g_window_cycle_sequence_number));
+  }
+}
+
+- (void)addMenuItems:(const Extension*)app {
+  DCHECK_EQ(appId_, app->id());
+  NSString* title = base::SysUTF8ToNSString(app->name());
 
   // Hide Chrome menu items.
   NSMenu* mainMenu = [NSApp mainMenu];
@@ -438,7 +570,7 @@ void SetItemWithTagVisible(NSMenuItem* top_level_item,
   [openDoppelganger_ enableForApp:app];
   [closeWindowDoppelganger_ enableForApp:app];
 
-  [appMenuItem_ setTitle:appId];
+  [appMenuItem_ setTitle:base::SysUTF8ToNSString(appId_)];
   [[appMenuItem_ submenu] setTitle:title];
 
   [mainMenu addItem:appMenuItem_];
@@ -459,11 +591,6 @@ void SetItemWithTagVisible(NSMenuItem* top_level_item,
 }
 
 - (void)removeMenuItems {
-  if (!appId_)
-    return;
-
-  appId_.reset();
-
   NSMenu* mainMenu = [NSApp mainMenu];
   [mainMenu removeItem:appMenuItem_];
   [mainMenu removeItem:fileMenuItem_];
@@ -494,7 +621,7 @@ void SetItemWithTagVisible(NSMenuItem* top_level_item,
     apps::ExtensionAppShimHandler::QuitAppForWindow(appWindow);
   } else {
     Browser* browser = chrome::FindBrowserWithWindow([NSApp keyWindow]);
-    const extensions::Extension* extension =
+    const Extension* extension =
         apps::ExtensionAppShimHandler::MaybeGetAppForBrowser(browser);
     if (extension)
       apps::ExtensionAppShimHandler::QuitHostedAppForWindow(browser->profile(),
@@ -510,7 +637,7 @@ void SetItemWithTagVisible(NSMenuItem* top_level_item,
     apps::ExtensionAppShimHandler::HideAppForWindow(appWindow);
   } else {
     Browser* browser = chrome::FindBrowserWithWindow([NSApp keyWindow]);
-    const extensions::Extension* extension =
+    const Extension* extension =
         apps::ExtensionAppShimHandler::MaybeGetAppForBrowser(browser);
     if (extension)
       apps::ExtensionAppShimHandler::HideHostedApp(browser->profile(),
diff --git a/chrome/browser/ui/cocoa/apps/app_shim_menu_controller_mac_interactive_uitest.mm b/chrome/browser/ui/cocoa/apps/app_shim_menu_controller_mac_interactive_uitest.mm
new file mode 100644
index 0000000..8f0bc4e
--- /dev/null
+++ b/chrome/browser/ui/cocoa/apps/app_shim_menu_controller_mac_interactive_uitest.mm
@@ -0,0 +1,134 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "chrome/browser/ui/cocoa/apps/app_shim_menu_controller_mac.h"
+
+#import <Cocoa/Cocoa.h>
+#import <Carbon/Carbon.h>
+
+#include "base/command_line.h"
+#include "base/mac/scoped_cftyperef.h"
+#include "base/run_loop.h"
+#include "chrome/browser/apps/app_browsertest_util.h"
+#include "chrome/browser/apps/app_shim/extension_app_shim_handler_mac.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/interactive_test_utils.h"
+#include "extensions/common/extension.h"
+#include "extensions/test/extension_test_message_listener.h"
+#import "ui/base/test/windowed_nsnotification_observer.h"
+
+using extensions::AppWindow;
+using extensions::Extension;
+using ui_test_utils::ShowAndFocusNativeWindow;
+
+namespace {
+
+class AppShimMenuControllerUITest : public extensions::PlatformAppBrowserTest {
+ protected:
+  AppShimMenuControllerUITest() {}
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    PlatformAppBrowserTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitch(switches::kEnableAppWindowCycling);
+  }
+
+  void SetUpOnMainThread() override {
+    PlatformAppBrowserTest::SetUpOnMainThread();
+    const Extension* extension =
+        LoadAndLaunchPlatformApp("minimal", "Launched");
+
+    // First create an extra app window and an extra browser window. Only after
+    // the first call to ui_test_utils::ShowAndFocusNativeWindow(..) will the
+    // windows activate, because the test binary has a default activation policy
+    // of "prohibited".
+    app1_ = GetFirstAppWindow();
+    app2_ = CreateAppWindow(extension);
+    browser1_ = browser()->window();
+    browser2_ = (new Browser(Browser::CreateParams(
+                     profile(), chrome::HOST_DESKTOP_TYPE_NATIVE)))->window();
+    browser2_->Show();
+
+    // Since a pending key status change on any window could cause the test to
+    // become flaky, watch everything closely.
+    app1_watcher_.reset([[WindowedNSNotificationObserver alloc]
+        initForNotification:NSWindowDidBecomeMainNotification
+                     object:app1_->GetNativeWindow()]);
+    app2_watcher_.reset([[WindowedNSNotificationObserver alloc]
+        initForNotification:NSWindowDidBecomeMainNotification
+                     object:app2_->GetNativeWindow()]);
+    browser1_watcher_.reset([[WindowedNSNotificationObserver alloc]
+        initForNotification:NSWindowDidBecomeMainNotification
+                     object:browser1_->GetNativeWindow()]);
+    browser2_watcher_.reset([[WindowedNSNotificationObserver alloc]
+        initForNotification:NSWindowDidBecomeMainNotification
+                     object:browser2_->GetNativeWindow()]);
+  }
+
+  void TearDownOnMainThread() override {
+    CloseAppWindow(app1_);
+    CloseAppWindow(app2_);
+    browser2_->Close();
+    PlatformAppBrowserTest::TearDownOnMainThread();
+  }
+
+  // First wait for and verify the given activation counts on all windows, then
+  // expect that |main_window| is the active window.
+  void ExpectActiveWithCounts(NSWindow* main_window,
+                              int app1_count,
+                              int app2_count,
+                              int browser1_count,
+                              int browser2_count) {
+    EXPECT_TRUE([app1_watcher_ waitForCount:app1_count]);
+    EXPECT_TRUE([app2_watcher_ waitForCount:app2_count]);
+    EXPECT_TRUE([browser1_watcher_ waitForCount:browser1_count]);
+    EXPECT_TRUE([browser2_watcher_ waitForCount:browser2_count]);
+    EXPECT_TRUE([main_window isMainWindow]);
+  }
+
+  // Send Cmd+`. Note that it needs to go into kCGSessionEventTap, so NSEvents
+  // and [NSApp sendEvent:] doesn't work.
+  void CycleWindows() {
+    bool key_down = true;  // Sending a keyUp doesn't seem to be necessary.
+    base::ScopedCFTypeRef<CGEventRef> event(
+        CGEventCreateKeyboardEvent(nullptr, kVK_ANSI_Grave, key_down));
+    EXPECT_TRUE(event);
+    CGEventSetFlags(event, kCGEventFlagMaskCommand);
+    CGEventPost(kCGSessionEventTap, event);
+  }
+
+  AppWindow* app1_;
+  AppWindow* app2_;
+  BrowserWindow* browser1_;
+  BrowserWindow* browser2_;
+  base::scoped_nsobject<WindowedNSNotificationObserver> app1_watcher_,
+      app2_watcher_, browser1_watcher_, browser2_watcher_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AppShimMenuControllerUITest);
+};
+
+// Test that switching to a packaged app changes window cycling behavior.
+IN_PROC_BROWSER_TEST_F(AppShimMenuControllerUITest, WindowCycling) {
+  EXPECT_FALSE([app1_->GetNativeWindow() isMainWindow]);
+  EXPECT_TRUE(ShowAndFocusNativeWindow(app1_->GetNativeWindow()));
+  ExpectActiveWithCounts(app1_->GetNativeWindow(), 1, 0, 0, 0);
+
+  // Packaged app is active, so Cmd+` should cycle between the two app windows.
+  CycleWindows();
+  ExpectActiveWithCounts(app2_->GetNativeWindow(), 1, 1, 0, 0);
+  CycleWindows();
+  ExpectActiveWithCounts(app1_->GetNativeWindow(), 2, 1, 0, 0);
+
+  // Activate one of the browsers. Then cycling should go between the browsers.
+  EXPECT_TRUE(ShowAndFocusNativeWindow(browser1_->GetNativeWindow()));
+  ExpectActiveWithCounts(browser1_->GetNativeWindow(), 2, 1, 1, 0);
+  CycleWindows();
+  ExpectActiveWithCounts(browser2_->GetNativeWindow(), 2, 1, 1, 1);
+  CycleWindows();
+  ExpectActiveWithCounts(browser1_->GetNativeWindow(), 2, 1, 2, 1);
+}
+
+}  // namespace
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 1f4f81d..d3b6477 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -967,6 +967,7 @@
       'browser/task_manager/task_manager_browsertest_util.cc',
       'browser/ui/autofill/autofill_popup_controller_interactive_uitest.cc',
       'browser/ui/browser_focus_uitest.cc',
+      'browser/ui/cocoa/apps/app_shim_menu_controller_mac_interactive_uitest.mm',
       'browser/ui/cocoa/apps/quit_with_apps_controller_mac_interactive_uitest.mm',
       'browser/ui/cocoa/extensions/browser_action_button_interactive_uitest.mm',
       'browser/ui/cocoa/panels/panel_cocoa_browsertest.mm',
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index fc53f0a..f6d1f41 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -1229,6 +1229,9 @@ const char kHostedAppQuitNotification[] = "enable-hosted-app-quit-notification";
 // Disable the toolkit-views App Info dialog for Mac.
 const char kDisableAppInfoDialogMac[] = "disable-app-info-dialog-mac";
 
+// Disables custom Cmd+` window cycling for platform apps and hosted apps.
+const char kDisableAppWindowCycling[] = "disable-app-window-cycling";
+
 // Disables app shim creation for hosted apps on Mac.
 const char kDisableHostedAppShimCreation[] = "disable-hosted-app-shim-creation";
 
@@ -1245,6 +1248,9 @@ const char kDisableSystemFullscreenForTesting[] =
 // chrome://apps and chrome://extensions and is already enabled on non-mac.
 const char kEnableAppInfoDialogMac[] = "enable-app-info-dialog-mac";
 
+// Enables custom Cmd+` window cycling for platform apps and hosted apps.
+const char kEnableAppWindowCycling[] = "enable-app-window-cycling";
+
 // Enables use of toolkit-views based native app windows.
 const char kEnableMacViewsNativeAppWindows[] =
     "enable-mac-views-native-app-windows";
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index c76f2d3..fce3e68 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -352,10 +352,12 @@ extern const char kMigrateDataDirForSxS[];
 extern const char kAppsKeepChromeAliveInTests[];
 extern const char kHostedAppQuitNotification[];
 extern const char kDisableAppInfoDialogMac[];
+extern const char kDisableAppWindowCycling[];
 extern const char kDisableHostedAppShimCreation[];
 extern const char kDisableMacViewsNativeAppWindows[];
 extern const char kDisableSystemFullscreenForTesting[];
 extern const char kEnableAppInfoDialogMac[];
+extern const char kEnableAppWindowCycling[];
 extern const char kEnableMacViewsNativeAppWindows[];
 extern const char kEnableTranslateNewUX[];
 extern const char kMetricsClientID[];
diff --git a/chrome/test/base/interactive_test_utils_mac.mm b/chrome/test/base/interactive_test_utils_mac.mm
index 83f3321..17fa8e8 100644
--- a/chrome/test/base/interactive_test_utils_mac.mm
+++ b/chrome/test/base/interactive_test_utils_mac.mm
@@ -15,6 +15,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #import "chrome/browser/ui/cocoa/view_id_util.h"
 #include "ui/base/test/ui_controls.h"
+#import "ui/base/test/windowed_nsnotification_observer.h"
 
 namespace ui_test_utils {
 
@@ -104,17 +105,25 @@ bool ShowAndFocusNativeWindow(gfx::NativeWindow window) {
   TransformProcessType(&psn,kProcessTransformToForegroundApplication);
   SetFrontProcess(&psn);
 
+  base::scoped_nsobject<WindowedNSNotificationObserver> async_waiter;
+  if (![window isKeyWindow]) {
+    // Only wait when expecting a change to actually occur.
+    async_waiter.reset([[WindowedNSNotificationObserver alloc]
+        initForNotification:NSWindowDidBecomeKeyNotification
+                     object:window]);
+  }
   [window makeKeyAndOrderFront:nil];
 
   // Wait until |window| becomes key window, then make sure the shortcuts for
   // "Close Window" and "Close Tab" are updated.
   // This is because normal AppKit menu updating does not get invoked when
   // events are sent via ui_test_utils::SendKeyPressSync.
-  base::RunLoop().RunUntilIdle();
+  BOOL notification_observed = [async_waiter wait];
+  base::RunLoop().RunUntilIdle();  // There may be other events queued. Flush.
   NSMenu* file_menu = [[[NSApp mainMenu] itemWithTag:IDC_FILE_MENU] submenu];
   [[file_menu delegate] menuNeedsUpdate:file_menu];
 
-  return true;
+  return !async_waiter || notification_observed;
 }
 
 }  // namespace ui_test_utils
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index d78c1e1..1990bc4 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -59472,6 +59472,7 @@ To add a new entry, add it with any value and run test to compute valid value.
   <int value="-1218608640" label="disable-offline-load-stale-cache"/>
   <int value="-1216837777" label="clear-data-reduction-proxy-data-savings"/>
   <int value="-1212273428" label="enable-experimental-app-list"/>
+  <int value="-1212167260" label="disable-app-window-cycling"/>
   <int value="-1203742042" label="enable-gesture-selection"/>
   <int value="-1201183153" label="enable-centered-app-list"/>
   <int value="-1184904651" label="enable-npapi"/>
@@ -59656,6 +59657,7 @@ To add a new entry, add it with any value and run test to compute valid value.
   <int value="711424932" label="enable-cloud-print-xps"/>
   <int value="730024226" label="enable-out-of-process-pdf"/>
   <int value="732703958" label="enable-gesture-tap-highlight"/>
+  <int value="752194066" label="enable-app-window-cycling"/>
   <int value="773919225" label="disable-office-editing-component-extension"/>
   <int value="779086132" label="enable-data-reduction-proxy-alt"/>
   <int value="782167080" label="enable-new-qp-input-view"/>
diff --git a/ui/base/test/windowed_nsnotification_observer.h b/ui/base/test/windowed_nsnotification_observer.h
index ffecd97..9a3cb24 100644
--- a/ui/base/test/windowed_nsnotification_observer.h
+++ b/ui/base/test/windowed_nsnotification_observer.h
@@ -15,16 +15,17 @@ class RunLoop;
 
 // Like WindowedNotificationObserver in content/public/test/test_utils.h, this
 // starts watching for a notification upon construction and can wait until the
-// notification is observed. This guarantees that a notification fired between
+// notification is observed. This guarantees that notifications fired between
 // calls to init and wait will be caught.
 @interface WindowedNSNotificationObserver : NSObject {
  @private
   base::scoped_nsobject<NSString> bundleId_;
-  BOOL notificationReceived_;
+  int notificationCount_;
   base::RunLoop* runLoop_;
 }
 
-// Watch for an NSNotification on the default notification center.
+// Watch for an NSNotification on the default notification center for the given
+// |notificationSender|, or a nil object if omitted.
 - (id)initForNotification:(NSString*)name;
 
 // Watch for an NSNotification on the default notification center from a
@@ -32,13 +33,16 @@ class RunLoop;
 - (id)initForNotification:(NSString*)name object:(id)sender;
 
 // Watch for an NSNotification on the shared workspace notification center for
-// the
-// given application.
+// the given application.
 - (id)initForWorkspaceNotification:(NSString*)name bundleId:(NSString*)bundleId;
 
-// Returns if the notification has been observed, or spins a RunLoop until it
-// is. Stops observing.
-- (void)wait;
+// Waits for |minimumCount| notifications to be observed and returns YES.
+// Returns NO on a timeout. This can be called multiple times.
+- (BOOL)waitForCount:(int)minimumCount;
+
+// Returns YES if any notification has been observed, or spins a RunLoop until
+// it either is observed, or a timeout occurs. Returns NO on a timeout.
+- (BOOL)wait;
 @end
 
 #endif  // UI_BASE_TEST_WINDOWED_NSNOTIFICATION_OBSERVER_H_
diff --git a/ui/base/test/windowed_nsnotification_observer.mm b/ui/base/test/windowed_nsnotification_observer.mm
index ae8d2f4..bb0b9c7 100644
--- a/ui/base/test/windowed_nsnotification_observer.mm
+++ b/ui/base/test/windowed_nsnotification_observer.mm
@@ -7,6 +7,7 @@
 #import <Cocoa/Cocoa.h>
 
 #include "base/run_loop.h"
+#include "base/test/test_timeouts.h"
 
 @interface WindowedNSNotificationObserver ()
 - (void)onNotification:(NSNotification*)notification;
@@ -41,31 +42,46 @@
   return self;
 }
 
+- (void)dealloc {
+  if (bundleId_)
+    [[[NSWorkspace sharedWorkspace] notificationCenter] removeObserver:self];
+  else
+    [[NSNotificationCenter defaultCenter] removeObserver:self];
+  [super dealloc];
+}
+
 - (void)onNotification:(NSNotification*)notification {
   if (bundleId_) {
     NSRunningApplication* application =
         [[notification userInfo] objectForKey:NSWorkspaceApplicationKey];
     if (![[application bundleIdentifier] isEqualToString:bundleId_])
       return;
-
-    [[[NSWorkspace sharedWorkspace] notificationCenter] removeObserver:self];
-  } else {
-    [[NSNotificationCenter defaultCenter] removeObserver:self];
   }
 
-  notificationReceived_ = YES;
+  ++notificationCount_;
   if (runLoop_)
     runLoop_->Quit();
 }
 
-- (void)wait {
-  if (notificationReceived_)
-    return;
+- (BOOL)waitForCount:(int)minimumCount {
+  while (notificationCount_ < minimumCount) {
+    const int oldCount = notificationCount_;
+    base::RunLoop runLoop;
+    base::MessageLoop::current()->task_runner()->PostDelayedTask(
+        FROM_HERE, runLoop.QuitClosure(), TestTimeouts::action_timeout());
+    runLoop_ = &runLoop;
+    runLoop.Run();
+    runLoop_ = nullptr;
+
+    // If there was no new notification, it must have been a timeout.
+    if (notificationCount_ == oldCount)
+      break;
+  }
+  return notificationCount_ >= minimumCount;
+}
 
-  base::RunLoop runLoop;
-  runLoop_ = &runLoop;
-  runLoop.Run();
-  runLoop_ = nullptr;
+- (BOOL)wait {
+  return [self waitForCount:1];
 }
 
 @end