Add onChanged callback for chrome.sessions API.

chrome/browser/extensions/api/sessions/sessions_api.cc

Index: chrome/browser/extensions/api/sessions/sessions_api.cc
diff --git a/chrome/browser/extensions/api/sessions/sessions_api.cc b/chrome/browser/extensions/api/sessions/sessions_api.cc
index 552320014446cb6538c1f3af86f8fd5b5ff77daf..f587a14b26d352358fb64dd5e67185734a30d02f 100644
--- a/chrome/browser/extensions/api/sessions/sessions_api.cc
+++ b/chrome/browser/extensions/api/sessions/sessions_api.cc
@@ -35,6 +35,7 @@
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/extension_function_dispatcher.h"
 #include "extensions/browser/extension_function_registry.h"
+#include "extensions/browser/extension_system.h"
 #include "extensions/common/error_utils.h"
 #include "net/base/net_util.h"
 #include "ui/base/layout.h"
@@ -577,4 +578,68 @@ bool SessionsRestoreFunction::RunImpl() {
       : RestoreLocalSession(*session_id, browser);
 }
 
+SessionsEventRouter::SessionsEventRouter(Profile* profile)
+    : profile_(profile), tab_restore_service_(NULL) {

[not at google - send to devlin, 2014/03/21 11:03:06]

just construct with the right value for tab_store_service_:

: profile_(profile),
  tab_restore_service_(TabRestoreServiceFactory::GetForProfile(profile))

+  tab_restore_service_ =
+      TabRestoreServiceFactory::GetForProfile(profile_);
+
+  // TabRestoreServiceFactory::GetForProfile() can return NULL (i.e., when in
+  // incognito mode)
+  if (tab_restore_service_) {
+    // This does nothing if the tabs have already been loaded or they
+    // shouldn't be loaded.

[not at google - send to devlin, 2014/03/21 11:03:06]

this comment is already on the interface

+    tab_restore_service_->LoadTabsFromLastSession();
+    tab_restore_service_->AddObserver(this);
+  }
+}
+
+SessionsEventRouter::~SessionsEventRouter() {
+  if (tab_restore_service_)
+    tab_restore_service_->RemoveObserver(this);
+}
+
+void SessionsEventRouter::TabRestoreServiceChanged(
+    TabRestoreService* service) {
+  scoped_ptr<base::ListValue> args(new base::ListValue());
+  scoped_ptr<Event> event(new Event(
+      api::sessions::OnChanged::kEventName, args.Pass()));
+  ExtensionSystem::Get(profile_)->event_router()->BroadcastEvent(event.Pass());

[not at google - send to devlin, 2014/03/21 11:03:06]

consider inlining |event|, but up to you:

ExtensionSystem::Get(profile_)->event_router()->BroadcastEvent(
    make_scoped_ptr(
        new Event(api::sessions::OnChanged::kEventName, args.Pass()))

+}
+
+void SessionsEventRouter::TabRestoreServiceDestroyed(
+    TabRestoreService* service) {
+  tab_restore_service_ = NULL;
+}
+
+SessionsAPI::SessionsAPI(content::BrowserContext* context)
+    : browser_context_(context) {
+  EventRouter* event_router =
+      ExtensionSystem::Get(browser_context_)->event_router();
+  event_router->RegisterObserver(this,
+      api::sessions::OnChanged::kEventName);

[not at google - send to devlin, 2014/03/21 11:03:06]

since you're inlining this in Shutdown() (I like inlining) do so here as well

+}
+
+SessionsAPI::~SessionsAPI() {
+}
+
+void SessionsAPI::Shutdown() {
+  ExtensionSystem::Get(browser_context_)->event_router()->UnregisterObserver(
+      this);
+}
+
+static base::LazyInstance<BrowserContextKeyedAPIFactory<SessionsAPI> >
+    g_factory = LAZY_INSTANCE_INITIALIZER;
+
+BrowserContextKeyedAPIFactory<SessionsAPI>*
+SessionsAPI::GetFactoryInstance() {
+  return g_factory.Pointer();
+}
+
+void SessionsAPI::OnListenerAdded(const EventListenerInfo& details) {
+  sessions_event_router_.reset(
+      new SessionsEventRouter(Profile::FromBrowserContext(browser_context_)));
+  ExtensionSystem::Get(browser_context_)->event_router()->UnregisterObserver(

[not at google - send to devlin, 2014/03/21 11:03:06]

I don't understand this. Why unregister when a listener is added? because
there's only 1 listener? I'm not familiar with the idiom; yoz@ ?

[Yoyo Zhou, 2014/03/21 15:56:02]

Basically you lazily register with EventRouter when you get your first listener.
After that you don't need to re-register for additional listeners.

[not at google - send to devlin, 2014/03/21 15:56:56]

Ah I see.

+      this);
+}
+
 }  // namespace extensions