Modify observe all window to observe browser tabs only.

Sometimes, when shutdown the system, the instance could be updated, but
the window might have been destroyed. Also ARC CTS might have the
unusual event flow to set the task active when the window is killed. So
observing all windows when update the instance could cause crash.

Actually CL:2307669 added observing all windows mainly because browser
tabs are not observed. So redo CL:2307669:
1. Observe browser tabs during the system init phase
2. Observe the new inserted tab

To avoid crash, OnInstance checks whether the window is observed, and
if not, remove the instance.

BUG=1116041
BUG=1116033
b:163005244

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

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

@@ -79,8 +79,21 @@ AppServiceAppWindowLauncherController::AppServiceAppWindowLauncherController(
   profile_list_.push_back(owner->profile());
 
   for (auto* browser : *BrowserList::GetInstance()) {
-    if (browser && browser->window() && browser->window()->GetNativeWindow())
+    if (browser && browser->window() && browser->window()->GetNativeWindow()) {
       observed_windows_.Add(browser->window()->GetNativeWindow());
+
+      // Observe the browser tabs
+      TabStripModel* tab_strip = browser->tab_strip_model();
+      for (int i = 0; i < tab_strip->count(); ++i) {
+        auto* tab = tab_strip->GetWebContentsAt(i);
+        if (!tab)
+          continue;
+        aura::Window* window = tab->GetNativeView();
+        if (window) {
+          observed_windows_.Add(window);
+        }
+      }
+    }
   }
 }
 
@@ -94,6 +107,9 @@ AppServiceAppWindowLauncherController::
         apps::AppServiceProxyFactory::GetForProfile(profile);
     proxy->InstanceRegistry().RemoveObserver(this);
   }
+
+  app_service_instance_helper_.reset();
+  observed_windows_.RemoveAll();
 }
 
 AppWindowLauncherItemController*
@@ -427,6 +443,12 @@ void AppServiceAppWindowLauncherController::ObserveWindow(
   observed_windows_.Add(window);
 }
 
+bool AppServiceAppWindowLauncherController::IsObservingWindow(
+    aura::Window* window) {
+  DCHECK(window);
+  return observed_windows_.IsObserving(window);
+}
+
 std::vector<aura::Window*>
 AppServiceAppWindowLauncherController::GetArcWindows() {
   std::vector<aura::Window*> arc_windows;

chrome/browser/ui/ash/launcher/app_service/app_service_app_window_launcher_controller.h

@@ -100,6 +100,7 @@ class AppServiceAppWindowLauncherController
   AppWindowBase* GetAppWindow(aura::Window* window);
 
   void ObserveWindow(aura::Window* window);
+  bool IsObservingWindow(aura::Window* window);
 
   std::vector<aura::Window*> GetArcWindows();
 

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

@@ -134,6 +134,11 @@ void AppServiceInstanceRegistryHelper::OnTabInserted(
   UpdateTabWindow(app_id, window);
   apps::InstanceState state = static_cast<apps::InstanceState>(
       apps::InstanceState::kStarted | apps::InstanceState::kRunning);
+
+  // Observe the tab, because when the system is shutdown or some other cases,
+  // the window could be destroyed without calling OnTabClosing. So observe the
+  // tab to get the notify when the window is destroyed.
+  controller_->ObserveWindow(window);
   OnInstances(app_id, window, std::string(), state);
 }
 
@@ -179,11 +184,16 @@ void AppServiceInstanceRegistryHelper::OnInstances(const std::string& app_id,
                                                    aura::Window* window,
                                                    const std::string& launch_id,
                                                    apps::InstanceState state) {
-  if (app_id.empty())
+  if (app_id.empty() || !window)
     return;
 
-  if (state != apps::InstanceState::kDestroyed)
-    controller_->ObserveWindow(window);
+  // If the window is not observed, this means the window is being destroyed. In
+  // this case, don't add the instance because we might keep the record for the
+  // destroyed window, which could cause crash.
+  if (state != apps::InstanceState::kDestroyed &&
+      !controller_->IsObservingWindow(window)) {
+    state = apps::InstanceState::kDestroyed;
+  }
 
   std::unique_ptr<apps::Instance> instance =
       std::make_unique<apps::Instance>(app_id, window);