Reduce calling count of UpdateAppState()

When web page is loading, a lot of TabChangedAt() is called. That means a
lot of UpdateAppState() is called, too.
For example, loading google drive site causes almost more than 70 calling
of TabChangedAt().
To replace TabChangedAt(), WebContentsObserver is used.

TabDetachedAt() is replaced with TabClosingAt().
When tab is detached, there are two cases.
The one case is tab is attached to other tab strip model. In this case, 
its LauncherItem is updated by ActiveTabChanged().
The other case is tab is closed. In this case TabClosingAt() can handle LauncherItem status.
TabReplacedAt() is only used when tab is discarded when low memory condition is occurred.
In this case, updating the launcher item status is not good because user don't need to know
about discarding a tab. Also it is reloaded again when user selects discarded tab.

R=skuhne@chromium.org
BUG=NONE
TEST=browser_tests, unit_tests

Review URL: https://codereview.chromium.org/23708028

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@226406 0039d316-1c4b-4281-b951-d872f2087c98

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

@@ -6,6 +6,7 @@
 
 #include "ash/shell.h"
 #include "ash/wm/window_util.h"
+#include "base/stl_util.h"
 #include "chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/browser/ui/browser.h"
@@ -15,12 +16,35 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "content/public/browser/web_contents.h"
-#include "content/public/browser/web_contents_view.h"
 #include "ui/aura/client/activation_client.h"
 #include "ui/aura/root_window.h"
 #include "ui/aura/window.h"
 #include "ui/gfx/screen.h"
 
+BrowserStatusMonitor::LocalWebContentsObserver::LocalWebContentsObserver(
+    content::WebContents* contents,
+    BrowserStatusMonitor* monitor)
+    : content::WebContentsObserver(contents),
+      monitor_(monitor) {
+}
+
+BrowserStatusMonitor::LocalWebContentsObserver::~LocalWebContentsObserver() {
+}
+
+void BrowserStatusMonitor::LocalWebContentsObserver::DidNavigateMainFrame(
+    const content::LoadCommittedDetails& details,
+    const content::FrameNavigateParams& params) {
+  Browser* browser = chrome::FindBrowserWithWebContents(web_contents());
+  ChromeLauncherController::AppState state =
+      ChromeLauncherController::APP_STATE_ACTIVE;
+  if (browser->window()->IsActive() &&
+      browser->tab_strip_model()->GetActiveWebContents() == web_contents())
+    state = ChromeLauncherController::APP_STATE_WINDOW_ACTIVE;
+
+  monitor_->UpdateAppItemState(web_contents(), state);
+  monitor_->UpdateBrowserItemState();
+}
+
 BrowserStatusMonitor::BrowserStatusMonitor(
     ChromeLauncherController* launcher_controller)
     : launcher_controller_(launcher_controller),
@@ -62,21 +86,54 @@ BrowserStatusMonitor::~BrowserStatusMonitor() {
        i != browser_list->end(); ++i) {
     OnBrowserRemoved(*i);
   }
+
+  STLDeleteContainerPairSecondPointers(webcontents_to_observer_map_.begin(),
+                                       webcontents_to_observer_map_.end());
+}
+
+void BrowserStatusMonitor::UpdateAppItemState(
+    content::WebContents* contents,
+    ChromeLauncherController::AppState app_state) {
+  DCHECK(contents);
+  launcher_controller_->UpdateAppState(contents, app_state);
+}
+
+void BrowserStatusMonitor::UpdateBrowserItemState() {
+  launcher_controller_->GetBrowserShortcutLauncherItemController()->
+      UpdateBrowserItemState();
 }
 
 void BrowserStatusMonitor::OnWindowActivated(aura::Window* gained_active,
                                              aura::Window* lost_active) {
-  Browser* browser = chrome::FindBrowserWithWindow(lost_active);
-  if (browser) {
-    UpdateAppAndBrowserState(
-        browser->tab_strip_model()->GetActiveWebContents());
+  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);
+    }
   }
 
