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

chrome/browser/extensions/api/tabs/tabs_event_router.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/extensions/api/tabs/tabs_event_router.h"
 
 #include "base/json/json_writer.h"
 #include "base/values.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/tabs/tabs_constants.h"
 #include "chrome/browser/extensions/api/tabs/tabs_windows_api.h"
 #include "chrome/browser/extensions/api/tabs/windows_event_router.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_iterator.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "components/favicon/content/content_favicon_driver.h"
 #include "content/public/browser/favicon_status.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
 #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;
 using content::WebContents;
 using ui_zoom::ZoomController;
 
 namespace extensions {
 
 namespace {
 
@@ skipping 17 matching lines @@
   event_args->Set(2, ExtensionTabUtil::CreateTabValue(contents, extension));
   return true;
 }
 
 }  // namespace
 
 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);
+  scoped_ptr<base::DictionaryValue> changed(new base::DictionaryValue);
 
-  if (contents->GetURL() != url_) {
-    url_ = contents->GetURL();
-    changed_properties->SetString(tabs_constants::kUrlKey, url_.spec());
+  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;
   }
 
-  return changed_properties;
+  GURL new_url = contents->GetLastCommittedURL();
+  if (url_ != new_url) {
+    changed->SetString(tabs_constants::kUrlKey, new_url.spec());
+    url_ = new_url;
+  }
+
+  if (changed->empty())
+    return nullptr;
+
+  return changed.Pass();
 }
 
 TabsEventRouter::TabsEventRouter(Profile* profile)
     : profile_(profile), favicon_scoped_observer_(this) {
   DCHECK(!profile->IsOffTheRecord());
 
   BrowserList::AddObserver(this);
 
   // Init() can happen after the browser is running, so catch up with any
   // windows that already exist.
@@ skipping 26 matching lines @@
   // Start listening to TabStripModel events for this browser.
   TabStripModel* tab_strip = browser->tab_strip_model();
   tab_strip->AddObserver(this);
 
   for (int i = 0; i < tab_strip->count(); ++i) {
     RegisterForTabNotifications(tab_strip->GetWebContentsAt(i));
   }
 }
 
 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()));
 
   // Observing NOTIFICATION_WEB_CONTENTS_DESTROYED is necessary because it's
   // possible for tabs to be created, detached and then destroyed without
   // ever having been re-attached and closed. This happens in the case of
   // a devtools WebContents that is opened in window, docked, then closed.
   registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
                  content::Source<WebContents>(contents));
@@ skipping 235 matching lines @@
 
   Profile* profile = Profile::FromBrowserContext(contents->GetBrowserContext());
   DispatchEvent(profile,
                 tabs::OnMoved::kEventName,
                 args.Pass(),
                 EventRouter::USER_GESTURE_UNKNOWN);
 }
 
 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());
 }
 
 void TabsEventRouter::FaviconUrlUpdated(WebContents* contents) {
     content::NavigationEntry* entry =
         contents->GetController().GetVisibleEntry();
     if (!entry || !entry->GetFavicon().valid)
       return;
@@ skipping 70 matching lines @@
   int tab_id = ExtensionTabUtil::GetTabId(contents);
   std::map<int, TabEntry>::iterator i = tab_entries_.find(tab_id);
   if (tab_entries_.end() == i)
     return NULL;
   return &i->second;
 }
 
 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();
   }
 }
 
 void TabsEventRouter::TabChangedAt(WebContents* contents,
                                    int index,
                                    TabChangeType change_type) {
   TabUpdated(contents, false);
 }
 
 void TabsEventRouter::TabReplacedAt(TabStripModel* tab_strip_model,
@@ skipping 69 matching lines @@
 void TabsEventRouter::OnFaviconUpdated(favicon::FaviconDriver* favicon_driver,
                                        bool icon_url_changed) {
   if (icon_url_changed) {
     favicon::ContentFaviconDriver* content_favicon_driver =
         static_cast<favicon::ContentFaviconDriver*>(favicon_driver);
     FaviconUrlUpdated(content_favicon_driver->web_contents());
   }
 }
 
 }  // namespace extensions