chromeos: Clean up BrowserStatusMonitor for ash shelf

For out-of-process ash code in //chrome/browser can't call into ash.
BrowserStatusMonitor was installing a global window activation observer
on ash::Shell. However, it doesn't need to do that anymore.

The code was written to change the brightness of the "running" dot we
show under the shelf icon for a v1 (non-packaged) app. However, with
the MD redesign of the shelf we always use the same dot so we don't
need to track activation state any more.

See removal of ash::STATUS_ACTIVE from enum ShelfItemStatus in
https://chromium-review.googlesource.com/c/chromium/src/+/758800

Bug: 850184, 557406
Test: browser_tests --gtest_filter=*Launcher*
Change-Id: I38ed5e563d8b52386c53fafd17174608cd4ef8d6
Reviewed-on: https://chromium-review.googlesource.com/1089914Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565119}

chrome/browser/ui/ash/launcher/DEPS

@@ -4,10 +4,6 @@ specific_include_rules = {
     "+ash/shell.h",
   ],
   # https://crbug.com/557406
-  "browser_status_monitor\.cc": [
-    "+ash/shell.h",
-  ],
-  # https://crbug.com/557406
   "chrome_launcher_controller\.cc": [
     "+ash/shell.h",
     "+ash/strings/grit/ash_strings.h",

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

@@ -7,7 +7,6 @@
 #include <memory>
 
 #include "ash/public/cpp/shelf_types.h"
-#include "ash/shell.h"
 #include "base/macros.h"
 #include "chrome/browser/ui/ash/ash_util.h"
 #include "chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.h"
@@ -23,9 +22,6 @@
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
-#include "ui/aura/window.h"
-#include "ui/aura/window_event_dispatcher.h"
-#include "ui/wm/public/activation_client.h"
 
 // This class monitors the WebContent of the all tab and notifies a navigation
 // to the BrowserStatusMonitor.
@@ -36,7 +32,7 @@ class BrowserStatusMonitor::LocalWebContentsObserver
                            BrowserStatusMonitor* monitor)
       : content::WebContentsObserver(contents), monitor_(monitor) {}
 
-  ~LocalWebContentsObserver() override {}
+  ~LocalWebContentsObserver() override = default;
 
   // content::WebContentsObserver
   void DidFinishNavigation(
@@ -45,21 +41,11 @@ class BrowserStatusMonitor::LocalWebContentsObserver
         !navigation_handle->HasCommitted())
       return;
 
-    ChromeLauncherController::AppState state =
-        ChromeLauncherController::APP_STATE_INACTIVE;
-    Browser* browser = chrome::FindBrowserWithWebContents(web_contents());
-    // Don't assume that |browser| still exists.
-    if (browser) {
-      if (browser->window()->IsActive() &&
-          browser->tab_strip_model()->GetActiveWebContents() == web_contents())
-        state = ChromeLauncherController::APP_STATE_WINDOW_ACTIVE;
-      else if (browser->window()->IsActive())
-        state = ChromeLauncherController::APP_STATE_ACTIVE;
-    }
-    monitor_->UpdateAppItemState(web_contents(), state);
+    monitor_->UpdateAppItemState(web_contents(), false /*remove*/);
     monitor_->UpdateBrowserItemState();
 
     // Navigating may change the ShelfID associated with the WebContents.
