Reduce calling count of UpdateAppState()

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

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/ash/launcher/browser_status_monitor.h"
 
 #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"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
 #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"
 #include "ui/aura/root_window.h"
 #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),
       observed_activation_clients_(this),
       observed_root_windows_(this) {
   DCHECK(launcher_controller_);
   BrowserList::AddObserver(this);
 
   // This check needs for win7_aura. Without this, all tests in
   // ChromeLauncherController will fail in win7_aura.
@@ skipping 21 matching lines @@
     ash::Shell::GetInstance()->GetScreen()->RemoveObserver(this);
 
   BrowserList::RemoveObserver(this);
 
   BrowserList* browser_list =
       BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH);
   for (BrowserList::const_iterator i = browser_list->begin();
        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,
                                              aura::Window* lost_active) {
   Browser* browser = chrome::FindBrowserWithWindow(lost_active);
   if (browser) {
     UpdateAppAndBrowserState(
         browser->tab_strip_model()->GetActiveWebContents());
   }
 
@@ skipping 73 matching lines @@
   if (old_contents)
     browser = chrome::FindBrowserWithWebContents(old_contents);
 
   if (browser && browser->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH)
     return;
 
   // Update immediately on a tab change.
   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,
                                          int index,
                                          bool foreground) {
   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) {
     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);
   }
   UpdateBrowserItemState();
 }
 
 void BrowserStatusMonitor::UpdateBrowserItemState() {
   launcher_controller_->GetBrowserShortcutLauncherItemController()->
       UpdateBrowserItemState();
 }