-  browser = chrome::FindBrowserWithWindow(gained_active);
-  if (browser) {
-    UpdateAppAndBrowserState(
-        browser->tab_strip_model()->GetActiveWebContents());
+  // 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();
 }
 
 void BrowserStatusMonitor::OnWindowDestroyed(aura::Window* window) {
@@ -145,77 +202,85 @@ void BrowserStatusMonitor::ActiveTabChanged(content::WebContents* old_contents,
                                             int index,
                                             int reason) {
   Browser* browser = NULL;
-  if (old_contents)
-    browser = chrome::FindBrowserWithWebContents(old_contents);
+  // Use |new_contents|. |old_contents| could be NULL.
+  DCHECK(new_contents);
+  browser = chrome::FindBrowserWithWebContents(new_contents);
 
   if (browser && browser->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH)
     return;
 
+  ChromeLauncherController::AppState state =
+      ChromeLauncherController::APP_STATE_INACTIVE;
+
   // Update immediately on a tab change.
-  if (browser &&
+  if (old_contents &&
       (TabStripModel::kNoTab !=
-           browser->tab_strip_model()->GetIndexOfWebContents(old_contents))) {
-    launcher_controller_->UpdateAppState(
-        old_contents,
-        ChromeLauncherController::APP_STATE_INACTIVE);
+           browser->tab_strip_model()->GetIndexOfWebContents(old_contents)))
+    UpdateAppItemState(old_contents, state);
+
+  if (new_contents) {
+    state = browser->window()->IsActive() ?
+        ChromeLauncherController::APP_STATE_WINDOW_ACTIVE :
+        ChromeLauncherController::APP_STATE_ACTIVE;
+    UpdateAppItemState(new_contents, state);
+    UpdateBrowserItemState();
   }
+}
 
-  UpdateAppAndBrowserState(new_contents);
+void BrowserStatusMonitor::TabReplacedAt(TabStripModel* tab_strip_model,
+                                         content::WebContents* old_contents,
+                                         content::WebContents* new_contents,
+                                         int index) {
+  DCHECK(old_contents && new_contents);
+  Browser* browser = chrome::FindBrowserWithWebContents(new_contents);
+
+  if (browser && browser->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH)
+    return;
+
+  UpdateAppItemState(old_contents,
+                     ChromeLauncherController::APP_STATE_REMOVED);
+  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);
+  AddWebContentsObserver(new_contents);
 }
 
 void BrowserStatusMonitor::TabInsertedAt(content::WebContents* contents,
                                          int index,
                                          bool foreground) {
-  UpdateAppAndBrowserState(contents);
+  // 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);
+  AddWebContentsObserver(contents);
 }
 