+    Browser* browser = chrome::FindBrowserWithWebContents(web_contents());
     if (browser &&
         browser->tab_strip_model()->GetActiveWebContents() == web_contents()) {
       monitor_->SetShelfIDForBrowserWindowContents(browser, web_contents());
@@ -84,14 +70,9 @@ BrowserStatusMonitor::BrowserStatusMonitor(
     : launcher_controller_(launcher_controller),
       browser_tab_strip_tracker_(this, this, this) {
   DCHECK(launcher_controller_);
-  // TODO(crbug.com/557406): Fix this interaction pattern in Mash.
-  if (ash::Shell::HasInstance())
-    ash::Shell::Get()->activation_client()->AddObserver(this);
 }
 
 BrowserStatusMonitor::~BrowserStatusMonitor() {
-  if (ash::Shell::HasInstance())
-    ash::Shell::Get()->activation_client()->RemoveObserver(this);
   browser_tab_strip_tracker_.StopObservingAndSendOnBrowserRemoved();
 }
 
@@ -101,18 +82,18 @@ void BrowserStatusMonitor::Initialize() {
   browser_tab_strip_tracker_.Init();
 }
 
-void BrowserStatusMonitor::UpdateAppItemState(
-    content::WebContents* contents,
-    ChromeLauncherController::AppState app_state) {
+void BrowserStatusMonitor::UpdateAppItemState(content::WebContents* contents,
+                                              bool remove) {
   DCHECK(contents);
   DCHECK(initialized_);
   // It is possible to come here from Browser::SwapTabContent where the contents
   // cannot be associated with a browser. A removal however should be properly
   // processed.
   Browser* browser = chrome::FindBrowserWithWebContents(contents);
-  if (app_state == ChromeLauncherController::APP_STATE_REMOVED ||
-      (browser && multi_user_util::IsProfileFromActiveUser(browser->profile())))
-    launcher_controller_->UpdateAppState(contents, app_state);
+  if (remove || (browser && multi_user_util::IsProfileFromActiveUser(
+                                browser->profile()))) {
+    launcher_controller_->UpdateAppState(contents, remove);
+  }
 }
 
 void BrowserStatusMonitor::UpdateBrowserItemState() {
@@ -121,40 +102,6 @@ void BrowserStatusMonitor::UpdateBrowserItemState() {
       ->UpdateBrowserItemState();
 }
 
-void BrowserStatusMonitor::OnWindowActivated(
-    wm::ActivationChangeObserver::ActivationReason reason,
-    aura::Window* gained_active,
-    aura::Window* lost_active) {
-  DCHECK(initialized_);
-  Browser* browser = NULL;
-  content::WebContents* contents_from_gained = NULL;
-  content::WebContents* contents_from_lost = NULL;
-  // Update active webcontents's app item state of |lost_active|, if existed.
-  if (lost_active) {
-    browser = chrome::FindBrowserWithWindow(lost_active);
-    if (browser)
-      contents_from_lost = browser->tab_strip_model()->GetActiveWebContents();
-    if (contents_from_lost) {
-      UpdateAppItemState(contents_from_lost,
-                         ChromeLauncherController::APP_STATE_INACTIVE);
-    }
-  }
-
-  // Update active webcontents's app item state of |gained_active|, if existed.
-  if (gained_active) {
-    browser = chrome::FindBrowserWithWindow(gained_active);
-    if (browser)
-      contents_from_gained = browser->tab_strip_model()->GetActiveWebContents();
-    if (contents_from_gained) {
-      UpdateAppItemState(contents_from_gained,
-                         ChromeLauncherController::APP_STATE_WINDOW_ACTIVE);
-    }
-  }
-
-  if (contents_from_lost || contents_from_gained)
-    UpdateBrowserItemState();
-}
-
 bool BrowserStatusMonitor::ShouldTrackBrowser(Browser* browser) {
   return true;
 }
@@ -186,20 +133,15 @@ void BrowserStatusMonitor::ActiveTabChanged(content::WebContents* old_contents,
   DCHECK(new_contents);
   browser = chrome::FindBrowserWithWebContents(new_contents);
 
-  ChromeLauncherController::AppState state =
-      ChromeLauncherController::APP_STATE_INACTIVE;
-
   // Update immediately on a tab change.
   if (old_contents &&
       (TabStripModel::kNoTab !=
-       browser->tab_strip_model()->GetIndexOfWebContents(old_contents)))
-    UpdateAppItemState(old_contents, state);
+       browser->tab_strip_model()->GetIndexOfWebContents(old_contents))) {
+    UpdateAppItemState(old_contents, false /*remove*/);
+  }
 
   if (new_contents) {
-    state = browser->window()->IsActive()
-                ? ChromeLauncherController::APP_STATE_WINDOW_ACTIVE
-                : ChromeLauncherController::APP_STATE_ACTIVE;
-    UpdateAppItemState(new_contents, state);
+    UpdateAppItemState(new_contents, false /*remove*/);
     UpdateBrowserItemState();
     SetShelfIDForBrowserWindowContents(browser, new_contents);
   }
@@ -212,15 +154,10 @@ void BrowserStatusMonitor::TabReplacedAt(TabStripModel* tab_strip_model,
   DCHECK(old_contents && new_contents);
   Browser* browser = chrome::FindBrowserWithWebContents(new_contents);
 
-  UpdateAppItemState(old_contents, ChromeLauncherController::APP_STATE_REMOVED);
+  UpdateAppItemState(old_contents, true /*remove*/);
   RemoveWebContentsObserver(old_contents);
 
-  ChromeLauncherController::AppState state =
-      ChromeLauncherController::APP_STATE_ACTIVE;
-  if (browser->window()->IsActive() &&
-      (tab_strip_model->GetActiveWebContents() == new_contents))
-    state = ChromeLauncherController::APP_STATE_WINDOW_ACTIVE;
-  UpdateAppItemState(new_contents, state);
+  UpdateAppItemState(new_contents, false /*remove*/);
   UpdateBrowserItemState();
 
   if (tab_strip_model->GetActiveWebContents() == new_contents)
@@ -233,22 +170,20 @@ void BrowserStatusMonitor::TabInsertedAt(TabStripModel* tab_strip_model,
                                          content::WebContents* contents,
                                          int index,
                                          bool foreground) {
-  // An inserted tab is not active - ActiveTabChanged() will be called to
-  // activate. We initialize therefore with |APP_STATE_INACTIVE|.
-  UpdateAppItemState(contents, ChromeLauncherController::APP_STATE_INACTIVE);
+  UpdateAppItemState(contents, false /*remove*/);
   AddWebContentsObserver(contents);
 }
 
 void BrowserStatusMonitor::TabClosingAt(TabStripModel* tab_strip_mode,
                                         content::WebContents* contents,
                                         int index) {
-  UpdateAppItemState(contents, ChromeLauncherController::APP_STATE_REMOVED);
+  UpdateAppItemState(contents, true /*remove*/);
   RemoveWebContentsObserver(contents);
 }
 
 void BrowserStatusMonitor::WebContentsDestroyed(
     content::WebContents* contents) {
-  UpdateAppItemState(contents, ChromeLauncherController::APP_STATE_REMOVED);
+  UpdateAppItemState(contents, true /*remove*/);
   RemoveWebContentsObserver(contents);
 }
 

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

@@ -16,19 +16,13 @@
 #include "chrome/browser/ui/browser_tab_strip_tracker.h"
 #include "chrome/browser/ui/browser_tab_strip_tracker_delegate.h"
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
-#include "ui/wm/public/activation_change_observer.h"
-
-namespace aura {
-class Window;
-}  // namespace aura
 
 class Browser;
 
 // BrowserStatusMonitor monitors creation/deletion of Browser and its
 // TabStripModel to keep the launcher representation up to date as the
 // active tab changes.
-class BrowserStatusMonitor : public wm::ActivationChangeObserver,
-                             public BrowserTabStripTrackerDelegate,
+class BrowserStatusMonitor : public BrowserTabStripTrackerDelegate,
                              public BrowserListObserver,
                              public TabStripModelObserver {
  public:
@@ -47,18 +41,12 @@ class BrowserStatusMonitor : public wm::ActivationChangeObserver,
   virtual void ActiveUserChanged(const std::string& user_email) {}
 
   // A shortcut to call the ChromeLauncherController's UpdateAppState().
-  void UpdateAppItemState(content::WebContents* contents,
-                          ChromeLauncherController::AppState app_state);
+  void UpdateAppItemState(content::WebContents* contents, bool remove);
 
   // A shortcut to call the BrowserShortcutLauncherItemController's
   // UpdateBrowserItemState().
   void UpdateBrowserItemState();
 
-  // wm::ActivationChangeObserver overrides:
-  void OnWindowActivated(wm::ActivationChangeObserver::ActivationReason reason,
-                         aura::Window* gained_active,
-                         aura::Window* lost_active) override;
-
   // BrowserTabStripTrackerDelegate overrides:
   bool ShouldTrackBrowser(Browser* browser) override;
 

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

@@ -451,7 +451,7 @@ void ChromeLauncherController::UpdateLauncherItemImage(
 }
 
 void ChromeLauncherController::UpdateAppState(content::WebContents* contents,
-                                              AppState app_state) {
+                                              bool remove) {
   ash::ShelfID shelf_id(launcher_controller_helper_->GetAppID(contents));
 
   // Check if the gMail app is loaded and it matches the given content.
@@ -470,7 +470,7 @@ void ChromeLauncherController::UpdateAppState(content::WebContents* contents,
     }
   }
 
-  if (app_state == APP_STATE_REMOVED)
+  if (remove)
     web_contents_to_app_id_.erase(contents);
   else
     web_contents_to_app_id_[contents] = shelf_id.app_id;

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

@@ -67,14 +67,6 @@ class ChromeLauncherController
       private app_list::AppListSyncableService::Observer,
       private sync_preferences::PrefServiceSyncableObserver {
  public:
-  // Used to update the state of non plaform apps, as web contents change.
-  enum AppState {
-    APP_STATE_ACTIVE,
-    APP_STATE_WINDOW_ACTIVE,
-    APP_STATE_INACTIVE,
-    APP_STATE_REMOVED
-  };
-
   // Returns the single ChromeLauncherController instance.
   static ChromeLauncherController* instance() { return instance_; }
 
@@ -151,9 +143,10 @@ class ChromeLauncherController
   // Updates the image for a specific shelf item from the app's icon loader.
   void UpdateLauncherItemImage(const std::string& app_id);
 
-  // Notify the controller that the state of an non platform app's tabs
-  // have changed,
-  void UpdateAppState(content::WebContents* contents, AppState app_state);
+  // Notifies the controller that |contents| changed so it can update the state
+  // of v1 (non-packaged) apps in the shelf. If |remove| is true then it removes
+  // the association of |contents| with an app.
+  void UpdateAppState(content::WebContents* contents, bool remove);
 
   // Returns ShelfID for |contents|. If |contents| is not an app or is not
   // pinned, returns the id of browser shrotcut.
@@ -414,6 +407,7 @@ class ChromeLauncherController
   std::vector<std::unique_ptr<AppIconLoader>> app_icon_loaders_;
 
   // Direct access to app_id for a web contents.
+  // NOTE: This tracks all WebContents, not just those associated with an app.
   WebContentsToAppIDMap web_contents_to_app_id_;
 
   // Used to track app windows.

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

@@ -47,8 +47,7 @@ void MultiProfileBrowserStatusMonitor::ActiveUserChanged(
         !multi_user_util::IsProfileFromActiveUser(browser->profile())) {
       for (int i = 0; i < browser->tab_strip_model()->count(); ++i) {
         launcher_controller_->UpdateAppState(
-            browser->tab_strip_model()->GetWebContentsAt(i),
-            ChromeLauncherController::APP_STATE_REMOVED);
+            browser->tab_strip_model()->GetWebContentsAt(i), true /*remove*/);
       }
     }
   }
@@ -57,13 +56,9 @@ void MultiProfileBrowserStatusMonitor::ActiveUserChanged(
   for (Browser* browser : *browser_list) {
     if (!browser->is_app() && browser->is_type_tabbed() &&
         multi_user_util::IsProfileFromActiveUser(browser->profile())) {
-      int active_index = browser->tab_strip_model()->active_index();
       for (int i = 0; i < browser->tab_strip_model()->count(); ++i) {
         launcher_controller_->UpdateAppState(
-            browser->tab_strip_model()->GetWebContentsAt(i),
-            browser->window()->IsActive() && i == active_index
-                ? ChromeLauncherController::APP_STATE_WINDOW_ACTIVE
-                : ChromeLauncherController::APP_STATE_INACTIVE);
+            browser->tab_strip_model()->GetWebContentsAt(i), false /*remove*/);
       }
     }
   }
@@ -113,8 +108,7 @@ void MultiProfileBrowserStatusMonitor::ConnectV1AppToLauncher(
   // (launcher item) and add the content (launcher item status).
   BrowserStatusMonitor::AddV1AppToShelf(browser);
   launcher_controller_->UpdateAppState(
-      browser->tab_strip_model()->GetActiveWebContents(),
-      ChromeLauncherController::APP_STATE_INACTIVE);
+      browser->tab_strip_model()->GetActiveWebContents(), false /*remove*/);
 }
 
 void MultiProfileBrowserStatusMonitor::DisconnectV1AppFromLauncher(
@@ -122,7 +116,6 @@ void MultiProfileBrowserStatusMonitor::DisconnectV1AppFromLauncher(
   // Removing a V1 app from the launcher requires to remove the content and
   // the launcher item.
   launcher_controller_->UpdateAppState(
-      browser->tab_strip_model()->GetActiveWebContents(),
-      ChromeLauncherController::APP_STATE_REMOVED);
+      browser->tab_strip_model()->GetActiveWebContents(), true /*remove*/);
   BrowserStatusMonitor::RemoveV1AppFromShelf(browser);
 }