[Mac] AppWindow.show() and focus() should unhide the shim.

Previously, the shim would request attention (bounce in the dock). The
only way to unhide the shim would be for a user to activate it.

This does not work for apps that should become focused due to other
events, e.g. when responding to a notification action.

BUG=489924

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

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

14 files changed, 63 insertions, 36 deletions

diff --git a/chrome/app_shim/chrome_main_app_mode_mac.mm b/chrome/app_shim/chrome_main_app_mode_mac.mm
index 7babb4a..ca3d271 100644
--- a/chrome/app_shim/chrome_main_app_mode_mac.mm
+++ b/chrome/app_shim/chrome_main_app_mode_mac.mm
@@ -128,6 +128,10 @@ class AppShimController : public IPC::Listener {
   // Hide this app.
   void OnHide();
 
+  // Set this app to the unhidden state. Happens when an app window shows
+  // itself.
+  void OnUnhideWithoutActivation();
+
   // Requests user attention.
   void OnRequestUserAttention();
   void OnSetUserAttention(apps::AppShimAttentionType attention_type);
@@ -282,6 +286,8 @@ bool AppShimController::OnMessageReceived(const IPC::Message& message) {
   IPC_BEGIN_MESSAGE_MAP(AppShimController, message)
     IPC_MESSAGE_HANDLER(AppShimMsg_LaunchApp_Done, OnLaunchAppDone)
     IPC_MESSAGE_HANDLER(AppShimMsg_Hide, OnHide)
+    IPC_MESSAGE_HANDLER(AppShimMsg_UnhideWithoutActivation,
+                        OnUnhideWithoutActivation)
     IPC_MESSAGE_HANDLER(AppShimMsg_RequestUserAttention, OnRequestUserAttention)
     IPC_MESSAGE_HANDLER(AppShimMsg_SetUserAttention, OnSetUserAttention)
     IPC_MESSAGE_UNHANDLED(handled = false)
@@ -311,6 +317,10 @@ void AppShimController::OnHide() {
   [NSApp hide:nil];
 }
 
+void AppShimController::OnUnhideWithoutActivation() {
+  [NSApp unhideWithoutActivation];
+}
+
 void AppShimController::OnRequestUserAttention() {
   OnSetUserAttention(apps::APP_SHIM_ATTENTION_INFORMATIONAL);
 }
diff --git a/chrome/browser/apps/app_shim/app_shim_handler_mac.h b/chrome/browser/apps/app_shim/app_shim_handler_mac.h
index 7eae133..5482ae9 100644
--- a/chrome/browser/apps/app_shim/app_shim_handler_mac.h
+++ b/chrome/browser/apps/app_shim/app_shim_handler_mac.h
@@ -25,6 +25,10 @@ class AppShimHandler {
     virtual void OnAppClosed() = 0;
     // Invoked when the app should be hidden.
     virtual void OnAppHide() = 0;
+    // Invoked when a window becomes visible while the app is hidden. Ensures
+    // the shim's "Hide/Show" state is updated correctly and the app can be
+    // re-hidden.
+    virtual void OnAppUnhideWithoutActivation() = 0;
     // Invoked when the app is requesting user attention.
     virtual void OnAppRequestUserAttention(AppShimAttentionType type) = 0;
 
diff --git a/chrome/browser/apps/app_shim/app_shim_host_mac.cc b/chrome/browser/apps/app_shim/app_shim_host_mac.cc
index 6395daf..6ce706e 100644
--- a/chrome/browser/apps/app_shim/app_shim_host_mac.cc
+++ b/chrome/browser/apps/app_shim/app_shim_host_mac.cc
@@ -120,6 +120,10 @@ void AppShimHost::OnAppHide() {
   Send(new AppShimMsg_Hide);
 }
 
+void AppShimHost::OnAppUnhideWithoutActivation() {
+  Send(new AppShimMsg_UnhideWithoutActivation);
+}
+
 void AppShimHost::OnAppRequestUserAttention(apps::AppShimAttentionType type) {
   Send(new AppShimMsg_SetUserAttention(type));
 }
diff --git a/chrome/browser/apps/app_shim/app_shim_host_mac.h b/chrome/browser/apps/app_shim/app_shim_host_mac.h
index 31b8c1b..870cc65 100644
--- a/chrome/browser/apps/app_shim/app_shim_host_mac.h
+++ b/chrome/browser/apps/app_shim/app_shim_host_mac.h
@@ -69,6 +69,7 @@ class AppShimHost : public IPC::Listener,
   void OnAppLaunchComplete(apps::AppShimLaunchResult result) override;
   void OnAppClosed() override;
   void OnAppHide() override;
+  void OnAppUnhideWithoutActivation() override;
   void OnAppRequestUserAttention(apps::AppShimAttentionType type) override;
   base::FilePath GetProfilePath() const override;
   std::string GetAppId() const override;
diff --git a/chrome/browser/apps/app_shim/app_shim_quit_interactive_uitest_mac.mm b/chrome/browser/apps/app_shim/app_shim_quit_interactive_uitest_mac.mm
index acb90ed..74ae8c9 100644
--- a/chrome/browser/apps/app_shim/app_shim_quit_interactive_uitest_mac.mm
+++ b/chrome/browser/apps/app_shim/app_shim_quit_interactive_uitest_mac.mm
@@ -39,6 +39,7 @@ class FakeHost : public apps::AppShimHandler::Host {
   void OnAppLaunchComplete(AppShimLaunchResult result) override {}
   void OnAppClosed() override { handler_->OnShimClose(this); }
   void OnAppHide() override {}
+  void OnAppUnhideWithoutActivation() override {}
   void OnAppRequestUserAttention(AppShimAttentionType type) override {}
   base::FilePath GetProfilePath() const override { return profile_path_; }
   std::string GetAppId() const override { return app_id_; }
diff --git a/chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc b/chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc
index a7c992b..0cd6280 100644
--- a/chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc
+++ b/chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc
@@ -364,21 +364,13 @@ void ExtensionAppShimHandler::FocusAppForWindow(AppWindow* app_window) {
 }
 
 // static
-bool ExtensionAppShimHandler::ActivateAndRequestUserAttentionForWindow(
+void ExtensionAppShimHandler::UnhideWithoutActivationForWindow(
     AppWindow* app_window) {
   ExtensionAppShimHandler* handler = GetInstance();
   Profile* profile = Profile::FromBrowserContext(app_window->browser_context());
   Host* host = handler->FindHost(profile, app_window->extension_id());
-  if (host) {
-    // Bring the window to the front without showing it.
-    AppWindowRegistry::Get(profile)->AppWindowActivated(app_window);
-    host->OnAppRequestUserAttention(APP_SHIM_ATTENTION_INFORMATIONAL);
-    return true;
-  } else {
-    // Just show the app.
-    SetAppHidden(profile, app_window->extension_id(), false);
-    return false;
-  }
+  if (host)
+    host->OnAppUnhideWithoutActivation();
 }
 
 // static
diff --git a/chrome/browser/apps/app_shim/extension_app_shim_handler_mac.h b/chrome/browser/apps/app_shim/extension_app_shim_handler_mac.h
index c83c78c..f498a48 100644
--- a/chrome/browser/apps/app_shim/extension_app_shim_handler_mac.h
+++ b/chrome/browser/apps/app_shim/extension_app_shim_handler_mac.h
@@ -101,9 +101,8 @@ class ExtensionAppShimHandler : public AppShimHandler,
 
   static void FocusAppForWindow(extensions::AppWindow* app_window);
 
-  // Brings the window to the front without showing it and instructs the shim to
-  // request user attention. If there is no shim, show the app and return false.
-  static bool ActivateAndRequestUserAttentionForWindow(
+  // Instructs the shim to set it's "Hide/Show" state to not-hidden.
+  static void UnhideWithoutActivationForWindow(
       extensions::AppWindow* app_window);
 
   // Instructs the shim to request user attention. Returns false if there is no
diff --git a/chrome/browser/apps/app_shim/extension_app_shim_handler_mac_unittest.cc b/chrome/browser/apps/app_shim/extension_app_shim_handler_mac_unittest.cc
index 9fbb9af..1df7cd9 100644
--- a/chrome/browser/apps/app_shim/extension_app_shim_handler_mac_unittest.cc
+++ b/chrome/browser/apps/app_shim/extension_app_shim_handler_mac_unittest.cc
@@ -123,6 +123,7 @@ class FakeHost : public apps::AppShimHandler::Host {
     ++close_count_;
   }
   void OnAppHide() override {}
+  void OnAppUnhideWithoutActivation() override {}
   void OnAppRequestUserAttention(AppShimAttentionType type) override {}
   base::FilePath GetProfilePath() const override {
     return profile_path_;
diff --git a/chrome/browser/ui/app_list/app_list_service_mac_interactive_uitest.mm b/chrome/browser/ui/app_list/app_list_service_mac_interactive_uitest.mm
index 1ebd166..3c99dc3 100644
--- a/chrome/browser/ui/app_list/app_list_service_mac_interactive_uitest.mm
+++ b/chrome/browser/ui/app_list/app_list_service_mac_interactive_uitest.mm
@@ -70,6 +70,7 @@ class AppListServiceMacInteractiveTest : public InProcessBrowserTest,
   }
   void OnAppClosed() override { NOTREACHED(); }
   void OnAppHide() override {}
+  void OnAppUnhideWithoutActivation() override {}
   void OnAppRequestUserAttention(apps::AppShimAttentionType type) override {}
   base::FilePath GetProfilePath() const override {
     NOTREACHED();  // Currently unused in this test.
diff --git a/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm b/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm
index 3115f1a..080cfce 100644
--- a/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm
+++ b/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm
@@ -466,12 +466,9 @@ gfx::Rect NativeAppWindowCocoa::GetBounds() const {
 
 void NativeAppWindowCocoa::Show() {
   if (is_hidden_with_app_) {
-    // If there is a shim to gently request attention, return here. Otherwise
-    // show the window as usual.
-    if (apps::ExtensionAppShimHandler::ActivateAndRequestUserAttentionForWindow(
-            app_window_)) {
-      return;
-    }
+    apps::ExtensionAppShimHandler::UnhideWithoutActivationForWindow(
+        app_window_);
+    is_hidden_with_app_ = false;
   }
 
   [window_controller_ showWindow:nil];
diff --git a/chrome/browser/ui/cocoa/apps/native_app_window_cocoa_browsertest.mm b/chrome/browser/ui/cocoa/apps/native_app_window_cocoa_browsertest.mm
index f03002df..6c0d454 100644
--- a/chrome/browser/ui/cocoa/apps/native_app_window_cocoa_browsertest.mm
+++ b/chrome/browser/ui/cocoa/apps/native_app_window_cocoa_browsertest.mm
@@ -111,12 +111,12 @@ IN_PROC_BROWSER_TEST_F(NativeAppWindowCocoaBrowserTest, HideShowWithApp) {
   other_native_window->HideWithApp();
   EXPECT_FALSE([other_ns_window isVisible]);
 
-  // HideWithApp, Show shows all windows for this app.
+  // HideWithApp, Show shows just one window since there's no shim.
   native_window->HideWithApp();
   EXPECT_FALSE([ns_window isVisible]);
   app_window->Show(AppWindow::SHOW_ACTIVE);
   EXPECT_TRUE([ns_window isVisible]);
-  EXPECT_TRUE([other_ns_window isVisible]);
+  EXPECT_FALSE([other_ns_window isVisible]);
 
   // Hide the other window.
   other_app_window->Hide();
@@ -140,6 +140,7 @@ class MockAppShimHost : public apps::AppShimHandler::Host {
   MOCK_METHOD1(OnAppLaunchComplete, void(apps::AppShimLaunchResult));
   MOCK_METHOD0(OnAppClosed, void());
   MOCK_METHOD0(OnAppHide, void());
+  MOCK_METHOD0(OnAppUnhideWithoutActivation, void());
   MOCK_METHOD1(OnAppRequestUserAttention, void(apps::AppShimAttentionType));
   MOCK_CONST_METHOD0(GetProfilePath, base::FilePath());
   MOCK_CONST_METHOD0(GetAppId, std::string());
@@ -180,19 +181,23 @@ IN_PROC_BROWSER_TEST_F(NativeAppWindowCocoaBrowserTest,
   native_window->HideWithApp();
   EXPECT_FALSE([ns_window isVisible]);
 
-  // Show does not show any windows, it notifies the shim instead.
-  EXPECT_CALL(mock_host, OnAppRequestUserAttention(_));
+  // Show notifies the shim to unhide.
+  EXPECT_CALL(mock_host, OnAppUnhideWithoutActivation());
   EXPECT_CALL(*mock, FindHost(_, _)).WillOnce(Return(&mock_host));
   app_window->Show(extensions::AppWindow::SHOW_ACTIVE);
-  EXPECT_FALSE([ns_window isVisible]);
+  EXPECT_TRUE([ns_window isVisible]);
   testing::Mock::VerifyAndClearExpectations(mock);
   testing::Mock::VerifyAndClearExpectations(&mock_host);
 
+  // HideWithApp
+  native_window->HideWithApp();
+  EXPECT_FALSE([ns_window isVisible]);
+
   // Activate does the same.
-  EXPECT_CALL(mock_host, OnAppRequestUserAttention(_));
+  EXPECT_CALL(mock_host, OnAppUnhideWithoutActivation());
   EXPECT_CALL(*mock, FindHost(_, _)).WillOnce(Return(&mock_host));
   native_window->Activate();
-  EXPECT_FALSE([ns_window isVisible]);
+  EXPECT_TRUE([ns_window isVisible]);
   testing::Mock::VerifyAndClearExpectations(mock);
   testing::Mock::VerifyAndClearExpectations(&mock_host);
 }
diff --git a/chrome/browser/ui/views/apps/chrome_native_app_window_views_mac.h b/chrome/browser/ui/views/apps/chrome_native_app_window_views_mac.h
index 852457d2..fda6c54 100644
--- a/chrome/browser/ui/views/apps/chrome_native_app_window_views_mac.h
+++ b/chrome/browser/ui/views/apps/chrome_native_app_window_views_mac.h
@@ -25,6 +25,7 @@ class ChromeNativeAppWindowViewsMac : public ChromeNativeAppWindowViews {
   // ui::BaseWindow implementation.
   void Show() override;
   void ShowInactive() override;
+  void Activate() override;
   void FlashFrame(bool flash) override;
 
   // NativeAppWindow implementation.
@@ -35,6 +36,9 @@ class ChromeNativeAppWindowViewsMac : public ChromeNativeAppWindowViews {
   void HideWithApp() override;
 
  private:
+  // Unset is_hidden_with_app_ and tell the shim to unhide.
+  void UnhideWithoutActivation();
+
   // Whether this window last became hidden due to a request to hide the entire
   // app, e.g. via the dock menu or Cmd+H. This is set by Hide/ShowWithApp.
   bool is_hidden_with_app_;
diff --git a/chrome/browser/ui/views/apps/chrome_native_app_window_views_mac.mm b/chrome/browser/ui/views/apps/chrome_native_app_window_views_mac.mm
index 3118db8..d99a1c91 100644
--- a/chrome/browser/ui/views/apps/chrome_native_app_window_views_mac.mm
+++ b/chrome/browser/ui/views/apps/chrome_native_app_window_views_mac.mm
@@ -37,15 +37,7 @@ ChromeNativeAppWindowViewsMac::CreateNonStandardAppFrame() {
 }
 
 void ChromeNativeAppWindowViewsMac::Show() {
-  if (is_hidden_with_app_) {
-    // If there is a shim to gently request attention, return here. Otherwise
-    // show the window as usual.
-    if (apps::ExtensionAppShimHandler::ActivateAndRequestUserAttentionForWindow(
-            app_window())) {
-      return;
-    }
-  }
-
+  UnhideWithoutActivation();
   ChromeNativeAppWindowViews::Show();
 }
 
@@ -56,6 +48,11 @@ void ChromeNativeAppWindowViewsMac::ShowInactive() {
   ChromeNativeAppWindowViews::ShowInactive();
 }
 
+void ChromeNativeAppWindowViewsMac::Activate() {
+  UnhideWithoutActivation();
+  ChromeNativeAppWindowViews::Activate();
+}
+
 void ChromeNativeAppWindowViewsMac::FlashFrame(bool flash) {
   apps::ExtensionAppShimHandler::RequestUserAttentionForWindow(
       app_window(), flash ? apps::APP_SHIM_ATTENTION_CRITICAL
@@ -72,3 +69,11 @@ void ChromeNativeAppWindowViewsMac::HideWithApp() {
   is_hidden_with_app_ = true;
   ChromeNativeAppWindowViews::Hide();
 }
+
+void ChromeNativeAppWindowViewsMac::UnhideWithoutActivation() {
+  if (is_hidden_with_app_) {
+    apps::ExtensionAppShimHandler::UnhideWithoutActivationForWindow(
+        app_window());
+    is_hidden_with_app_ = false;
+  }
+}
diff --git a/chrome/common/mac/app_shim_messages.h b/chrome/common/mac/app_shim_messages.h
index 09727f1..9593929 100644
--- a/chrome/common/mac/app_shim_messages.h
+++ b/chrome/common/mac/app_shim_messages.h
@@ -69,3 +69,6 @@ IPC_MESSAGE_CONTROL0(AppShimHostMsg_QuitApp)
 // Instructs the shim to request or cancel user attention.
 IPC_MESSAGE_CONTROL1(AppShimMsg_SetUserAttention,
                      apps::AppShimAttentionType /* attention_type */)
+
+// Instructs the shim to show the app.
+IPC_MESSAGE_CONTROL0(AppShimMsg_UnhideWithoutActivation)