MacPWAs BMO: Remove some uses of extensions::Extension

The general plan here is to make ExtensionAppShimHandler not know
about extensions, but be able to interact with either apps or
extensions via its ExtensionAppShimHandler::Delegate interface.

To this end, start changing the interface to take app ids instead of
extensions::Extensions. Most of this is mechanical, but a few
aren't quite:
* The Delegate::GetWindows method can't return an extensions type
  anymore, so inline its usage into new ShowAppWindows and
  CloseAppWindows methods (which, internally, will be able to).
* Remove RequestUserAttentionForWindow, and have the caller (which
  is extensions-based app code) make the appropriate calls.

Update tests to behave reasonably.

Bug: 1057195
Change-Id: Ife8f37b608a9a601eafd38da44c2ad50b99f41c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2080487Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746502}

chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc

@@ -69,7 +69,6 @@
 #include "extensions/common/constants.h"
 #include "ui/base/cocoa/focus_window_set.h"
 
-using extensions::AppWindow;
 using extensions::AppWindowRegistry;
 using extensions::Extension;
 using extensions::ExtensionRegistry;
@@ -311,10 +310,27 @@ bool ExtensionAppShimHandler::Delegate::IsProfileLockedForPath(
   return profiles::IsProfileLocked(full_path);
 }
 
