chrome.tabs.onUpdated doesn't called with 'loading' twice.

chrome/browser/extensions/api/tabs/tabs_event_router.cc

Index: chrome/browser/extensions/api/tabs/tabs_event_router.cc
diff --git a/chrome/browser/extensions/api/tabs/tabs_event_router.cc b/chrome/browser/extensions/api/tabs/tabs_event_router.cc
index 6f5d5207b3a717a48fc2ac4b6d0551bd44d27064..3afb8f523fbea0444141ae04d704d835e8197395 100644
--- a/chrome/browser/extensions/api/tabs/tabs_event_router.cc
+++ b/chrome/browser/extensions/api/tabs/tabs_event_router.cc
@@ -25,7 +25,6 @@
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/web_contents.h"
 
-using base::DictionaryValue;
 using base::ListValue;
 using base::FundamentalValue;
 using content::NavigationController;
@@ -63,36 +62,42 @@ TabsEventRouter::TabEntry::TabEntry() : complete_waiting_on_load_(false),
                                         url_() {
 }
 
-base::DictionaryValue* TabsEventRouter::TabEntry::UpdateLoadState(
+scoped_ptr<base::DictionaryValue> TabsEventRouter::TabEntry::UpdateComplete(
     const WebContents* contents) {
   // The tab may go in & out of loading (for instance if iframes navigate).
   // We only want to respond to the first change from loading to !loading after
   // the NAV_ENTRY_COMMITTED was fired.
   if (!complete_waiting_on_load_ || contents->IsLoading())
-    return NULL;
+    return nullptr;
 
   // Send "complete" state change.
   complete_waiting_on_load_ = false;
-  base::DictionaryValue* changed_properties = new base::DictionaryValue();
-  changed_properties->SetString(tabs_constants::kStatusKey,
-                                tabs_constants::kStatusValueComplete);
-  return changed_properties;
+  scoped_ptr<base::DictionaryValue> changed(new base::DictionaryValue);
+  changed->SetString(tabs_constants::kStatusKey,
+                     tabs_constants::kStatusValueComplete);
+  return changed.Pass();
 }
 
-base::DictionaryValue* TabsEventRouter::TabEntry::DidNavigate(
+scoped_ptr<base::DictionaryValue> TabsEventRouter::TabEntry::Update(
     const WebContents* contents) {
-  // Send "loading" state change.
-  complete_waiting_on_load_ = true;
-  base::DictionaryValue* changed_properties = new base::DictionaryValue();
-  changed_properties->SetString(tabs_constants::kStatusKey,
-                                tabs_constants::kStatusValueLoading);
-
-  if (contents->GetURL() != url_) {
-    url_ = contents->GetURL();
-    changed_properties->SetString(tabs_constants::kUrlKey, url_.spec());
+  scoped_ptr<base::DictionaryValue> changed(new base::DictionaryValue);
+
+  if (!complete_waiting_on_load_ && contents->IsLoading()) {

[limasdf, 2015/07/06 17:14:44]

'loading' is not included if previous status is 'loading'.

The only true change is here. (other parts are just style fix and tests)

+    changed->SetString(tabs_constants::kStatusKey,
+                       tabs_constants::kStatusValueLoading);
+    complete_waiting_on_load_ = true;
+  }
+
+  GURL new_url = contents->GetLastCommittedURL();
+  if (url_ != new_url) {
+    changed->SetString(tabs_constants::kUrlKey, new_url.spec());
+    url_ = new_url;
   }
 
-  return changed_properties;
+  if (changed->empty())
+    return nullptr;
+
+  return changed.Pass();
 }
 
 TabsEventRouter::TabsEventRouter(Profile* profile)
@@ -139,6 +144,8 @@ void TabsEventRouter::RegisterForBrowserNotifications(Browser* browser) {
 }
 
 void TabsEventRouter::RegisterForTabNotifications(WebContents* contents) {
+  // TODO(limasdf): Attach WebContentsObservers to |contents| rather than
+  // registering for notifications.
   registrar_.Add(
       this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
       content::Source<NavigationController>(&contents->GetController()));
@@ -394,15 +401,14 @@ void TabsEventRouter::TabMoved(WebContents* contents,
 
 void TabsEventRouter::TabUpdated(WebContents* contents, bool did_navigate) {
   TabEntry* entry = GetTabEntry(contents);
-  scoped_ptr<base::DictionaryValue> changed_properties;
-
   if (!entry)
     return;
 
+  scoped_ptr<base::DictionaryValue> changed_properties;
   if (did_navigate)
-    changed_properties.reset(entry->DidNavigate(contents));
+    changed_properties = entry->Update(contents);
   else
-    changed_properties.reset(entry->UpdateLoadState(contents));
+    changed_properties = entry->UpdateComplete(contents);
 
   if (changed_properties)
     DispatchTabUpdatedEvent(contents, changed_properties.Pass());
@@ -493,21 +499,27 @@ TabsEventRouter::TabEntry* TabsEventRouter::GetTabEntry(WebContents* contents) {
 void TabsEventRouter::Observe(int type,
                               const content::NotificationSource& source,
                               const content::NotificationDetails& details) {
-  if (type == content::NOTIFICATION_NAV_ENTRY_COMMITTED) {
-    NavigationController* source_controller =
-        content::Source<NavigationController>(source).ptr();
-    TabUpdated(source_controller->GetWebContents(), true);
-  } else if (type == content::NOTIFICATION_WEB_CONTENTS_DESTROYED) {
-    // Tab was destroyed after being detached (without being re-attached).
-    WebContents* contents = content::Source<WebContents>(source).ptr();
-    registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
-        content::Source<NavigationController>(&contents->GetController()));
-    registrar_.Remove(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
-        content::Source<WebContents>(contents));
-    favicon_scoped_observer_.Remove(
-        favicon::ContentFaviconDriver::FromWebContents(contents));
-  } else {
-    NOTREACHED();
+  switch (type) {
+    case content::NOTIFICATION_NAV_ENTRY_COMMITTED: {
+      NavigationController* source_controller =
+          content::Source<NavigationController>(source).ptr();
+      TabUpdated(source_controller->GetWebContents(), true);
+      break;
+    }
+    case content::NOTIFICATION_WEB_CONTENTS_DESTROYED: {
+      // Tab was destroyed after being detached (without being re-attached).
+      WebContents* contents = content::Source<WebContents>(source).ptr();
+      registrar_.Remove(
+          this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+          content::Source<NavigationController>(&contents->GetController()));
+      registrar_.Remove(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
+                        content::Source<WebContents>(contents));
+      favicon_scoped_observer_.Remove(
+          favicon::ContentFaviconDriver::FromWebContents(contents));
+      break;
+    }
+    default:
+      NOTREACHED();
   }
 }