Create explicit AppState/ProfileState structures in AppShimHandler

The next steps here will be to allow the AppShimHost to be owned either
by the ProfileState (non-multi-profile-mode) or the AppState (multi-
profile mode).

Bug: 982024
Change-Id: Idbc2de4927895800a79e6300a6dbf24fc3ff652e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1771945Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690922}

chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc

@@ -237,6 +237,19 @@ bool UsesRemoteViews(const extensions::Extension* extension) {
 
 namespace apps {
 
+ExtensionAppShimHandler::ProfileState::ProfileState() = default;
+ExtensionAppShimHandler::ProfileState::ProfileState(ProfileState&& other) =
+    default;
+ExtensionAppShimHandler::ProfileState& ExtensionAppShimHandler::ProfileState::
+operator=(ProfileState&& other) = default;
+ExtensionAppShimHandler::ProfileState::~ProfileState() = default;
+
+ExtensionAppShimHandler::AppState::AppState() = default;
+ExtensionAppShimHandler::AppState::AppState(AppState&& other) = default;
+ExtensionAppShimHandler::AppState& ExtensionAppShimHandler::AppState::operator=(
+    AppState&& other) = default;
+ExtensionAppShimHandler::AppState::~AppState() = default;
+
 base::FilePath ExtensionAppShimHandler::Delegate::GetFullProfilePath(
     const base::FilePath& relative_profile_path) {
   ProfileManager* profile_manager = g_browser_process->profile_manager();
@@ -396,8 +409,15 @@ ExtensionAppShimHandler::~ExtensionAppShimHandler() {
 
 AppShimHost* ExtensionAppShimHandler::FindHost(Profile* profile,
                                                const std::string& app_id) {
-  HostMap::iterator it = hosts_.find(make_pair(profile, app_id));
-  return it == hosts_.end() ? nullptr : it->second.get();
+  auto found_app = apps_.find(app_id);
+  if (found_app == apps_.end())
+    return nullptr;
+  AppState& app_state = found_app->second;
+  auto found_profile = app_state.profiles.find(profile);
+  if (found_profile == app_state.profiles.end())
+    return nullptr;
+  ProfileState& profile_state = found_profile->second;
+  return profile_state.host.get();
 }
 
 AppShimHost* ExtensionAppShimHandler::FindOrCreateHost(
@@ -405,14 +425,10 @@ AppShimHost* ExtensionAppShimHandler::FindOrCreateHost(
     const extensions::Extension* extension) {
   if (web_app::AppShimLaunchDisabled())
     return nullptr;
-  auto key = make_pair(profile, extension->id());
-  auto it = hosts_.find(key);
-  if (it == hosts_.end()) {
-    std::unique_ptr<AppShimHost> new_host =
-        delegate_->CreateHost(profile, extension);
-    it = hosts_.emplace(make_pair(key, std::move(new_host))).first;
-  }
-  return it->second.get();
+  ProfileState& profile_state = apps_[extension->id()].profiles[profile];
+  if (!profile_state.host)
+    profile_state.host = delegate_->CreateHost(profile, extension);
+  return profile_state.host.get();
 }
 
 AppShimHost* ExtensionAppShimHandler::GetHostForBrowser(Browser* browser) {
@@ -546,8 +562,13 @@ void ExtensionAppShimHandler::OnChromeWillHide() {
   // Send OnAppHide to all the shims so that they go into the hidden state.
   // This is necessary so that when the shim is next focused, it will know to
   // unhide.
-  for (auto& entry : hosts_)
-    entry.second->OnAppHide();
+  for (auto& iter_app : apps_) {
+    AppState& app_state = iter_app.second;
+    for (auto& iter_profile : app_state.profiles) {
+      ProfileState& profile_state = iter_profile.second;
+      profile_state.host->OnAppHide();
+    }
+  }
 }
 
 void ExtensionAppShimHandler::OnShimLaunchRequested(
@@ -624,10 +645,8 @@ const Extension* ExtensionAppShimHandler::MaybeGetExtensionOrCloseHost(
 
   const Extension* extension =
       delegate_->MaybeGetAppExtension(profile, host->GetAppId());
-  if (!extension) {
-    auto key = std::make_pair(profile, host->GetAppId());
-    hosts_.erase(key);
-  }
+  if (!extension)
+    CloseShimForApp(profile, host->GetAppId());
 
   if (profile_out)
     *profile_out = profile;
@@ -645,6 +664,28 @@ void ExtensionAppShimHandler::CloseBrowsersForApp(Profile* profile,
     browser->window()->Close();
 }
 
+void ExtensionAppShimHandler::CloseShimsForProfile(Profile* profile) {
+  for (auto iter_app = apps_.begin(); iter_app != apps_.end();) {
+    AppState& app_state = iter_app->second;
+    app_state.profiles.erase(profile);
+    if (app_state.profiles.empty())
+      iter_app = apps_.erase(iter_app);
+    else
+      ++iter_app;
+  }
+}
+
+void ExtensionAppShimHandler::CloseShimForApp(Profile* profile,
+                                              const std::string& app_id) {
+  auto found_app = apps_.find(app_id);
+  if (found_app == apps_.end())
+    return;
+  AppState& app_state = found_app->second;
+  app_state.profiles.erase(profile);
+  if (app_state.profiles.empty())
+    apps_.erase(found_app);
+}
+
 void ExtensionAppShimHandler::OnProfileLoaded(
     std::unique_ptr<AppShimHostBootstrap> bootstrap,
     Profile* profile) {
@@ -735,10 +776,18 @@ bool ExtensionAppShimHandler::IsAcceptablyCodeSigned(pid_t pid) const {
 void ExtensionAppShimHandler::OnShimProcessDisconnected(AppShimHost* host) {
   // This might be called when shutting down. Don't try to look up the profile
   // since profile_manager might not be around.
-  for (HostMap::iterator it = hosts_.begin(); it != hosts_.end(); ) {
-    HostMap::iterator current = it++;
-    if (current->second.get() == host)
-      hosts_.erase(current);
+  for (auto iter_app = apps_.begin(); iter_app != apps_.end(); ++iter_app) {
+    AppState& app_state = iter_app->second;
+    for (auto iter_profile = app_state.profiles.begin();
+         iter_profile != app_state.profiles.end(); ++iter_profile) {
+      ProfileState& profile_state = iter_profile->second;
+      if (profile_state.host.get() == host) {
+        app_state.profiles.erase(iter_profile);
+        if (app_state.profiles.empty())
+          apps_.erase(iter_app);
+        return;
+      }
+    }
   }
 }
 
@@ -844,13 +893,7 @@ void ExtensionAppShimHandler::Observe(
 
       AppLifetimeMonitorFactory::GetForBrowserContext(profile)->RemoveObserver(
           this);
-      // Shut down every shim associated with this profile.
-      for (HostMap::iterator it = hosts_.begin(); it != hosts_.end(); ) {
-        if (profile->IsSameProfile(it->first.first))
-          it = hosts_.erase(it);
-        else
-          ++it;
-      }
+      CloseShimsForProfile(profile);
       break;
     }
     case chrome::NOTIFICATION_BROWSER_OPENED: {
@@ -893,9 +936,8 @@ void ExtensionAppShimHandler::OnAppActivated(content::BrowserContext* context,
 
 void ExtensionAppShimHandler::OnAppDeactivated(content::BrowserContext* context,
                                                const std::string& app_id) {
-  auto key = std::make_pair(static_cast<Profile*>(context), app_id);
-  hosts_.erase(key);
-  if (hosts_.empty())
+  CloseShimForApp(static_cast<Profile*>(context), app_id);
+  if (apps_.empty())
     delegate_->MaybeTerminate();
 }
 

chrome/browser/apps/app_shim/extension_app_shim_handler_mac.h

@@ -171,9 +171,6 @@ class ExtensionAppShimHandler : public AppShimHandler,
   void OnBrowserRemoved(Browser* browser) override;
 
  protected:
-  typedef std::map<std::pair<Profile*, std::string>,
-                   std::unique_ptr<AppShimHost>>
-      HostMap;
   typedef std::set<Browser*> BrowserSet;
   typedef std::map<std::pair<Profile*, std::string>, BrowserSet> AppBrowserMap;
 
@@ -196,6 +193,12 @@ class ExtensionAppShimHandler : public AppShimHandler,
   // Closes all browsers associated with an app.
   void CloseBrowsersForApp(Profile* profile, const std::string& app_id);
 
+  // Close all app shims associated with the specified profile.
+  void CloseShimsForProfile(Profile* profile);
+
+  // Close one specified app.
+  void CloseShimForApp(Profile* profile, const std::string& app_id);
+
   // This is passed to Delegate::LoadProfileAsync for shim-initiated launches
   // where the profile was not yet loaded.
   void OnProfileLoaded(std::unique_ptr<AppShimHostBootstrap> bootstrap,
@@ -207,7 +210,39 @@ class ExtensionAppShimHandler : public AppShimHandler,
 
   std::unique_ptr<Delegate> delegate_;
 
-  HostMap hosts_;
+  // The state for an individual (app, Profile) pair. This includes the
+  // AppShimHost.
+  // TODO(https://crbug.com/982024): Add browser windows.
+  struct ProfileState {
+    ProfileState();
+    ProfileState(ProfileState&& other);
+    ProfileState& operator=(ProfileState&& other);
+    ~ProfileState();
+
+    std::unique_ptr<AppShimHost> host;
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(ProfileState);
+  };
+
+  // The state for an individual app. This includes the state for all
+  // profiles that are using the app.
+  // TODO(https://crbug.com/982024): Add AppShimHost when shared amongst
+  // profiles.
+  struct AppState {
+    AppState();
+    AppState(AppState&& other);
+    AppState& operator=(AppState&& other);
+    ~AppState();
+
+    std::map<Profile*, ProfileState> profiles;
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(AppState);
+  };
+
+  // Map from extension id to the state for that app.
+  std::map<std::string, AppState> apps_;
 
   // A map of app ids to associated browser windows.
   AppBrowserMap app_browser_windows_;