-AppWindowList ExtensionAppShimHandler::Delegate::GetWindows(
+bool ExtensionAppShimHandler::Delegate::ShowAppWindows(
     Profile* profile,
-    const std::string& extension_id) {
-  return AppWindowRegistry::Get(profile)->GetAppWindowsForApp(extension_id);
+    const std::string& app_id) {
+  AppWindowList windows =
+      AppWindowRegistry::Get(profile)->GetAppWindowsForApp(app_id);
+  for (auto it = windows.rbegin(); it != windows.rend(); ++it) {
+    if (*it)
+      (*it)->GetBaseWindow()->Show();
+  }
+  return !windows.empty();
+}
+
+void ExtensionAppShimHandler::Delegate::CloseAppWindows(
+    Profile* profile,
+    const std::string& app_id) {
+  AppWindowList windows =
+      AppWindowRegistry::Get(profile)->GetAppWindowsForApp(app_id);
+  for (auto it = windows.begin(); it != windows.end(); ++it) {
+    if (*it)
+      (*it)->GetBaseWindow()->Close();
+  }
 }
 
 const Extension* ExtensionAppShimHandler::Delegate::MaybeGetAppExtension(
@@ -325,7 +341,8 @@ const Extension* ExtensionAppShimHandler::Delegate::MaybeGetAppExtension(
 
 bool ExtensionAppShimHandler::Delegate::AllowShimToConnect(
     Profile* profile,
-    const extensions::Extension* extension) {
+    const std::string& app_id) {
+  const Extension* extension = MaybeGetAppExtension(profile, app_id);
   if (!profile || !extension)
     return false;
   if (extension->is_hosted_app() &&
@@ -349,15 +366,22 @@ std::unique_ptr<AppShimHost> ExtensionAppShimHandler::Delegate::CreateHost(
 
 void ExtensionAppShimHandler::Delegate::EnableExtension(
     Profile* profile,
-    const std::string& extension_id,
+    const std::string& app_id,
     base::OnceCallback<void()> callback) {
-  (new EnableViaPrompt(profile, extension_id, std::move(callback)))->Run();
+  const Extension* extension = MaybeGetAppExtension(profile, app_id);
+  if (extension)
+    std::move(callback).Run();
+  else
+    (new EnableViaPrompt(profile, app_id, std::move(callback)))->Run();
 }
 
 void ExtensionAppShimHandler::Delegate::LaunchApp(
     Profile* profile,
-    const Extension* extension,
+    const std::string& app_id,
     const std::vector<base::FilePath>& files) {
+  const extensions::Extension* extension =
+      MaybeGetAppExtension(profile, app_id);
+  DCHECK(extension);
   extensions::RecordAppLaunchType(
       extension_misc::APP_LAUNCH_CMD_LINE_APP, extension->GetType());
   if (extension->is_hosted_app()) {
@@ -399,10 +423,16 @@ void ExtensionAppShimHandler::Delegate::OpenAppURLInBrowserWindow(
 
 void ExtensionAppShimHandler::Delegate::LaunchShim(
     Profile* profile,
-    const Extension* extension,
+    const std::string& app_id,
     bool recreate_shims,
     apps::ShimLaunchedCallback launched_callback,
     apps::ShimTerminatedCallback terminated_callback) {
+  const Extension* extension = MaybeGetAppExtension(profile, app_id);
+  if (!extension) {
+    std::move(launched_callback).Run(base::Process());
+    return;
+  }
+
   // Only force recreation of shims when RemoteViews is in use (that is, for
   // PWAs). Otherwise, shims may be created unexpectedly.
   // https://crbug.com/941160
@@ -502,15 +532,6 @@ const Extension* ExtensionAppShimHandler::MaybeGetAppForBrowser(
       web_app::GetAppIdFromApplicationName(browser->app_name()));
 }
 
-void ExtensionAppShimHandler::RequestUserAttentionForWindow(
-    AppWindow* app_window,
-    chrome::mojom::AppShimAttentionType attention_type) {
-  Profile* profile = Profile::FromBrowserContext(app_window->browser_context());
-  AppShimHost* host = FindHost(profile, app_window->extension_id());
-  if (host && !host->UsesRemoteViews())
-    host->GetAppShim()->SetUserAttention(attention_type);
-}
-
 void ExtensionAppShimHandler::OnShimLaunchRequested(
     AppShimHost* host,
     bool recreate_shims,
@@ -531,16 +552,7 @@ void ExtensionAppShimHandler::OnShimLaunchRequested(
       profile = delegate_->ProfileForPath(host->GetProfilePath());
     }
   }
-
-  const Extension* extension =
-      delegate_->MaybeGetAppExtension(profile, host->GetAppId());
-  if (!profile || !extension) {
-    // If the profile or extension has been unloaded, indicate that the launch
-    // failed. This will close the AppShimHost eventually, if appropriate.
-    std::move(launched_callback).Run(base::Process());
-    return;
-  }
-  delegate_->LaunchShim(profile, extension, recreate_shims,
+  delegate_->LaunchShim(profile, host->GetAppId(), recreate_shims,
                         std::move(launched_callback),
                         std::move(terminated_callback));
 }
@@ -569,7 +581,7 @@ void ExtensionAppShimHandler::OnShimProcessConnectedForRegisterOnly(
   // exit), but many tests assume this behavior, and need to be updated.
   Profile* profile = delegate_->ProfileForPath(bootstrap->GetProfilePath());
   const Extension* extension = delegate_->MaybeGetAppExtension(profile, app_id);
-  if (profile && extension && delegate_->AllowShimToConnect(profile, extension))
+  if (profile && extension && delegate_->AllowShimToConnect(profile, app_id))
     GetOrCreateProfileState(profile, extension);
 
   // Because this was a register-only launch, it must have been launched by
@@ -637,12 +649,10 @@ void ExtensionAppShimHandler::OnShimProcessConnectedForLaunch(
     for (const auto& profile_path : profile_paths_to_launch) {
       if (profile_path.empty())
         continue;
-      LoadProfileAppCallback callback_wrapped = base::BindOnce(
-          [](base::OnceClosure callback_to_wrap, Profile*,
-             const extensions::Extension*) {
-            std::move(callback_to_wrap).Run();
-          },
-          std::move(callback));
+      LoadProfileAppCallback callback_wrapped =
+          base::BindOnce([](base::OnceClosure callback_to_wrap,
+                            Profile*) { std::move(callback_to_wrap).Run(); },
+                         std::move(callback));
       callback = base::BindOnce(&ExtensionAppShimHandler::LoadProfileAndApp,
                                 weak_factory_.GetWeakPtr(), profile_path,
                                 app_id, std::move(callback_wrapped));
@@ -689,7 +699,7 @@ void ExtensionAppShimHandler::OnShimProcessConnectedAndProfilesToLaunchLoaded(
     // Create a ProfileState for this app, if appropriate (e.g, not for
     // open-in-a-tab bookmark apps).
     ProfileState* profile_state = nullptr;
-    if (delegate_->AllowShimToConnect(profile, extension))
+    if (delegate_->AllowShimToConnect(profile, app_id))
       profile_state = GetOrCreateProfileState(profile, extension);
 
     // If there exist any open window for this profile, then bring them to the
@@ -707,7 +717,7 @@ void ExtensionAppShimHandler::OnShimProcessConnectedAndProfilesToLaunchLoaded(
     // Launch the app (open a window for it) if there were no open windows for
     // it already, or if we were asked to open files.
     if (!had_open_windows || !launch_files.empty()) {
-      delegate_->LaunchApp(profile, extension, launch_files);
+      delegate_->LaunchApp(profile, app_id, launch_files);
       launch_files.clear();
     }
 
@@ -849,7 +859,7 @@ void ExtensionAppShimHandler::OnProfileLoaded(
     // TODO(jackhou): Add some UI for this case and remove the LOG.
     LOG(ERROR) << "Requested directory is not a known profile '"
                << profile_path.value() << "'.";
-    std::move(callback).Run(profile, nullptr);
+    std::move(callback).Run(profile);
     return;
   }
 
@@ -859,7 +869,7 @@ void ExtensionAppShimHandler::OnProfileLoaded(
   // life within a certain window.
   const Extension* extension = delegate_->MaybeGetAppExtension(profile, app_id);
   if (extension) {
-    std::move(callback).Run(profile, extension);
+    std::move(callback).Run(profile);
   } else {
     delegate_->EnableExtension(
         profile, app_id,
@@ -872,12 +882,7 @@ void ExtensionAppShimHandler::OnProfileLoaded(
 void ExtensionAppShimHandler::OnAppEnabled(const base::FilePath& profile_path,
                                            const std::string& app_id,
                                            LoadProfileAppCallback callback) {
-  // If the profile doesn't exist, it may have been deleted during the enable
-  // prompt.
-  Profile* profile = delegate_->ProfileForPath(profile_path);
-  const Extension* extension =
-      profile ? delegate_->MaybeGetAppExtension(profile, app_id) : nullptr;
-  std::move(callback).Run(profile, extension);
+  std::move(callback).Run(delegate_->ProfileForPath(profile_path));
 }
 
 bool ExtensionAppShimHandler::IsAcceptablyCodeSigned(pid_t pid) const {
@@ -921,13 +926,8 @@ void ExtensionAppShimHandler::OnShimProcessDisconnected(AppShimHost* host) {
     apps_.erase(found_app);
 
   // Close app windows if we decided to do so above.
-  if (close_windows) {
-    AppWindowList windows = delegate_->GetWindows(profile, app_id);
-    for (auto it = windows.begin(); it != windows.end(); ++it) {
-      if (*it)
-        (*it)->GetBaseWindow()->Close();
-    }
-  }
+  if (close_windows)
+    delegate_->CloseAppWindows(profile, app_id);
 }
 
 void ExtensionAppShimHandler::OnShimFocus(AppShimHost* host) {
@@ -943,16 +943,11 @@ void ExtensionAppShimHandler::OnShimFocus(AppShimHost* host) {
     return;
   }
 
-  AppWindowList windows = delegate_->GetWindows(profile, host->GetAppId());
-  if (!windows.empty()) {
-    for (auto it = windows.rbegin(); it != windows.rend(); ++it) {
-      if (*it)
-        (*it)->GetBaseWindow()->Show();
-    }
+  if (delegate_->ShowAppWindows(profile, host->GetAppId()))
     return;
-  }
 
-  delegate_->LaunchApp(profile, extension, std::vector<base::FilePath>());
+  delegate_->LaunchApp(profile, host->GetAppId(),
+                       std::vector<base::FilePath>());
 }
 
 void ExtensionAppShimHandler::OnShimOpenedFiles(
@@ -970,10 +965,7 @@ void ExtensionAppShimHandler::OnShimOpenedFiles(
     profile = delegate_->ProfileForPath(host->GetProfilePath());
   }
   DCHECK(profile);
-  const Extension* extension =
-      delegate_->MaybeGetAppExtension(profile, host->GetAppId());
-  DCHECK(extension);
-  delegate_->LaunchApp(profile, extension, files);
+  delegate_->LaunchApp(profile, host->GetAppId(), files);
 }
 
 void ExtensionAppShimHandler::OnShimSelectedProfile(
@@ -983,12 +975,13 @@ void ExtensionAppShimHandler::OnShimSelectedProfile(
       profile_path, host->GetAppId(),
       base::BindOnce(
           &ExtensionAppShimHandler::OnShimSelectedProfileAndAppLoaded,
-          weak_factory_.GetWeakPtr()));
+          weak_factory_.GetWeakPtr(), host->GetAppId()));
 }
 
 void ExtensionAppShimHandler::OnShimSelectedProfileAndAppLoaded(
-    Profile* profile,
-    const extensions::Extension* extension) {
+    const std::string& app_id,
+    Profile* profile) {
+  const Extension* extension = delegate_->MaybeGetAppExtension(profile, app_id);
   if (!extension)
     return;
 
@@ -1007,7 +1000,7 @@ void ExtensionAppShimHandler::OnShimSelectedProfileAndAppLoaded(
   } else {
     // Otherwise, launch the app for this profile (which will open a new
     // window).
-    delegate_->LaunchApp(profile, extension, std::vector<base::FilePath>());
+    delegate_->LaunchApp(profile, app_id, std::vector<base::FilePath>());
   }
 }
 

chrome/browser/apps/app_shim/extension_app_shim_handler_mac.h

@@ -35,7 +35,6 @@ class BrowserContext;
 }  // namespace content
 
 namespace extensions {
-class AppWindow;
 class Extension;
 }  // namespace extensions
 
@@ -65,10 +64,12 @@ class ExtensionAppShimHandler : public AppShimHostBootstrap::Client,
     // locked.
     virtual bool IsProfileLockedForPath(const base::FilePath& path);
 
-    // Return the app windows (not browser windows) for a legacy app.
-    virtual extensions::AppWindowRegistry::AppWindowList GetWindows(
-        Profile* profile,
-        const std::string& extension_id);
+    // Show all app windows (for non-PWA apps). Return true if there existed any
+    // windows.
+    virtual bool ShowAppWindows(Profile* profile, const std::string& app_id);
+
+    // Close all app windows (for non-PWA apps).
+    virtual void CloseAppWindows(Profile* profile, const std::string& app_id);
 
     // Look up an extension from its id.
     virtual const extensions::Extension* MaybeGetAppExtension(
@@ -78,7 +79,7 @@ class ExtensionAppShimHandler : public AppShimHostBootstrap::Client,
     // Return true if the specified app should use an app shim (false, e.g, for
     // bookmark apps that open in tabs).
     virtual bool AllowShimToConnect(Profile* profile,
-                                    const extensions::Extension* extension);
+                                    const std::string& app_id);
 
     // Create an AppShimHost for the specified parameters (intercept-able for
     // tests).
@@ -96,7 +97,7 @@ class ExtensionAppShimHandler : public AppShimHostBootstrap::Client,
 
     // Launch the app in Chrome. This will (often) create a new window.
     virtual void LaunchApp(Profile* profile,
-                           const extensions::Extension* extension,
+                           const std::string& app_id,
                            const std::vector<base::FilePath>& files);
 
     // Open the specified URL in a new Chrome window. This is the fallback when
@@ -108,7 +109,7 @@ class ExtensionAppShimHandler : public AppShimHostBootstrap::Client,
 
     // Launch the shim process for an app.
     virtual void LaunchShim(Profile* profile,
-                            const extensions::Extension* extension,
+                            const std::string& app_id,
                             bool recreate_shims,
                             ShimLaunchedCallback launched_callback,
                             ShimTerminatedCallback terminated_callback);
@@ -145,12 +146,6 @@ class ExtensionAppShimHandler : public AppShimHostBootstrap::Client,
 
   static const extensions::Extension* MaybeGetAppForBrowser(Browser* browser);
 
-  // Instructs the shim to request user attention. Returns false if there is no
-  // shim for this window.
-  void RequestUserAttentionForWindow(
-      extensions::AppWindow* app_window,
-      chrome::mojom::AppShimAttentionType attention_type);
-
   // AppShimHostBootstrap::Client:
   void OnShimProcessConnected(
       std::unique_ptr<AppShimHostBootstrap> bootstrap) override;
@@ -250,14 +245,12 @@ class ExtensionAppShimHandler : public AppShimHostBootstrap::Client,
       std::unique_ptr<AppShimHostBootstrap> bootstrap);
 
   // Continuation of OnShimSelectedProfile, once the profile has loaded.
-  void OnShimSelectedProfileAndAppLoaded(
-      Profile* profile,
-      const extensions::Extension* extension);
+  void OnShimSelectedProfileAndAppLoaded(const std::string& app_id,
+                                         Profile* profile);
 
   // Load the specified profile and extension, and run |callback| with
   // the result. The callback's arguments may be nullptr on failure.
-  using LoadProfileAppCallback =
-      base::OnceCallback<void(Profile*, const extensions::Extension*)>;
+  using LoadProfileAppCallback = base::OnceCallback<void(Profile*)>;
   void LoadProfileAndApp(const base::FilePath& profile_path,
                          const std::string& app_id,
                          LoadProfileAppCallback callback);

chrome/browser/apps/app_shim/extension_app_shim_handler_mac_unittest.cc

@@ -56,7 +56,8 @@ class MockDelegate : public ExtensionAppShimHandler::Delegate {
   }
   MOCK_METHOD1(IsProfileLockedForPath, bool(const base::FilePath&));
 
-  MOCK_METHOD2(GetWindows, AppWindowList(Profile*, const std::string&));
+  MOCK_METHOD2(ShowAppWindows, bool(Profile*, const std::string&));
+  MOCK_METHOD2(CloseAppWindows, void(Profile*, const std::string&));
 
   MOCK_METHOD2(MaybeGetAppExtension,
                const Extension*(content::BrowserContext*, const std::string&));
@@ -64,16 +65,16 @@ class MockDelegate : public ExtensionAppShimHandler::Delegate {
                void(Profile*, const std::string&, base::OnceCallback<void()>));
   MOCK_METHOD3(LaunchApp,
                void(Profile*,
-                    const Extension*,
+                    const std::string& app_id,
                     const std::vector<base::FilePath>&));
   MOCK_METHOD2(OpenAppURLInBrowserWindow,
                void(const base::FilePath&, const GURL& url));
 
   // Conditionally mock LaunchShim. Some tests will execute |launch_callback|
   // with a particular value.
-  MOCK_METHOD3(DoLaunchShim, void(Profile*, const Extension*, bool));
+  MOCK_METHOD3(DoLaunchShim, void(Profile*, const std::string&, bool));
   void LaunchShim(Profile* profile,
-                  const Extension* extension,
+                  const std::string& app_id,
                   bool recreate_shim,
                   ShimLaunchedCallback launched_callback,
                   ShimTerminatedCallback terminated_callback) override {
@@ -81,7 +82,7 @@ class MockDelegate : public ExtensionAppShimHandler::Delegate {
       *launch_shim_callback_capture_ = std::move(launched_callback);
     if (terminated_shim_callback_capture_)
       *terminated_shim_callback_capture_ = std::move(terminated_callback);
-    DoLaunchShim(profile, extension, recreate_shim);
+    DoLaunchShim(profile, app_id, recreate_shim);
   }
   void SetCaptureShimLaunchedCallback(ShimLaunchedCallback* callback) {
     launch_shim_callback_capture_ = callback;
@@ -98,7 +99,7 @@ class MockDelegate : public ExtensionAppShimHandler::Delegate {
     allow_shim_to_connect_ = should_create_host;
   }
   bool AllowShimToConnect(Profile* profile,
-                          const extensions::Extension* extension) override {
+                          const std::string& app_id) override {
     return allow_shim_to_connect_;
   }
 
@@ -405,13 +406,6 @@ class ExtensionAppShimHandlerTest : public testing::Test {
     EXPECT_CALL(*delegate_, ProfileForPath(profile_path_c_))
         .WillRepeatedly(Return(&profile_c_));
 
-    // In most tests, we don't care about the result of GetWindows, it just
-    // needs to be non-empty.
-    AppWindowList app_window_list;
-    app_window_list.push_back(static_cast<extensions::AppWindow*>(NULL));
-    EXPECT_CALL(*delegate_, GetWindows(_, _))
-        .WillRepeatedly(Return(app_window_list));
-
     EXPECT_CALL(*delegate_, MaybeGetAppExtension(&profile_a_, kTestAppIdA))
         .WillRepeatedly(Return(extension_a_.get()));
     EXPECT_CALL(*delegate_, MaybeGetAppExtension(&profile_b_, kTestAppIdA))
@@ -608,8 +602,7 @@ TEST_F(ExtensionAppShimHandlerTest, LaunchAndCloseShim) {
   EXPECT_EQ(host_ab_.get(), handler_->FindHost(&profile_a_, kTestAppIdB));
 
   std::vector<base::FilePath> some_file(1, base::FilePath("some_file"));
-  EXPECT_CALL(*delegate_,
-              LaunchApp(&profile_b_, extension_b_.get(), some_file));
+  EXPECT_CALL(*delegate_, LaunchApp(&profile_b_, kTestAppIdB, some_file));
   DoShimLaunch(bootstrap_bb_, std::move(host_bb_unique_),
                chrome::mojom::AppShimLaunchType::kNormal, some_file);
   EXPECT_EQ(host_bb_.get(), handler_->FindHost(&profile_b_, kTestAppIdB));
@@ -641,41 +634,33 @@ TEST_F(ExtensionAppShimHandlerTest, AppLifetime) {
   // When the app activates, a host is created. If there is no shim, one is
   // launched.
   delegate_->SetHostForCreate(std::move(host_aa_unique_));
-  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, extension_a_.get(), false));
+  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, kTestAppIdA, false));
   handler_->OnAppActivated(&profile_a_, kTestAppIdA);
   EXPECT_EQ(host_aa_.get(), handler_->FindHost(&profile_a_, kTestAppIdA));
 
   // Normal shim launch adds an entry in the map.
   // App should not be launched here, but return success to the shim.
-  EXPECT_CALL(*delegate_,
-              LaunchApp(&profile_a_, extension_a_.get(), _))
-      .Times(0);
+  EXPECT_CALL(*delegate_, LaunchApp(&profile_a_, kTestAppIdA, _)).Times(0);
   RegisterOnlyLaunch(bootstrap_aa_, nullptr);
   EXPECT_EQ(chrome::mojom::AppShimLaunchResult::kSuccess,
             *bootstrap_aa_result_);
   EXPECT_EQ(host_aa_.get(), handler_->FindHost(&profile_a_, kTestAppIdA));
 
   // Return no app windows for OnShimFocus. This will result in a launch call.
-  AppWindowList app_window_list;
-  EXPECT_CALL(*delegate_, GetWindows(&profile_a_, kTestAppIdA))
-      .WillRepeatedly(Return(app_window_list));
-  EXPECT_CALL(*delegate_, LaunchApp(&profile_a_, extension_a_.get(), _))
-      .Times(1);
+  EXPECT_CALL(*delegate_, ShowAppWindows(&profile_a_, kTestAppIdA))
+      .WillRepeatedly(Return(false));
+  EXPECT_CALL(*delegate_, LaunchApp(&profile_a_, kTestAppIdA, _)).Times(1);
   ShimNormalFocus(host_aa_.get());
 
   // Return one window. This should do nothing.
-  app_window_list.push_back(static_cast<extensions::AppWindow*>(nullptr));
-  EXPECT_CALL(*delegate_, GetWindows(&profile_a_, kTestAppIdA))
-      .WillRepeatedly(Return(app_window_list));
-  EXPECT_CALL(*delegate_,
-              LaunchApp(&profile_a_, extension_a_.get(), _))
-      .Times(0);
+  EXPECT_CALL(*delegate_, ShowAppWindows(&profile_a_, kTestAppIdA))
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*delegate_, LaunchApp(&profile_a_, kTestAppIdA, _)).Times(0);
   ShimNormalFocus(host_aa_.get());
 
   // Open files should trigger a launch with those files.
   std::vector<base::FilePath> some_file(1, base::FilePath("some_file"));
-  EXPECT_CALL(*delegate_,
-              LaunchApp(&profile_a_, extension_a_.get(), some_file));
+  EXPECT_CALL(*delegate_, LaunchApp(&profile_a_, kTestAppIdA, some_file));
   host_aa_->FilesOpened(some_file);
 
   // Process disconnect will cause the host to be deleted.
@@ -693,14 +678,14 @@ TEST_F(ExtensionAppShimHandlerTest, FailToLaunch) {
   ShimLaunchedCallback launch_callback;
   delegate_->SetCaptureShimLaunchedCallback(&launch_callback);
   delegate_->SetHostForCreate(std::move(host_aa_unique_));
-  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, extension_a_.get(), false));
+  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, kTestAppIdA, false));
   handler_->OnAppActivated(&profile_a_, kTestAppIdA);
   EXPECT_EQ(host_aa_.get(), handler_->FindHost(&profile_a_, kTestAppIdA));
   EXPECT_TRUE(launch_callback);
 
   // Run the callback claiming that the launch failed. This should trigger
   // another launch, this time forcing shim recreation.
-  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, extension_a_.get(), true));
+  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, kTestAppIdA, true));
   std::move(launch_callback).Run(base::Process());
   EXPECT_TRUE(launch_callback);
 
@@ -718,7 +703,7 @@ TEST_F(ExtensionAppShimHandlerTest, FailToConnect) {
   delegate_->SetCaptureShimTerminatedCallback(&terminated_callback);
 
   delegate_->SetHostForCreate(std::move(host_aa_unique_));
-  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, extension_a_.get(), false));
+  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, kTestAppIdA, false));
   handler_->OnAppActivated(&profile_a_, kTestAppIdA);
   EXPECT_EQ(host_aa_.get(), handler_->FindHost(&profile_a_, kTestAppIdA));
   EXPECT_TRUE(launched_callback);
@@ -731,7 +716,7 @@ TEST_F(ExtensionAppShimHandlerTest, FailToConnect) {
 
   // Report that the process terminated. This should trigger a re-create and
   // re-launch.
-  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, extension_a_.get(), true));
+  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, kTestAppIdA, true));
   std::move(terminated_callback).Run();
   EXPECT_TRUE(launched_callback);
   EXPECT_TRUE(terminated_callback);
@@ -757,7 +742,7 @@ TEST_F(ExtensionAppShimHandlerTest, FailCodeSignature) {
 
   // Fail to code-sign. This should result in a host being created, and a launch
   // having been requested.
-  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, extension_a_.get(), false));
+  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, kTestAppIdA, false));
   NormalLaunch(bootstrap_aa_, std::move(host_aa_unique_));
   EXPECT_EQ(host_aa_.get(), handler_->FindHost(&profile_a_, kTestAppIdA));
   EXPECT_TRUE(launched_callback);
