Set the correct InstanceState for Chrome.

When the app_id is empty from web contents, the tab could be an webpage
without app_id, so set the app_id as Chrome app_id, and set the correct
InstanceState for Chrome:
1. When the tab is active, it should be started, running and visible.
2. When the tab or an app is inactive, it should be started, running.

This CL also fix a crash bug which is caused by AppWindowBase smart
pointer.

Update AppServiceAppWindowLauncherController and browser_status_monitor
init order, so when browser status is updated, AppService app window
can be updated with the function OnInstanceUpdate.

BUG=1011235

Change-Id: Ie5146d44517f844d9bced6812b4c4c6e83930845
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1959191
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723592}

chrome/browser/ui/ash/launcher/app_service_app_window_launcher_controller.cc

@@ -257,13 +257,15 @@ void AppServiceAppWindowLauncherController::UnregisterWindow(
 void AppServiceAppWindowLauncherController::AddWindowToShelf(
     aura::Window* window,
     const ash::ShelfID& shelf_id) {
-  base::Contains(aura_window_to_app_window_, window);
+  if (base::Contains(aura_window_to_app_window_, window))
+    return;
 
   auto app_window_ptr = std::make_unique<AppWindowBase>(
       shelf_id, views::Widget::GetWidgetForNativeWindow(window));
+  AppWindowBase* app_window = app_window_ptr.get();
   aura_window_to_app_window_[window] = std::move(app_window_ptr);
 
-  AddAppWindowToShelf(app_window_ptr.get());
+  AddAppWindowToShelf(app_window);
 }
 
 void AppServiceAppWindowLauncherController::SetWindowActivated(
@@ -277,20 +279,25 @@ void AppServiceAppWindowLauncherController::SetWindowActivated(
     return;
 
   apps::InstanceState state = apps::InstanceState::kUnknown;
-  proxy_->InstanceRegistry().ForOneInstance(
-      window,
-      [&state](const apps::InstanceUpdate& update) { state = update.State(); });
-
-  // When sets the instance active state, the instance should be in started and
-  // running state.
-  state = static_cast<apps::InstanceState>(
-      state | apps::InstanceState::kStarted | apps::InstanceState::kRunning);
-
-  state = (active)
-              ? static_cast<apps::InstanceState>(state |
-                                                 apps::InstanceState::kActive)
-              : static_cast<apps::InstanceState>(state &
-                                                 ~apps::InstanceState::kActive);
+  if (active) {
+    // If the app is active, it should be started, running, and visible.
+    state = static_cast<apps::InstanceState>(
+        apps::InstanceState::kStarted | apps::InstanceState::kRunning |
+        apps::InstanceState::kActive | apps::InstanceState::kVisible);
+  } else {
+    proxy_->InstanceRegistry().ForOneInstance(
+        window, [&state](const apps::InstanceUpdate& update) {
+          state = update.State();
+        });
+
+    // When sets the instance active state, the instance should be in started
+    // and running state.
+    state = static_cast<apps::InstanceState>(
+        state | apps::InstanceState::kStarted | apps::InstanceState::kRunning);
+
+    state =
+        static_cast<apps::InstanceState>(state & ~apps::InstanceState::kActive);
+  }
   app_service_instance_helper_->OnInstances(shelf_id.app_id, window,
                                             std::string(), state);
 }
@@ -417,7 +424,8 @@ ash::ShelfID AppServiceAppWindowLauncherController::GetShelfId(
 
   if (!shelf_id.IsNull()) {
     if (proxy_->AppRegistryCache().GetAppType(shelf_id.app_id) ==
-        apps::mojom::AppType::kUnknown) {
+            apps::mojom::AppType::kUnknown &&
+        shelf_id.app_id != extension_misc::kChromeAppId) {
       return ash::ShelfID();
     }
     return shelf_id;

chrome/browser/ui/ash/launcher/app_service_instance_registry_helper.cc

@@ -19,6 +19,7 @@
 #include "chrome/services/app_service/public/cpp/instance_update.h"
 #include "chrome/services/app_service/public/mojom/types.mojom.h"
 #include "content/public/browser/web_contents.h"
+#include "extensions/common/constants.h"
 
 AppServiceInstanceRegistryHelper::AppServiceInstanceRegistryHelper(
     Profile* profile)
@@ -43,29 +44,33 @@ void AppServiceInstanceRegistryHelper::OnActiveTabChanged(
     return;
 
   if (old_contents) {
-    apps::InstanceState state = apps::InstanceState::kUnknown;
-    proxy_->InstanceRegistry().ForOneInstance(
-        old_contents->GetNativeView(),
-        [&state](const apps::InstanceUpdate& update) {
-          state = update.State();
-        });
-    state =
-        static_cast<apps::InstanceState>(state & ~apps::InstanceState::kActive);
-    OnInstances(launcher_controller_helper_->GetAppID(old_contents),
-                GetWindow(old_contents), std::string(), state);
+    std::string app_id = launcher_controller_helper_->GetAppID(old_contents);
+    // If app_id is empty, we should not set it as inactive because this is
+    // Chrome's tab.
+    if (!app_id.empty()) {
+      apps::InstanceState state = apps::InstanceState::kUnknown;
+      proxy_->InstanceRegistry().ForOneInstance(
+          old_contents->GetNativeView(),
+          [&state](const apps::InstanceUpdate& update) {
+            state = update.State();
+          });
+      // If the app has been inactive, we don't need to update the instance.
+      if ((state & apps::InstanceState::kActive) !=
+          apps::InstanceState::kUnknown) {
+        state = static_cast<apps::InstanceState>(state &
+                                                 ~apps::InstanceState::kActive);
+        OnInstances(app_id, GetWindow(old_contents), std::string(), state);
+      }
+    }
   }
 
   if (new_contents) {
-    apps::InstanceState state = apps::InstanceState::kUnknown;
-    proxy_->InstanceRegistry().ForOneInstance(
-        new_contents->GetNativeView(),
-        [&state](const apps::InstanceUpdate& update) {
-          state = update.State();
-        });
-    state =
-        static_cast<apps::InstanceState>(state | apps::InstanceState::kActive);
-    OnInstances(launcher_controller_helper_->GetAppID(new_contents),
-                GetWindow(new_contents), std::string(), state);
+    // If the app is active, it should be started, running, and visible.
+    apps::InstanceState state = static_cast<apps::InstanceState>(
+        apps::InstanceState::kStarted | apps::InstanceState::kRunning |
+        apps::InstanceState::kActive | apps::InstanceState::kVisible);
+    OnInstances(GetAppId(new_contents), GetWindow(new_contents), std::string(),
+                state);
   }
 }
 
@@ -85,10 +90,8 @@ void AppServiceInstanceRegistryHelper::OnTabInserted(
     return;
 
   apps::InstanceState state = static_cast<apps::InstanceState>(
-      apps::InstanceState::kStarted | apps::InstanceState::kRunning |
-      apps::InstanceState::kActive | apps::InstanceState::kVisible);
-  OnInstances(launcher_controller_helper_->GetAppID(contents),
-              GetWindow(contents), std::string(), state);
+      apps::InstanceState::kStarted | apps::InstanceState::kRunning);
+  OnInstances(GetAppId(contents), GetWindow(contents), std::string(), state);
 }
 
 void AppServiceInstanceRegistryHelper::OnTabClosing(
@@ -96,8 +99,12 @@ void AppServiceInstanceRegistryHelper::OnTabClosing(
   if (!base::FeatureList::IsEnabled(features::kAppServiceInstanceRegistry))
     return;
 
-  OnInstances(launcher_controller_helper_->GetAppID(contents),
-              GetWindow(contents), std::string(),
+  std::string app_id = launcher_controller_helper_->GetAppID(contents);
+  // If app_id is empty, we should monitor the browser for Chrome.
+  if (app_id.empty())
+    return;
+
+  OnInstances(app_id, GetWindow(contents), std::string(),
               apps::InstanceState::kDestroyed);
 }
 
@@ -135,11 +142,19 @@ bool AppServiceInstanceRegistryHelper::IsWebApp(
   return false;
 }
 
+std::string AppServiceInstanceRegistryHelper::GetAppId(
+    content::WebContents* contents) const {
+  std::string app_id = launcher_controller_helper_->GetAppID(contents);
+  if (!app_id.empty())
+    return app_id;
+  return extension_misc::kChromeAppId;
+}
+
 aura::Window* AppServiceInstanceRegistryHelper::GetWindow(
     content::WebContents* contents) {
   std::string app_id = launcher_controller_helper_->GetAppID(contents);
   aura::Window* window = contents->GetNativeView();
-  if (IsWebApp(app_id))
+  if (app_id.empty() || IsWebApp(app_id))
     window = window->GetToplevelWindow();
   return window;
 }

chrome/browser/ui/ash/launcher/app_service_instance_registry_helper.h

@@ -60,6 +60,7 @@ class AppServiceInstanceRegistryHelper {
   bool IsWebApp(const std::string& app_id) const;
 
  private:
+  std::string GetAppId(content::WebContents* contents) const;
   aura::Window* GetWindow(content::WebContents* contents);
 
   apps::AppServiceProxy* proxy_ = nullptr;

chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc

@@ -258,6 +258,11 @@ ChromeLauncherController::ChromeLauncherController(Profile* profile,
   }
 
   if (base::FeatureList::IsEnabled(features::kAppServiceInstanceRegistry)) {
+    std::unique_ptr<AppServiceAppWindowLauncherController>
+        app_service_controller =
+            std::make_unique<AppServiceAppWindowLauncherController>(this);
+    app_service_app_window_controller_ = app_service_controller.get();
+    app_window_controllers_.emplace_back(std::move(app_service_controller));
     if (SessionControllerClientImpl::IsMultiProfileAvailable()) {
       // If running in separated destkop mode, we create the multi profile
       // version of status monitor.
@@ -270,11 +275,6 @@ ChromeLauncherController::ChromeLauncherController(Profile* profile,
       browser_status_monitor_ = std::make_unique<BrowserStatusMonitor>(this);
       browser_status_monitor_->Initialize();
     }
-    std::unique_ptr<AppServiceAppWindowLauncherController>
-        app_service_controller =
-            std::make_unique<AppServiceAppWindowLauncherController>(this);
-    app_service_app_window_controller_ = app_service_controller.get();
-    app_window_controllers_.emplace_back(std::move(app_service_controller));
     return;
   }