Reduce calling count of UpdateAppState()

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

Index: chrome/browser/ui/ash/launcher/browser_status_monitor.cc
diff --git a/chrome/browser/ui/ash/launcher/browser_status_monitor.cc b/chrome/browser/ui/ash/launcher/browser_status_monitor.cc
index c9965ed1b419acd26631e71fa2b4b000c4a1f1a7..202da6dd96dfaa89d3a8e17462638f823f076bb4 100644
--- a/chrome/browser/ui/ash/launcher/browser_status_monitor.cc
+++ b/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"
@@ -14,6 +15,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/web_applications/web_app.h"
+#include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
 #include "ui/aura/client/activation_client.h"
@@ -21,6 +23,31 @@
 #include "ui/aura/window.h"
 #include "ui/gfx/screen.h"
 
+BrowserStatusMonitor::ActiveWebContentsObserver::ActiveWebContentsObserver(
+    content::WebContents* contents,
+    BrowserStatusMonitor* monitor)
+    : content::WebContentsObserver(contents),
+      monitor_(monitor) {
+}
+
+BrowserStatusMonitor::ActiveWebContentsObserver::~ActiveWebContentsObserver() {
+}
+
+void BrowserStatusMonitor::ActiveWebContentsObserver::NavigateToPendingEntry(
+      const GURL& url,
+      content::NavigationController::ReloadType reload_type) {
+  // Don't need to update item state when page is reloaded.

[Mr4D (OOO till 08-26), 2013/09/13 14:35:18]

What about:

No need to update the item state when the page gets reloaded.

[simonhong_, 2013/09/15 17:27:08]

I removed this method and only DidNavigateMainFrame() is used.
When NavigateToPendingEntry() is called we can't update apps state because
webcontents is not updated to new url at this time.

+  if (reload_type == content::NavigationController::NO_RELOAD) {

[Mr4D (OOO till 08-26), 2013/09/13 14:35:18]

No {} here.

[simonhong_, 2013/09/15 17:27:08]

Done.

+    monitor_->UpdateAppAndBrowserState(web_contents());
+  }
+}
+
+void BrowserStatusMonitor::ActiveWebContentsObserver::DidNavigateMainFrame(
+    const content::LoadCommittedDetails& details,
+    const content::FrameNavigateParams& params) {
+  monitor_->UpdateAppAndBrowserState(web_contents());
+}
+
 BrowserStatusMonitor::BrowserStatusMonitor(
     ChromeLauncherController* launcher_controller)
     : launcher_controller_(launcher_controller),
@@ -62,6 +89,9 @@ BrowserStatusMonitor::~BrowserStatusMonitor() {
        i != browser_list->end(); ++i) {
     OnBrowserRemoved(*i);
   }
+
+  STLDeleteContainerPairSecondPointers(webcontents_to_observer_map_.begin(),
+                                       webcontents_to_observer_map_.end());
 }
 
 void BrowserStatusMonitor::OnWindowActivated(aura::Window* gained_active,
@@ -155,12 +185,25 @@ void BrowserStatusMonitor::ActiveTabChanged(content::WebContents* old_contents,
   if (browser &&
       (TabStripModel::kNoTab !=
            browser->tab_strip_model()->GetIndexOfWebContents(old_contents))) {
+    if (webcontents_to_observer_map_.find(old_contents) !=

[Mr4D (OOO till 08-26), 2013/09/13 14:35:18]

You might want to put this deletion code into a function and call it from
ActiveTabChanged as well as from TabClosingAt.

[simonhong_, 2013/09/15 17:27:08]

I changed to observe all web contents.
Inactive webcontent can also change its url.
So only deletion code is used at TabClosingAt().

+             webcontents_to_observer_map_.end()) {
+      delete webcontents_to_observer_map_[old_contents];
+      webcontents_to_observer_map_.erase(old_contents);
+    }
+
     launcher_controller_->UpdateAppState(
         old_contents,
         ChromeLauncherController::APP_STATE_INACTIVE);
   }
 
-  UpdateAppAndBrowserState(new_contents);
+  if (new_contents) {
+    if (webcontents_to_observer_map_.find(new_contents) ==
+            webcontents_to_observer_map_.end()) {
+        webcontents_to_observer_map_[new_contents] =
+            new ActiveWebContentsObserver(new_contents, this);
+    }
+    UpdateAppAndBrowserState(new_contents);
+  }
 }
 
 void BrowserStatusMonitor::TabInsertedAt(content::WebContents* contents,
@@ -169,30 +212,20 @@ void BrowserStatusMonitor::TabInsertedAt(content::WebContents* contents,
   UpdateAppAndBrowserState(contents);
 }
 
-void BrowserStatusMonitor::TabDetachedAt(content::WebContents* contents,
-                                         int index) {
+void BrowserStatusMonitor::TabClosingAt(TabStripModel* tab_strip_mode,
+                                        content::WebContents* contents,
+                                        int index) {

[Mr4D (OOO till 08-26), 2013/09/13 14:35:18]

Is this really the same event? Did you try to drag a tab from one browser to
another and this was called?

[simonhong_, 2013/09/15 17:27:08]

Not same event.

When drag and drop a tab from one browser(A) to another browser(B),
TabClosingAt() is not called. Followings are the call sequence.
1) TabDetachedAt() called at A.
2) ActiveTabChanged() called at A.
3) TabInsertedAt() called at C(temp browser).
4) ActiveTabChanged() called at C.
5) TabDetachedAt() called at C.
6) TabInsertedAt() called at B.
7) ActiveTabChanged() called at B.
During the drag and drop same WebContents is used.
So we don't need to destroy observer here.

TabClosingAt () is only called when tab is closed.
So observer is destroyd when TabClosingAt() is called.

+  if (webcontents_to_observer_map_.find(contents) !=
+          webcontents_to_observer_map_.end()) {
+    delete webcontents_to_observer_map_[contents];
+    webcontents_to_observer_map_.erase(contents);
+  }
+
   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::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(
     content::WebContents* contents) {
   if (contents) {