@@ -776,7 +761,7 @@ TEST_F(ExtensionAppShimHandlerTest, FailCodeSignature) {
 
   // Simulate the termination after the register failed.
   handler_->SetAcceptablyCodeSigned(true);
-  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, extension_a_.get(), true));
+  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, kTestAppIdA, true));
   std::move(terminated_callback).Run();
   EXPECT_TRUE(launched_callback);
   EXPECT_TRUE(terminated_callback);
@@ -796,11 +781,6 @@ TEST_F(ExtensionAppShimHandlerTest, MaybeTerminate) {
             *bootstrap_ab_result_);
   EXPECT_EQ(host_ab_.get(), handler_->FindHost(&profile_a_, kTestAppIdB));
 
-  // Return empty window list.
-  AppWindowList app_window_list;
-  EXPECT_CALL(*delegate_, GetWindows(_, _))
-      .WillRepeatedly(Return(app_window_list));
-
   // Quitting when there's another shim should not terminate.
   EXPECT_CALL(*delegate_, MaybeTerminate())
       .Times(0);
@@ -856,11 +836,6 @@ TEST_F(ExtensionAppShimHandlerTest, LoadProfile) {
 TEST_F(ExtensionAppShimHandlerTest, ExtensionUninstalled) {
   LaunchAndActivate(bootstrap_aa_, std::move(host_aa_unique_), &profile_a_);
 
-  // Have GetWindows() return an empty window list for focus (otherwise, it
-  // will contain a single nullptr, which can't be focused). Expect 1 call only.
-  AppWindowList empty_window_list;
-  EXPECT_CALL(*delegate_, GetWindows(_, _)).WillOnce(Return(empty_window_list));
-
   ShimNormalFocus(host_aa_.get());
   EXPECT_NE(nullptr, host_aa_.get());
 
@@ -885,8 +860,7 @@ TEST_F(ExtensionAppShimHandlerTest, PreExistingHost) {
   // Launch the app for this host. It should find the pre-existing host, and the
   // pre-existing host's launch result should be set.
   EXPECT_CALL(*handler_, OnShimFocus(host_aa_.get())).Times(1);
-  EXPECT_CALL(*delegate_, LaunchApp(&profile_a_, extension_a_.get(), _))
-      .Times(0);
+  EXPECT_CALL(*delegate_, LaunchApp(&profile_a_, kTestAppIdA, _)).Times(0);
   EXPECT_FALSE(host_aa_->did_connect_to_host());
   DoShimLaunch(bootstrap_aa_, nullptr,
                chrome::mojom::AppShimLaunchType::kRegisterOnly,
@@ -951,7 +925,7 @@ TEST_F(ExtensionAppShimHandlerTest, MultiProfileShimLaunch) {
   delegate_->SetCaptureShimTerminatedCallback(&terminated_callback);
 
   // Launch the app for profile A. This should trigger a shim launch request.
-  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, extension_a_.get(),
+  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, kTestAppIdA,
                                        false /* recreate_shim */));
   EXPECT_EQ(nullptr, handler_->FindHost(&profile_a_, kTestAppIdA));
   handler_->OnAppActivated(&profile_a_, kTestAppIdA);
@@ -978,7 +952,7 @@ TEST_F(ExtensionAppShimHandlerTest, MultiProfileSelectMenu) {
   delegate_->SetCaptureShimTerminatedCallback(&terminated_callback);
 
   // Launch the app for profile A. This should trigger a shim launch request.
-  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, extension_a_.get(),
+  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, kTestAppIdA,
                                        false /* recreate_shim */));
   EXPECT_EQ(nullptr, handler_->FindHost(&profile_a_, kTestAppIdA));
   handler_->OnAppActivated(&profile_a_, kTestAppIdA);
@@ -994,7 +968,7 @@ TEST_F(ExtensionAppShimHandlerTest, MultiProfileSelectMenu) {
 
   // Select profile B from the menu. This should request that the app be
   // launched.
-  EXPECT_CALL(*delegate_, LaunchApp(&profile_b_, extension_a_.get(), _));
+  EXPECT_CALL(*delegate_, LaunchApp(&profile_b_, kTestAppIdA, _));
   host_aa_->ProfileSelectedFromMenu(profile_path_b_);
   EXPECT_CALL(*delegate_, DoLaunchShim(_, _, _)).Times(0);
   handler_->OnAppActivated(&profile_b_, kTestAppIdA);
@@ -1024,13 +998,12 @@ TEST_F(ExtensionAppShimHandlerTest, ProfileMenuOneProfile) {
   // When the app activates, a host is created. This will trigger building
   // the avatar menu.
   delegate_->SetHostForCreate(std::move(host_aa_unique_));
-  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, extension_a_.get(), false));
+  EXPECT_CALL(*delegate_, DoLaunchShim(&profile_a_, kTestAppIdA, false));
   handler_->OnAppActivated(&profile_a_, kTestAppIdA);
   EXPECT_EQ(host_aa_.get(), handler_->FindHost(&profile_a_, kTestAppIdA));
 
   // Launch the shim.
-  EXPECT_CALL(*delegate_, LaunchApp(&profile_a_, extension_a_.get(), _))
-      .Times(0);
+  EXPECT_CALL(*delegate_, LaunchApp(&profile_a_, kTestAppIdA, _)).Times(0);
   RegisterOnlyLaunch(bootstrap_aa_, nullptr);
   EXPECT_EQ(chrome::mojom::AppShimLaunchResult::kSuccess,
             *bootstrap_aa_result_);
@@ -1084,10 +1057,8 @@ TEST_F(ExtensionAppShimHandlerTest, FindProfileFromBadProfile) {
 
   // Launch the shim requesting profile C.
   delegate_->SetHostForCreate(std::move(host_aa_unique_));
-  EXPECT_CALL(*delegate_, LaunchApp(&profile_a_, extension_a_.get(), _))
-      .Times(1);
-  EXPECT_CALL(*delegate_, LaunchApp(&profile_b_, extension_a_.get(), _))
-      .Times(0);
+  EXPECT_CALL(*delegate_, LaunchApp(&profile_a_, kTestAppIdA, _)).Times(1);
+  EXPECT_CALL(*delegate_, LaunchApp(&profile_b_, kTestAppIdA, _)).Times(0);
   EXPECT_CALL(*delegate_, EnableExtension(&profile_c_, kTestAppIdA, _))
       .WillOnce(RunOnceCallback<2>());
   NormalLaunch(bootstrap_ca_, nullptr);
@@ -1103,10 +1074,8 @@ TEST_F(ExtensionAppShimHandlerTest, FindProfileFromNoProfile) {
 
   // Launch the shim without specifying a profile.
   delegate_->SetHostForCreate(std::move(host_aa_unique_));
-  EXPECT_CALL(*delegate_, LaunchApp(&profile_a_, extension_a_.get(), _))
-      .Times(1);
-  EXPECT_CALL(*delegate_, LaunchApp(&profile_b_, extension_a_.get(), _))
-      .Times(0);
+  EXPECT_CALL(*delegate_, LaunchApp(&profile_a_, kTestAppIdA, _)).Times(1);
+  EXPECT_CALL(*delegate_, LaunchApp(&profile_b_, kTestAppIdA, _)).Times(0);
   NormalLaunch(bootstrap_xa_, nullptr);
   EXPECT_EQ(chrome::mojom::AppShimLaunchResult::kSuccess,
             *bootstrap_xa_result_);

chrome/browser/ui/views/apps/chrome_native_app_window_views_mac.mm

@@ -8,6 +8,7 @@
 
 #import "base/mac/scoped_nsobject.h"
 #include "chrome/browser/apps/app_shim/extension_app_shim_handler_mac.h"
+#include "chrome/browser/profiles/profile.h"
 #import "chrome/browser/ui/views/apps/app_window_native_widget_mac.h"
 #import "chrome/browser/ui/views/apps/native_app_window_frame_view_mac.h"
 #import "ui/gfx/mac/coordinate_conversion.h"
@@ -165,9 +166,15 @@ void ChromeNativeAppWindowViewsMac::Restore() {
 }
 
 void ChromeNativeAppWindowViewsMac::FlashFrame(bool flash) {
-  apps::ExtensionAppShimHandler::Get()->RequestUserAttentionForWindow(
-      app_window(), flash ? chrome::mojom::AppShimAttentionType::kCritical
-                          : chrome::mojom::AppShimAttentionType::kCancel);
+  Profile* profile =
+      Profile::FromBrowserContext(app_window()->browser_context());
+  AppShimHost* shim_host = apps::ExtensionAppShimHandler::Get()->FindHost(
+      profile, app_window()->extension_id());
+  if (!shim_host)
+    return;
+  shim_host->GetAppShim()->SetUserAttention(
+      flash ? chrome::mojom::AppShimAttentionType::kCritical
+            : chrome::mojom::AppShimAttentionType::kCancel);
 }
 
 void ChromeNativeAppWindowViewsMac::OnWidgetCreated(views::Widget* widget) {