-void BrowserStatusMonitor::TabDetachedAt(content::WebContents* contents,
-                                         int index) {
-  launcher_controller_->UpdateAppState(
-      contents, ChromeLauncherController::APP_STATE_REMOVED);
-  UpdateBrowserItemState();
-}
-
-void BrowserStatusMonitor::TabChangedAt(
-    content::WebContents* contents,
-    int index,
-    TabStripModelObserver::TabChangeType change_type) {
-  UpdateAppAndBrowserState(contents);
+void BrowserStatusMonitor::TabClosingAt(TabStripModel* tab_strip_mode,
+                                        content::WebContents* contents,
+                                        int index) {
+  UpdateAppItemState(contents,
+                     ChromeLauncherController::APP_STATE_REMOVED);
+  RemoveWebContentsObserver(contents);
 }
 
-void BrowserStatusMonitor::TabReplacedAt(TabStripModel* tab_strip_model,
-                                         content::WebContents* old_contents,
-                                         content::WebContents* new_contents,
-                                         int index) {
-  launcher_controller_->UpdateAppState(
-      old_contents,
-      ChromeLauncherController::APP_STATE_REMOVED);
-  UpdateAppAndBrowserState(new_contents);
-}
-
-void BrowserStatusMonitor::UpdateAppAndBrowserState(
+void BrowserStatusMonitor::AddWebContentsObserver(
     content::WebContents* contents) {
-  if (contents) {
-    ChromeLauncherController::AppState app_state =
-        ChromeLauncherController::APP_STATE_INACTIVE;
-
-    Browser* browser = chrome::FindBrowserWithWebContents(contents);
-    DCHECK(browser);
-    if (browser->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH)
-      return;
-    if (browser->tab_strip_model()->GetActiveWebContents() == contents) {
-      if (browser->window()->IsActive())
-        app_state = ChromeLauncherController::APP_STATE_WINDOW_ACTIVE;
-      else
-        app_state = ChromeLauncherController::APP_STATE_ACTIVE;
-    }
-
-    launcher_controller_->UpdateAppState(contents, app_state);
+  if (webcontents_to_observer_map_.find(contents) ==
+          webcontents_to_observer_map_.end()) {
+    webcontents_to_observer_map_[contents] =
+        new LocalWebContentsObserver(contents, this);
   }
-  UpdateBrowserItemState();
 }
 
-void BrowserStatusMonitor::UpdateBrowserItemState() {
-  launcher_controller_->GetBrowserShortcutLauncherItemController()->
-      UpdateBrowserItemState();
+void BrowserStatusMonitor::RemoveWebContentsObserver(
+    content::WebContents* contents) {
+  DCHECK(webcontents_to_observer_map_.find(contents) !=
+      webcontents_to_observer_map_.end());
+  delete webcontents_to_observer_map_[contents];
+  webcontents_to_observer_map_.erase(contents);
 }

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

@@ -12,8 +12,10 @@
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/scoped_observer.h"
+#include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/browser/ui/browser_list_observer.h"
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "ui/aura/client/activation_change_observer.h"
 #include "ui/aura/window_observer.h"
 #include "ui/gfx/display_observer.h"
@@ -27,7 +29,6 @@ class ActivationClient;
 }  // namespace aura
 
 class Browser;
-class ChromeLauncherController;
 
 // BrowserStatusMonitor monitors creation/deletion of Browser and its
 // TabStripModel to keep the launcher representation up to date as the
@@ -41,6 +42,14 @@ class BrowserStatusMonitor : public aura::client::ActivationChangeObserver,
   explicit BrowserStatusMonitor(ChromeLauncherController* launcher_controller);
   virtual ~BrowserStatusMonitor();
 
+  // A shortcut to call the ChromeLauncherController's UpdateAppState().
+  void UpdateAppItemState(content::WebContents* contents,
+                          ChromeLauncherController::AppState app_state);
+
+  // A shortcut to call the BrowserShortcutLauncherItemController's
+  // UpdateBrowserItemState().
+  void UpdateBrowserItemState();
+
   // aura::client::ActivationChangeObserver overrides:
   virtual void OnWindowActivated(aura::Window* gained_active,
                                  aura::Window* lost_active) OVERRIDE;
@@ -62,29 +71,46 @@ class BrowserStatusMonitor : public aura::client::ActivationChangeObserver,
                                 content::WebContents* new_contents,
                                 int index,
                                 int reason) OVERRIDE;
-  virtual void TabInsertedAt(content::WebContents* contents,
-                             int index,
-                             bool foreground) OVERRIDE;
-  virtual void TabDetachedAt(content::WebContents* contents,
-                             int index) OVERRIDE;
-  virtual void TabChangedAt(
-      content::WebContents* contents,
-      int index,
-      TabStripModelObserver::TabChangeType change_type) OVERRIDE;
   virtual void TabReplacedAt(TabStripModel* tab_strip_model,
                              content::WebContents* old_contents,
                              content::WebContents* new_contents,
                              int index) OVERRIDE;
+  virtual void TabInsertedAt(content::WebContents* contents,
+                             int index,
+                             bool foreground) OVERRIDE;
+  virtual void TabClosingAt(TabStripModel* tab_strip_mode,
+                            content::WebContents* contents,
+                            int index) OVERRIDE;
 
  private:
+  // This class monitors the WebContent of the all tab and notifies a navigation
+  // to the BrowserStatusMonitor.
+  class LocalWebContentsObserver : public content::WebContentsObserver {
+   public:
+    LocalWebContentsObserver(content::WebContents* contents,
+                             BrowserStatusMonitor* monitor);
+    virtual ~LocalWebContentsObserver();
+
+    // content::WebContentsObserver overrides:
+    virtual void DidNavigateMainFrame(
+        const content::LoadCommittedDetails& details,
+        const content::FrameNavigateParams& params) OVERRIDE;
+
+   private:
+    BrowserStatusMonitor* monitor_;
+
+    DISALLOW_COPY_AND_ASSIGN(LocalWebContentsObserver);
+  };
+
   typedef std::map<Browser*, std::string> BrowserToAppIDMap;
+  typedef std::map<content::WebContents*, LocalWebContentsObserver*>
+      WebContentsToObserverMap;
 
-  // Update app state for |contents| and browser state.
-  void UpdateAppAndBrowserState(content::WebContents* contents);
+  // Create LocalWebContentsObserver for |contents|.
+  void AddWebContentsObserver(content::WebContents* contents);
 
-  // A shortcut to call the BrowserShortcutLauncherItemController's
-  // UpdateBrowserItemState().
-  void UpdateBrowserItemState();
+  // Remove LocalWebContentsObserver for |contents|.
+  void RemoveWebContentsObserver(content::WebContents* contents);
 
   ChromeLauncherController* launcher_controller_;
 
@@ -97,6 +123,8 @@ class BrowserStatusMonitor : public aura::client::ActivationChangeObserver,
 
   BrowserToAppIDMap browser_to_app_id_map_;
 
+  WebContentsToObserverMap webcontents_to_observer_map_;
+
   DISALLOW_COPY_AND_ASSIGN(BrowserStatusMonitor);
 };
 

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

@@ -390,9 +390,10 @@ void ChromeLauncherController::SetItemStatus(
     ash::LauncherID id,
     ash::LauncherItemStatus status) {
   int index = model_->ItemIndexByID(id);
+  ash::LauncherItemStatus old_status = model_->items()[index].status;
   // Since ordinary browser windows are not registered, we might get a negative
   // index here.
-  if (index >= 0) {
+  if (index >= 0 && old_status != status) {
     ash::LauncherItem item = model_->items()[index];
     item.status = status;
     model_->Set(index, item);
@@ -830,6 +831,11 @@ void ChromeLauncherController::RemoveTabFromRunningApp(
     WebContents* tab,
     const std::string& app_id) {
   web_contents_to_app_id_.erase(tab);
+  // BrowserShortcutLauncherItemController::UpdateBrowserItemState() will update
+  // the state when no application is associated with the tab.
+  if (app_id.empty())
+    return;
+
   AppIDToWebContentsListMap::iterator i_app_id =
       app_id_to_web_contents_list_.find(app_id);
   if (i_app_id != app_id_to_web_contents_list_.end()) {
@@ -867,7 +873,7 @@ void ChromeLauncherController::UpdateAppState(content::WebContents* contents,
   if (app_state == APP_STATE_REMOVED) {
     // The tab has gone away.
     RemoveTabFromRunningApp(contents, app_id);
-  } else {
+  } else if (!app_id.empty()) {
     WebContentsList& tab_list(app_id_to_web_contents_list_[app_id]);
 
     if (app_state == APP_STATE_INACTIVE) {
@@ -875,6 +881,7 @@ void ChromeLauncherController::UpdateAppState(content::WebContents* contents,
           std::find(tab_list.begin(), tab_list.end(), contents);
       if (i_tab == tab_list.end())
         tab_list.push_back(contents);
+      // TODO(simon.hong81): Does this below case exist?
       if (i_tab != tab_list.begin()) {
         // Going inactive, but wasn't the front tab, indicating that a new
         // tab has already become active.

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

@@ -1026,6 +1026,49 @@ IN_PROC_BROWSER_TEST_F(LauncherAppBrowserTest, Navigation) {
   EXPECT_EQ(ash::STATUS_ACTIVE, (*model_->ItemByID(shortcut_id)).status);
 }
 
+// Confirm that a tab can be moved between browsers while maintaining the
+// correct running state.
+IN_PROC_BROWSER_TEST_F(LauncherAppBrowserTest, TabDragAndDrop) {
+  TabStripModel* tab_strip_model1 = browser()->tab_strip_model();
+  EXPECT_EQ(1, tab_strip_model1->count());
+  int browser_index = ash::GetBrowserItemIndex(*model_);
+  EXPECT_TRUE(browser_index >= 0);
+  EXPECT_EQ(1u, chrome::GetTotalBrowserCount());
+
+  // Create a shortcut for app1.
+  ash::LauncherID shortcut_id = CreateShortcut("app1");
+  EXPECT_EQ(ash::STATUS_ACTIVE, model_->items()[browser_index].status);
+  EXPECT_EQ(ash::STATUS_CLOSED, (*model_->ItemByID(shortcut_id)).status);
+
+  // Activate app1 and check its item status.
+  ActivateLauncherItem(model_->ItemIndexByID(shortcut_id));
+  EXPECT_EQ(2, tab_strip_model1->count());
+  EXPECT_EQ(ash::STATUS_RUNNING, model_->items()[browser_index].status);
+  EXPECT_EQ(ash::STATUS_ACTIVE, (*model_->ItemByID(shortcut_id)).status);
+
+  // Create a new browser with blank tab.
+  Browser* browser2 = CreateBrowser(profile());
+  EXPECT_EQ(2u, chrome::GetTotalBrowserCount());
+  TabStripModel* tab_strip_model2 = browser2->tab_strip_model();
+  EXPECT_EQ(1, tab_strip_model2->count());
+  EXPECT_EQ(ash::STATUS_ACTIVE, model_->items()[browser_index].status);
+  EXPECT_EQ(ash::STATUS_RUNNING, (*model_->ItemByID(shortcut_id)).status);
+
+  // Detach a tab at index 1 (app1) from |tab_strip_model1| and insert it as an
+  // active tab at index 1 to |tab_strip_model2|.
+  content::WebContents* detached_tab = tab_strip_model1->DetachWebContentsAt(1);
+  tab_strip_model2->InsertWebContentsAt(1,
+                                        detached_tab,
+                                        TabStripModel::ADD_ACTIVE);
+  EXPECT_EQ(1, tab_strip_model1->count());
+  EXPECT_EQ(2, tab_strip_model2->count());
+  EXPECT_EQ(ash::STATUS_RUNNING, model_->items()[browser_index].status);
+  EXPECT_EQ(ash::STATUS_ACTIVE, (*model_->ItemByID(shortcut_id)).status);
+
+  tab_strip_model1->CloseAllTabs();
+  tab_strip_model2->CloseAllTabs();
+}
+
 IN_PROC_BROWSER_TEST_F(LauncherAppBrowserTest, MultipleOwnedTabs) {
   TabStripModel* tab_strip = browser()->tab_strip_model();
   int tab_count = tab_strip->count();