sync: Route local sessions events to SessionsSyncManager

Builds on https://codereview.chromium.org/74653002/

What's left after this:
- Full reassociation on ProcessSyncChanges errors.
- Integration Tests
- Stale Session pruning.

Since GetActiveEntry is now deprecated, switched to use GetVisibleEntry instead per bug 273710.

BUG=98892
R=rlarocque@chromium.org, zea@chromium.org

Review URL: https://codereview.chromium.org/79973002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@238317 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 520 insertions, 98 deletions

diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 5173ed7..ed1e417 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -45,9 +45,11 @@
 #include "chrome/browser/sync/glue/session_model_associator.h"
 #include "chrome/browser/sync/glue/sync_backend_host.h"
 #include "chrome/browser/sync/glue/sync_backend_host_impl.h"
+#include "chrome/browser/sync/glue/sync_start_util.h"
 #include "chrome/browser/sync/glue/synced_device_tracker.h"
 #include "chrome/browser/sync/glue/typed_url_data_type_controller.h"
 #include "chrome/browser/sync/profile_sync_components_factory_impl.h"
+#include "chrome/browser/sync/sessions2/notification_service_sessions_router.h"
 #include "chrome/browser/sync/sessions2/sessions_sync_manager.h"
 #include "chrome/browser/sync/sync_global_error.h"
 #include "chrome/browser/sync/user_selectable_sync_type.h"
@@ -91,6 +93,7 @@ using browser_sync::ChangeProcessor;
 using browser_sync::DataTypeController;
 using browser_sync::DataTypeManager;
 using browser_sync::FailedDataTypesHandler;
+using browser_sync::NotificationServiceSessionsRouter;
 using browser_sync::SyncBackendHost;
 using syncer::ModelType;
 using syncer::ModelTypeSet;
@@ -196,7 +199,12 @@ ProfileSyncService::ProfileSyncService(
 
   if (CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableSyncSessionsV2)) {
-    sessions_sync_manager_.reset(new SessionsSyncManager(profile, this));
+    syncer::SyncableService::StartSyncFlare flare(
+        sync_start_util::GetFlareForSyncableService(profile->GetPath()));
+    scoped_ptr<browser_sync::LocalSessionEventRouter> router(
+        new NotificationServiceSessionsRouter(profile, flare));
+    sessions_sync_manager_.reset(
+        new SessionsSyncManager(profile, this, router.Pass()));
   }
 }
 
diff --git a/chrome/browser/sync/sessions2/notification_service_sessions_router.cc b/chrome/browser/sync/sessions2/notification_service_sessions_router.cc
new file mode 100644
index 0000000..0845890
--- /dev/null
+++ b/chrome/browser/sync/sessions2/notification_service_sessions_router.cc
@@ -0,0 +1,157 @@
+// 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/sync/sessions2/notification_service_sessions_router.h"
+
+#include "base/logging.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/extensions/tab_helper.h"
+#include "chrome/browser/favicon/favicon_changed_details.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sync/glue/sync_start_util.h"
+#include "chrome/browser/sync/glue/synced_tab_delegate.h"
+#include "chrome/browser/ui/browser.h"
+#include "content/public/browser/navigation_controller.h"
+#include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_source.h"
+#include "content/public/browser/web_contents.h"
+
+#if defined(ENABLE_MANAGED_USERS)
+#include "chrome/browser/managed_mode/managed_user_service.h"
+#include "chrome/browser/managed_mode/managed_user_service_factory.h"
+#endif
+
+using content::NavigationController;
+using content::WebContents;
+
+namespace browser_sync {
+
+NotificationServiceSessionsRouter::NotificationServiceSessionsRouter(
+    Profile* profile, const syncer::SyncableService::StartSyncFlare& flare)
+        : handler_(NULL),
+          profile_(profile),
+          flare_(flare),
+          weak_ptr_factory_(this) {
+
+  registrar_.Add(this, chrome::NOTIFICATION_TAB_PARENTED,
+      content::NotificationService::AllSources());
+  registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
+      content::NotificationService::AllSources());
+  registrar_.Add(this, content::NOTIFICATION_NAV_LIST_PRUNED,
+      content::NotificationService::AllSources());
+  registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_CHANGED,
+      content::NotificationService::AllSources());
+  registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+      content::NotificationService::AllSources());
+  registrar_.Add(this,
+      chrome::NOTIFICATION_TAB_CONTENTS_APPLICATION_EXTENSION_CHANGED,
+      content::NotificationService::AllSources());
+  registrar_.Add(this,
+      content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+      content::NotificationService::AllBrowserContextsAndSources());
+  registrar_.Add(this, chrome::NOTIFICATION_FAVICON_CHANGED,
+      content::Source<Profile>(profile_));
+#if defined(ENABLE_MANAGED_USERS)
+  if (profile_->IsManaged()) {
+    ManagedUserService* managed_user_service =
+        ManagedUserServiceFactory::GetForProfile(profile_);
+    managed_user_service->AddNavigationBlockedCallback(
+        base::Bind(&NotificationServiceSessionsRouter::OnNavigationBlocked,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+#endif
+}
+
+NotificationServiceSessionsRouter::~NotificationServiceSessionsRouter() {}
+
+void NotificationServiceSessionsRouter::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  switch (type) {
+    case chrome::NOTIFICATION_FAVICON_CHANGED: {
+      content::Details<FaviconChangedDetails> favicon_details(details);
+      if (handler_)
+        handler_->OnFaviconPageUrlsUpdated(favicon_details->urls);
+      return;
+    }
+    // Source<WebContents>.
+    case chrome::NOTIFICATION_TAB_PARENTED:
+    case content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME:
+    case content::NOTIFICATION_WEB_CONTENTS_DESTROYED: {
+      WebContents* web_contents = content::Source<WebContents>(source).ptr();
+      SyncedTabDelegate* tab =
+          SyncedTabDelegate::ImplFromWebContents(web_contents);
+      if (!tab || tab->profile() != profile_)
+        return;
+      if (handler_)
+        handler_->OnLocalTabModified(tab);
+      break;
+    }
+    // Source<NavigationController>.
+    case content::NOTIFICATION_NAV_LIST_PRUNED:
+    case content::NOTIFICATION_NAV_ENTRY_CHANGED:
+    case content::NOTIFICATION_NAV_ENTRY_COMMITTED: {
+      SyncedTabDelegate* tab = SyncedTabDelegate::ImplFromWebContents(
+          content::Source<NavigationController>(source).ptr()->
+              GetWebContents());
+      if (!tab || tab->profile() != profile_)
+        return;
+      if (handler_)
+        handler_->OnLocalTabModified(tab);
+      break;
+    }
+    case chrome::NOTIFICATION_TAB_CONTENTS_APPLICATION_EXTENSION_CHANGED: {
+      extensions::TabHelper* extension_tab_helper =
+          content::Source<extensions::TabHelper>(source).ptr();
+      if (extension_tab_helper->web_contents()->GetBrowserContext() !=
+              profile_) {
+        return;
+      }
+      if (extension_tab_helper->extension_app()) {
+        SyncedTabDelegate* tab = SyncedTabDelegate::ImplFromWebContents(
+            extension_tab_helper->web_contents());
+        if (handler_)
+          handler_->OnLocalTabModified(tab);
+        break;
+      }
+      return;
+    }
+    default:
+      LOG(ERROR) << "Received unexpected notification of type " << type;
+      return;
+  }
+
+  if (!flare_.is_null()) {
+    flare_.Run(syncer::SESSIONS);
+    flare_.Reset();
+  }
+}
+
+void NotificationServiceSessionsRouter::OnNavigationBlocked(
+    content::WebContents* web_contents) {
+  SyncedTabDelegate* tab =
+      SyncedTabDelegate::ImplFromWebContents(web_contents);
+  if (!tab)
+    return;
+
+  DCHECK(tab->profile() == profile_);
+  handler_->OnLocalTabModified(tab);
+}
+
+void NotificationServiceSessionsRouter::StartRoutingTo(
+    LocalSessionEventHandler* handler) {
+  DCHECK(!handler_);
+  handler_ = handler;
+}
+
+void NotificationServiceSessionsRouter::Stop() {
+  weak_ptr_factory_.InvalidateWeakPtrs();
+  registrar_.RemoveAll();
+  handler_ = NULL;
+}
+
+}  // namespace browser_sync
diff --git a/chrome/browser/sync/sessions2/notification_service_sessions_router.h b/chrome/browser/sync/sessions2/notification_service_sessions_router.h
new file mode 100644
index 0000000..7a6c653
--- /dev/null
+++ b/chrome/browser/sync/sessions2/notification_service_sessions_router.h
@@ -0,0 +1,61 @@
+// 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.
+
+#ifndef CHROME_BROWSER_SYNC_SESSIONS2_NOTIFICATION_SERVICE_SESSIONS_ROUTER_H_
+#define CHROME_BROWSER_SYNC_SESSIONS2_NOTIFICATION_SERVICE_SESSIONS_ROUTER_H_
+
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/sync/sessions2/sessions_sync_manager.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+
+class Profile;
+
+namespace content {
+class WebContents;
+}
+
+namespace browser_sync {
+
+// A SessionsSyncManager::LocalEventRouter that drives session sync via
+// the NotificationService.
+class NotificationServiceSessionsRouter
+    : public LocalSessionEventRouter,
+      public content::NotificationObserver {
+ public:
+  NotificationServiceSessionsRouter(
+      Profile* profile,
+      const syncer::SyncableService::StartSyncFlare& flare);
+  virtual ~NotificationServiceSessionsRouter();
+
+  // content::NotificationObserver implementation.
+  // BrowserSessionProvider -> sync API model change application.
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  // SessionsSyncManager::LocalEventRouter implementation.
+  virtual void StartRoutingTo(LocalSessionEventHandler* handler) OVERRIDE;
+  virtual void Stop() OVERRIDE;
+
+ private:
+  // Called when the URL visited in |web_contents| was blocked by the
+  // ManagedUserService. We forward this on to our handler_ via the
+  // normal OnLocalTabModified, but pass through here via a WeakPtr
+  // callback from ManagedUserService and to extract the tab delegate
+  // from WebContents.
+  void OnNavigationBlocked(content::WebContents* web_contents);
+
+  LocalSessionEventHandler* handler_;
+  content::NotificationRegistrar registrar_;
+  Profile* const profile_;
+  syncer::SyncableService::StartSyncFlare flare_;
+  base::WeakPtrFactory<NotificationServiceSessionsRouter> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(NotificationServiceSessionsRouter);
+};
+
+}  // namespace browser_sync
+
+#endif  // CHROME_BROWSER_SYNC_SESSIONS2_NOTIFICATION_SERVICE_SESSIONS_ROUTER_H_
diff --git a/chrome/browser/sync/sessions2/sessions_sync_manager.cc b/chrome/browser/sync/sessions2/sessions_sync_manager.cc
index 66ae4d3..e81d926 100644
--- a/chrome/browser/sync/sessions2/sessions_sync_manager.cc
+++ b/chrome/browser/sync/sessions2/sessions_sync_manager.cc
@@ -37,16 +37,24 @@ static const int kMaxSyncFavicons = 200;
 // The maximum number of navigations in each direction we care to sync.
 static const int kMaxSyncNavigationCount = 6;
 
+// The URL at which the set of synced tabs is displayed. We treat it differently
+// from all other URL's as accessing it triggers a sync refresh of Sessions.
+static const char kNTPOpenTabSyncURL[] = "chrome://newtab/#open_tabs";
+
 SessionsSyncManager::SessionsSyncManager(
     Profile* profile,
-    SyncInternalApiDelegate* delegate)
+    SyncInternalApiDelegate* delegate,
+    scoped_ptr<LocalSessionEventRouter> router)
     : favicon_cache_(profile, kMaxSyncFavicons),
       sync_prefs_(profile->GetPrefs()),
       profile_(profile),
       delegate_(delegate),
-      local_session_header_node_id_(TabNodePool2::kInvalidTabNodeID) {
+      local_session_header_node_id_(TabNodePool2::kInvalidTabNodeID),
+      local_event_router_(router.Pass()) {
 }
 
+LocalSessionEventRouter::~LocalSessionEventRouter() {}
+
 SessionsSyncManager::~SessionsSyncManager() {
 }
 
@@ -116,6 +124,8 @@ syncer::SyncMergeResult SessionsSyncManager::MergeDataAndStartSyncing(
 
   merge_result.set_error(
       sync_processor_->ProcessSyncChanges(FROM_HERE, new_changes));
+
+  local_event_router_->StartRoutingTo(this);
   return merge_result;
 }
 
@@ -293,13 +303,43 @@ void SessionsSyncManager::AssociateTab(SyncedTabDelegate* const tab,
       base::Time::Now();
 }
 
-void SessionsSyncManager::OnLocalTabModified(
-    const SyncedTabDelegate& modified_tab, syncer::SyncError* error) {
-  NOTIMPLEMENTED() << "TODO(tim): SessionModelAssociator::Observe equivalent.";
+void SessionsSyncManager::OnLocalTabModified(SyncedTabDelegate* modified_tab) {
+  const content::NavigationEntry* entry = modified_tab->GetActiveEntry();
+  if (!modified_tab->IsBeingDestroyed() &&
+      entry &&
+      entry->GetVirtualURL().is_valid() &&
+      entry->GetVirtualURL().spec() == kNTPOpenTabSyncURL) {
+    DVLOG(1) << "Triggering sync refresh for sessions datatype.";
+    const syncer::ModelTypeSet types(syncer::SESSIONS);
+    content::NotificationService::current()->Notify(
+        chrome::NOTIFICATION_SYNC_REFRESH_LOCAL,
+        content::Source<Profile>(profile_),
+        content::Details<const syncer::ModelTypeSet>(&types));
+  }
+
+  syncer::SyncChangeList changes;
+  // Associate tabs first so the synced session tracker is aware of them.
+  AssociateTab(modified_tab, &changes);
+  // Note, we always associate windows because it's possible a tab became
+  // "interesting" by going to a valid URL, in which case it needs to be added
+  // to the window's tab information.
+  AssociateWindows(DONT_RELOAD_TABS, &changes);
+  sync_processor_->ProcessSyncChanges(FROM_HERE, changes);
 }
 
-void SessionsSyncManager::OnBrowserOpened() {
-  NOTIMPLEMENTED() << "TODO(tim): SessionModelAssociator::Observe equivalent.";
+void SessionsSyncManager::OnFaviconPageUrlsUpdated(
+    const std::set<GURL>& updated_favicon_page_urls) {
+  // TODO(zea): consider a separate container for tabs with outstanding favicon
+  // loads so we don't have to iterate through all tabs comparing urls.
+  for (std::set<GURL>::const_iterator i = updated_favicon_page_urls.begin();
+       i != updated_favicon_page_urls.end(); ++i) {
+    for (TabLinksMap::iterator tab_iter = local_tab_map_.begin();
+         tab_iter != local_tab_map_.end();
+         ++tab_iter) {
+      if (tab_iter->second->url() == *i)
+        favicon_cache_.OnPageFaviconUpdated(*i);
+    }
+  }
 }
 
 bool SessionsSyncManager::ShouldSyncTab(const SyncedTabDelegate& tab) const {
@@ -345,21 +385,6 @@ bool SessionsSyncManager::ShouldSyncWindow(
   return window->IsTypeTabbed() || window->IsTypePopup();
 }
 
-void SessionsSyncManager::ForwardRelevantFaviconUpdatesToFaviconCache(
-    const std::set<GURL>& updated_favicon_page_urls) {
-  // TODO(zea): consider a separate container for tabs with outstanding favicon
-  // loads so we don't have to iterate through all tabs comparing urls.
-  for (std::set<GURL>::const_iterator i = updated_favicon_page_urls.begin();
-       i != updated_favicon_page_urls.end(); ++i) {
-    for (TabLinksMap::iterator tab_iter = local_tab_map_.begin();
-         tab_iter != local_tab_map_.end();
-         ++tab_iter) {
-      if (tab_iter->second->url() == *i)
-        favicon_cache_.OnPageFaviconUpdated(*i);
-    }
-  }
-}
-
 void SessionsSyncManager::StopSyncing(syncer::ModelType type) {
   NOTIMPLEMENTED();
 }
@@ -396,6 +421,8 @@ syncer::SyncError SessionsSyncManager::ProcessSyncChanges(
           // Another client has attempted to delete our local data (possibly by
           // error or a clock is inaccurate). Just ignore the deletion for now
           // to avoid any possible ping-pong delete/reassociate sequence.
+          // TODO(tim): Bug 98892.  This corrupts TabNodePool. Perform full
+          // re-association.
           LOG(WARNING) << "Local session data deleted. Ignoring until next "
                        << "local navigation event.";
         } else if (session.has_header()) {
@@ -868,7 +895,6 @@ void SessionsSyncManager::SetSessionTabFromDelegate(
   session_tab->session_storage_persistent_id.clear();
 }
 
-
 FaviconCache* SessionsSyncManager::GetFaviconCache() {
   return &favicon_cache_;
 }
diff --git a/chrome/browser/sync/sessions2/sessions_sync_manager.h b/chrome/browser/sync/sessions2/sessions_sync_manager.h
index 88bb884..4eaa0df 100644
--- a/chrome/browser/sync/sessions2/sessions_sync_manager.h
+++ b/chrome/browser/sync/sessions2/sessions_sync_manager.h
@@ -13,6 +13,7 @@
 #include "base/basictypes.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_vector.h"
+#include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
 #include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/sessions/session_types.h"
@@ -45,14 +46,46 @@ class DataTypeErrorHandler;
 class SyncedTabDelegate;
 class SyncedWindowDelegate;
 
+// An interface defining the ways in which local open tab events can interact
+// with session sync.  All local tab events flow to sync via this interface.
+// In that way it is analogous to sync changes flowing to the local model
+// via ProcessSyncChanges, just with a more granular breakdown.
+class LocalSessionEventHandler {
+ public:
+  // A local navigation event took place that affects the synced session
+  // for this instance of Chrome.
+  virtual void OnLocalTabModified(SyncedTabDelegate* modified_tab) = 0;
+
+  // A local navigation occurred that triggered updates to favicon data for
+  // each URL in |updated_page_urls|.  This is routed through Sessions Sync so
+  // that we can filter (exclude) favicon updates for pages that aren't
+  // currently part of the set of local open tabs, and pass relevant updates
+  // on to FaviconCache for out-of-band favicon syncing.
+  virtual void OnFaviconPageUrlsUpdated(
+      const std::set<GURL>& updated_page_urls) = 0;
+};
+
+// The LocalSessionEventRouter is responsible for hooking itself up to various
+// notification sources in the browser process and forwarding relevant
+// events to a handler as defined in the LocalSessionEventHandler contract.
+class LocalSessionEventRouter {
+ public:
+  virtual ~LocalSessionEventRouter();
+  virtual void StartRoutingTo(LocalSessionEventHandler* handler) = 0;
+  virtual void Stop() = 0;
+};
+
 // Contains all logic for associating the Chrome sessions model and
 // the sync sessions model.
 class SessionsSyncManager : public syncer::SyncableService,
-                            public OpenTabsUIDelegate {
+                            public OpenTabsUIDelegate,
+                            public LocalSessionEventHandler {
  public:
   // Isolates SessionsSyncManager from having to depend on sync internals.
   class SyncInternalApiDelegate {
    public:
+    virtual ~SyncInternalApiDelegate() {}
+
     // Returns sync's representation of the local device info.
     // Return value is an empty scoped_ptr if the device info is unavailable.
     virtual scoped_ptr<DeviceInfo> GetLocalDeviceInfo() const = 0;
@@ -62,33 +95,10 @@ class SessionsSyncManager : public syncer::SyncableService,
   };
 
   SessionsSyncManager(Profile* profile,
-                      SyncInternalApiDelegate* delegate);
+                      SyncInternalApiDelegate* delegate,
+                      scoped_ptr<LocalSessionEventRouter> router);
   virtual ~SessionsSyncManager();
 
-  // A local navigation event took place that affects the synced session
-  // for this instance of Chrome.
-  void OnLocalTabModified(const SyncedTabDelegate& modified_tab,
-                          syncer::SyncError* error);
-
-  // When a Browser window is opened, we want to know so we can make sure our
-  // bookkeeping of open windows / sessions on this device is up-to-date.
-  void OnBrowserOpened();
-
-  // A local navigation occurred that triggered updates to favicon data for
-  // each URL in |updated_page_urls|.  This is routed through Sessions Sync so
-  // that we can filter (exclude) favicon updates for pages that aren't
-  // currently part of the set of local open tabs, and pass relevant updates
-  // on to FaviconCache for out-of-band favicon syncing.
-  void ForwardRelevantFaviconUpdatesToFaviconCache(
-      const std::set<GURL>& updated_favicon_page_urls);
-
-  // Returns the tag used to uniquely identify this machine's session in the
-  // sync model.
-  const std::string& current_machine_tag() const {
-    DCHECK(!current_machine_tag_.empty());
-    return current_machine_tag_;
-  }
-
   // syncer::SyncableService implementation.
   virtual syncer::SyncMergeResult MergeDataAndStartSyncing(
       syncer::ModelType type,
@@ -102,14 +112,6 @@ class SessionsSyncManager : public syncer::SyncableService,
       const tracked_objects::Location& from_here,
       const syncer::SyncChangeList& change_list) OVERRIDE;
 
-  // Return the virtual URL of the current tab, even if it's pending.
-  static GURL GetCurrentVirtualURL(const SyncedTabDelegate& tab_delegate);
-
-  // Return the favicon url of the current tab, even if it's pending.
-  static GURL GetCurrentFaviconURL(const SyncedTabDelegate& tab_delegate);
-
-  FaviconCache* GetFaviconCache();
-
   // OpenTabsUIDelegate implementation.
   virtual bool GetSyncedFaviconForPageURL(
       const std::string& pageurl,
@@ -124,6 +126,26 @@ class SessionsSyncManager : public syncer::SyncableService,
                              const SessionTab** tab) OVERRIDE;
   virtual void DeleteForeignSession(const std::string& tag) OVERRIDE;
 
+  // LocalSessionEventHandler implementation.
+  virtual void OnLocalTabModified(SyncedTabDelegate* modified_tab) OVERRIDE;
+  virtual void OnFaviconPageUrlsUpdated(
+      const std::set<GURL>& updated_favicon_page_urls) OVERRIDE;
+
+  // Returns the tag used to uniquely identify this machine's session in the
+  // sync model.
+  const std::string& current_machine_tag() const {
+    DCHECK(!current_machine_tag_.empty());
+    return current_machine_tag_;
+  }
+
+  // Return the virtual URL of the current tab, even if it's pending.
+  static GURL GetCurrentVirtualURL(const SyncedTabDelegate& tab_delegate);
+
+  // Return the favicon url of the current tab, even if it's pending.
+  static GURL GetCurrentFaviconURL(const SyncedTabDelegate& tab_delegate);
+
+  FaviconCache* GetFaviconCache();
+
  private:
   // Keep all the links to local tab data in one place. A tab_node_id and tab
   // must be passed at creation. The tab_node_id is not mutable, although
@@ -308,6 +330,8 @@ class SessionsSyncManager : public syncer::SyncableService,
   // client.
   int local_session_header_node_id_;
 
+  scoped_ptr<LocalSessionEventRouter> local_event_router_;
+
   DISALLOW_COPY_AND_ASSIGN(SessionsSyncManager);
 };
 
diff --git a/chrome/browser/sync/sessions2/sessions_sync_manager_unittest.cc b/chrome/browser/sync/sessions2/sessions_sync_manager_unittest.cc
index 3f21000..c0e284f 100644
--- a/chrome/browser/sync/sessions2/sessions_sync_manager_unittest.cc
+++ b/chrome/browser/sync/sessions2/sessions_sync_manager_unittest.cc
@@ -12,6 +12,7 @@
 #include "chrome/browser/sync/glue/device_info.h"
 #include "chrome/browser/sync/glue/session_sync_test_helper.h"
 #include "chrome/browser/sync/glue/synced_tab_delegate.h"
+#include "chrome/browser/sync/sessions2/notification_service_sessions_router.h"
 #include "chrome/browser/ui/sync/tab_contents_synced_tab_delegate.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
@@ -42,8 +43,15 @@ class TestSyncProcessorStub : public syncer::SyncChangeProcessor {
   virtual syncer::SyncError ProcessSyncChanges(
       const tracked_objects::Location& from_here,
       const syncer::SyncChangeList& change_list) OVERRIDE {
+    if (error_.IsSet()) {
+      syncer::SyncError error = error_;
+      error_ = syncer::SyncError();
+      return error;
+    }
+
     if (output_)
-      output_->assign(change_list.begin(), change_list.end());
+      output_->insert(output_->end(), change_list.begin(), change_list.end());
+
     return syncer::SyncError();
   }
 
@@ -51,7 +59,13 @@ class TestSyncProcessorStub : public syncer::SyncChangeProcessor {
       const OVERRIDE {
     return syncer::SyncDataList();
   }
+
+  void FailProcessSyncChangesWith(const syncer::SyncError& error) {
+    error_ = error;
+  }
+
  private:
+  syncer::SyncError error_;
   syncer::SyncChangeList* output_;
 };
 
@@ -92,20 +106,36 @@ void AddTabsToSyncDataList(const std::vector<sync_pb::SessionSpecifics> tabs,
   }
 }
 
+class DummyRouter : public LocalSessionEventRouter {
+ public:
+  virtual ~DummyRouter() {}
+  virtual void StartRoutingTo(LocalSessionEventHandler* handler) OVERRIDE {}
+  virtual void Stop() OVERRIDE {}
+};
+
+scoped_ptr<LocalSessionEventRouter> NewDummyRouter() {
+  return scoped_ptr<LocalSessionEventRouter>(new DummyRouter());
+}
+
 }  // namespace
 
 class SessionsSyncManagerTest
     : public BrowserWithTestWindowTest,
       public SessionsSyncManager::SyncInternalApiDelegate {
  public:
-  SessionsSyncManagerTest() {}
+  SessionsSyncManagerTest() : test_processor_(NULL) {}
 
   virtual void SetUp() OVERRIDE {
     BrowserWithTestWindowTest::SetUp();
-    manager_.reset(new SessionsSyncManager(profile(), this));
+    browser_sync::NotificationServiceSessionsRouter* router(
+        new browser_sync::NotificationServiceSessionsRouter(
+            profile(), syncer::SyncableService::StartSyncFlare()));
+    manager_.reset(new SessionsSyncManager(profile(), this,
+      scoped_ptr<LocalSessionEventRouter>(router)));
   }
 
   virtual void TearDown() OVERRIDE {
+    test_processor_ = NULL;
     helper()->Reset();
     manager_.reset();
     BrowserWithTestWindowTest::TearDown();
@@ -129,10 +159,10 @@ class SessionsSyncManagerTest
 
   void InitWithSyncDataTakeOutput(const syncer::SyncDataList& initial_data,
                                   syncer::SyncChangeList* output) {
+    test_processor_ = new TestSyncProcessorStub(output);
     syncer::SyncMergeResult result = manager_->MergeDataAndStartSyncing(
         syncer::SESSIONS, initial_data,
-        scoped_ptr<syncer::SyncChangeProcessor>(
-            new TestSyncProcessorStub(output)),
+        scoped_ptr<syncer::SyncChangeProcessor>(test_processor_),
         scoped_ptr<syncer::SyncErrorFactory>(
             new syncer::SyncErrorFactoryMock()));
     EXPECT_FALSE(result.error().IsSet());
@@ -142,6 +172,12 @@ class SessionsSyncManagerTest
     InitWithSyncDataTakeOutput(syncer::SyncDataList(), NULL);
   }
 
+  void TriggerProcessSyncChangesError() {
+    test_processor_->FailProcessSyncChangesWith(syncer::SyncError(
+        FROM_HERE, syncer::SyncError::DATATYPE_ERROR, "Error",
+        syncer::SESSIONS));
+  }
+
   syncer::SyncChangeList* FilterOutLocalHeaderChanges(
       syncer::SyncChangeList* list) {
     syncer::SyncChangeList::iterator it = list->begin();
@@ -163,6 +199,7 @@ class SessionsSyncManagerTest
  private:
   scoped_ptr<SessionsSyncManager> manager_;
   SessionSyncTestHelper helper_;
+  TestSyncProcessorStub* test_processor_;
 };
 
 TEST_F(SessionsSyncManagerTest, PopulateSessionHeader) {
@@ -519,7 +556,7 @@ TEST_F(SessionsSyncManagerTest, MergeLocalSessionNoTabs) {
   SyncData d(SyncData::CreateRemoteData(1, data.GetSpecifics(), base::Time()));
   syncer::SyncDataList in(&d, &d + 1);
   out.clear();
-  SessionsSyncManager manager2(profile(), this);
+  SessionsSyncManager manager2(profile(), this, NewDummyRouter());
   syncer::SyncMergeResult result = manager2.MergeDataAndStartSyncing(
       syncer::SESSIONS, in,
       scoped_ptr<syncer::SyncChangeProcessor>(
@@ -912,7 +949,7 @@ TEST_F(SessionsSyncManagerTest, SaveUnassociatedNodesForReassociation) {
   SyncData d(SyncData::CreateRemoteData(1, entity, base::Time()));
   syncer::SyncDataList in(&d, &d + 1);
   changes.clear();
-  SessionsSyncManager manager2(profile(), this);
+  SessionsSyncManager manager2(profile(), this, NewDummyRouter());
   syncer::SyncMergeResult result = manager2.MergeDataAndStartSyncing(
       syncer::SESSIONS, in,
       scoped_ptr<syncer::SyncChangeProcessor>(
@@ -942,12 +979,13 @@ TEST_F(SessionsSyncManagerTest, MergeDeletesCorruptNode) {
             changes[0].sync_data().GetTag());
 }
 
+// Test that things work if a tab is initially ignored.
 TEST_F(SessionsSyncManagerTest, AssociateWindowsDontReloadTabs) {
   syncer::SyncChangeList out;
   // Go to a URL that is ignored by session syncing.
   AddTab(browser(), GURL("chrome://preferences/"));
   InitWithSyncDataTakeOutput(syncer::SyncDataList(), &out);
-  ASSERT_EQ(2U, out.size());
+  ASSERT_EQ(2U, out.size());  // Header add and update.
   EXPECT_EQ(
       0,
       out[1].sync_data().GetSpecifics().session().header().window_size());
@@ -956,12 +994,7 @@ TEST_F(SessionsSyncManagerTest, AssociateWindowsDontReloadTabs) {
   // Go to a sync-interesting URL.
   NavigateAndCommitActiveTab(GURL("http://foo2"));
 
-  // Simulate a selective association (e.g in response to tab event) as
-  // would occur in practice from ProcessSyncChanges.
-  content::WebContents* c =
-      browser()->tab_strip_model()->GetActiveWebContents();
-  manager()->AssociateTab(SyncedTabDelegate::ImplFromWebContents(c), &out);
-  ASSERT_EQ(2U, out.size());
+  EXPECT_EQ(3U, out.size());  // Tab add, update, and header update.
 
   EXPECT_TRUE(StartsWithASCII(out[0].sync_data().GetTag(),
                               manager()->current_machine_tag(), true));
@@ -976,14 +1009,9 @@ TEST_F(SessionsSyncManagerTest, AssociateWindowsDontReloadTabs) {
   EXPECT_TRUE(out[1].sync_data().GetSpecifics().session().has_tab());
   EXPECT_EQ(SyncChange::ACTION_UPDATE, out[1].change_type());
 
-  out.clear();
-  manager()->AssociateWindows(SessionsSyncManager::DONT_RELOAD_TABS,
-                              &out);
-
-  EXPECT_EQ(1U, out.size());
-  EXPECT_TRUE(out[0].IsValid());
-  EXPECT_EQ(SyncChange::ACTION_UPDATE, out[0].change_type());
-  const SyncData data(out[0].sync_data());
+  EXPECT_TRUE(out[2].IsValid());
+  EXPECT_EQ(SyncChange::ACTION_UPDATE, out[2].change_type());
+  const SyncData data(out[2].sync_data());
   EXPECT_EQ(manager()->current_machine_tag(), data.GetTag());
   const sync_pb::SessionSpecifics& specifics(data.GetSpecifics().session());
   EXPECT_EQ(manager()->current_machine_tag(), specifics.session_tag());
@@ -993,12 +1021,107 @@ TEST_F(SessionsSyncManagerTest, AssociateWindowsDontReloadTabs) {
   EXPECT_EQ(1, header_s.window(0).tab_size());
 }
 
+TEST_F(SessionsSyncManagerTest, OnLocalTabModified) {
+  syncer::SyncChangeList out;
+  // Init with no local data, relies on MergeLocalSessionNoTabs.
+  InitWithSyncDataTakeOutput(syncer::SyncDataList(), &out);
+  ASSERT_FALSE(manager()->current_machine_tag().empty());
+  ASSERT_EQ(2U, out.size());
+
+  // Copy the original header.
+  sync_pb::EntitySpecifics header(out[0].sync_data().GetSpecifics());
+  out.clear();
+
+  const GURL foo1("http://foo/1");
+  const GURL foo2("http://foo/2");
+  const GURL bar1("http://bar/1");
+  const GURL bar2("http://bar/2");
+  AddTab(browser(), foo1);
+  NavigateAndCommitActiveTab(foo2);
+  AddTab(browser(), bar1);
+  NavigateAndCommitActiveTab(bar2);
+
+  // One add, one update for each AddTab.
+  // One update for each NavigateAndCommit.
+  // = 6 total tab updates.
+  // One header update corresponding to each of those.
+  // = 6 total header updates.
+  // 12 total updates.
+  ASSERT_EQ(12U, out.size());
+
+  // Verify the tab node creations and updates to ensure the SyncProcessor
+  // sees the right operations.
+  for (int i = 0; i < 12; i++) {
+    SCOPED_TRACE(i);
+    EXPECT_TRUE(out[i].IsValid());
+    const SyncData data(out[i].sync_data());
+    EXPECT_TRUE(StartsWithASCII(data.GetTag(),
+                                manager()->current_machine_tag(), true));
+    const sync_pb::SessionSpecifics& specifics(data.GetSpecifics().session());
+    EXPECT_EQ(manager()->current_machine_tag(), specifics.session_tag());
+    if (i % 6 == 0) {
+      // First thing on an AddTab is a no-op header update for parented tab.
+      EXPECT_EQ(header.SerializeAsString(),
+                data.GetSpecifics().SerializeAsString());
+    } else if (i % 6 == 1) {
+      // Next, the TabNodePool should create the tab node.
+      EXPECT_EQ(SyncChange::ACTION_ADD, out[i].change_type());
+    } else if (i % 6 == 2) {
+      // Then we see the tab update to the URL.
+      EXPECT_EQ(SyncChange::ACTION_UPDATE, out[i].change_type());
+      ASSERT_TRUE(specifics.has_tab());
+    } else if (i % 6 == 3) {
+      // The header needs to be updated to reflect the new window state.
+      EXPECT_EQ(SyncChange::ACTION_UPDATE, out[i].change_type());
+      EXPECT_TRUE(specifics.has_header());
+    } else if (i % 6 == 4) {
+      // Now we move on to NavigateAndCommit.  Update the tab.
+      EXPECT_EQ(SyncChange::ACTION_UPDATE, out[i].change_type());
+      ASSERT_TRUE(specifics.has_tab());
+    } else if (i % 6 == 5) {
+      // The header needs to be updated to reflect the new window state.
+      EXPECT_EQ(SyncChange::ACTION_UPDATE, out[i].change_type());
+      ASSERT_TRUE(specifics.has_header());
+      header = data.GetSpecifics();
+    }
+  }
+
+  // Verify the actual content to ensure sync sees the right data.
+  // When it's all said and done, the header should reflect two tabs.
+  const sync_pb::SessionHeader& session_header = header.session().header();
+  ASSERT_EQ(1, session_header.window_size());
+  EXPECT_EQ(2, session_header.window(0).tab_size());
+
+  // ASSERT_TRUEs above allow us to dive in freely here.
+  // Verify first tab.
+  const sync_pb::SessionTab& tab1_1 =
+      out[2].sync_data().GetSpecifics().session().tab();
+  ASSERT_EQ(1, tab1_1.navigation_size());
+  EXPECT_EQ(foo1.spec(), tab1_1.navigation(0).virtual_url());
+  const sync_pb::SessionTab& tab1_2 =
+      out[4].sync_data().GetSpecifics().session().tab();
+  ASSERT_EQ(2, tab1_2.navigation_size());
+  EXPECT_EQ(foo1.spec(), tab1_2.navigation(0).virtual_url());
+  EXPECT_EQ(foo2.spec(), tab1_2.navigation(1).virtual_url());
+
+  // Verify second tab.
+  const sync_pb::SessionTab& tab2_1 =
+      out[8].sync_data().GetSpecifics().session().tab();
+  ASSERT_EQ(1, tab2_1.navigation_size());
+  EXPECT_EQ(bar1.spec(), tab2_1.navigation(0).virtual_url());
+  const sync_pb::SessionTab& tab2_2 =
+      out[10].sync_data().GetSpecifics().session().tab();
+  ASSERT_EQ(2, tab2_2.navigation_size());
+  EXPECT_EQ(bar1.spec(), tab2_2.navigation(0).virtual_url());
+  EXPECT_EQ(bar2.spec(), tab2_2.navigation(1).virtual_url());
+}
+
 // Ensure model association associates the pre-existing tabs.
 TEST_F(SessionsSyncManagerTest, MergeLocalSessionExistingTabs) {
   AddTab(browser(), GURL("http://foo1"));
-  NavigateAndCommitActiveTab(GURL("http://foo2"));
+  NavigateAndCommitActiveTab(GURL("http://foo2"));  // Adds back entry.
   AddTab(browser(), GURL("http://bar1"));
-  NavigateAndCommitActiveTab(GURL("http://bar2"));
+  NavigateAndCommitActiveTab(GURL("http://bar2"));  // Adds back entry.
 
   syncer::SyncChangeList out;
   InitWithSyncDataTakeOutput(syncer::SyncDataList(), &out);
@@ -1077,8 +1200,8 @@ TEST_F(SessionsSyncManagerTest, CheckPrerenderedWebContentsSwap) {
 
   // To simulate WebContents swap during prerendering, create new WebContents
   // and swap with old WebContents.
-  content::WebContents* old_web_contents =
-      browser()->tab_strip_model()->GetActiveWebContents();
+  scoped_ptr<content::WebContents> old_web_contents;
+  old_web_contents.reset(browser()->tab_strip_model()->GetActiveWebContents());
 
   // Create new WebContents, with the required tab helpers.
   WebContents* new_web_contents = WebContents::CreateWithSessionStorage(
@@ -1090,42 +1213,39 @@ TEST_F(SessionsSyncManagerTest, CheckPrerenderedWebContentsSwap) {
       .CopyStateFrom(old_web_contents->GetController());
 
   // Swap the WebContents.
-  int index =
-      browser()->tab_strip_model()->GetIndexOfWebContents(old_web_contents);
+  int index = browser()->tab_strip_model()->GetIndexOfWebContents(
+      old_web_contents.get());
   browser()->tab_strip_model()->ReplaceWebContentsAt(index, new_web_contents);
 
-  manager()->AssociateWindows(SessionsSyncManager::RELOAD_TABS, &out);
-  ASSERT_EQ(7U, out.size());
+  ASSERT_EQ(9U, out.size());
   EXPECT_EQ(SyncChange::ACTION_ADD, out[4].change_type());
   EXPECT_EQ(SyncChange::ACTION_UPDATE, out[5].change_type());
 
   // Navigate away.
   NavigateAndCommitActiveTab(GURL("http://bar2"));
-  manager()->AssociateWindows(SessionsSyncManager::RELOAD_TABS, &out);
 
   // Delete old WebContents. This should not crash.
-  delete old_web_contents;
-  manager()->AssociateWindows(SessionsSyncManager::RELOAD_TABS, &out);
+  old_web_contents.reset();
 
   // Try more navigations and verify output size. This can also reveal
   // bugs (leaks) on memcheck bots if the SessionSyncManager
   // didn't properly clean up the tab pool or session tracker.
   NavigateAndCommitActiveTab(GURL("http://bar3"));
-  manager()->AssociateWindows(SessionsSyncManager::RELOAD_TABS, &out);
 
   AddTab(browser(), GURL("http://bar4"));
-  manager()->AssociateWindows(SessionsSyncManager::RELOAD_TABS, &out);
   NavigateAndCommitActiveTab(GURL("http://bar5"));
-  manager()->AssociateWindows(SessionsSyncManager::RELOAD_TABS, &out);
-  ASSERT_EQ(20U, out.size());
+  ASSERT_EQ(19U, out.size());
 }
 
 namespace {
 class SessionNotificationObserver : public content::NotificationObserver {
  public:
-  SessionNotificationObserver() : notified_of_update_(false) {
+  SessionNotificationObserver() : notified_of_update_(false),
+                                  notified_of_refresh_(false) {
     registrar_.Add(this, chrome::NOTIFICATION_FOREIGN_SESSION_UPDATED,
                    content::NotificationService::AllSources());
+    registrar_.Add(this, chrome::NOTIFICATION_SYNC_REFRESH_LOCAL,
+                   content::NotificationService::AllSources());
   }
   virtual void Observe(int type,
                        const content::NotificationSource& source,
@@ -1134,21 +1254,30 @@ class SessionNotificationObserver : public content::NotificationObserver {
       case chrome::NOTIFICATION_FOREIGN_SESSION_UPDATED:
         notified_of_update_ = true;
         break;
+      case chrome::NOTIFICATION_SYNC_REFRESH_LOCAL:
+        notified_of_refresh_ = true;
+        break;
       default:
         NOTREACHED();
         break;
     }
   }
   bool notified_of_update() const { return notified_of_update_; }
-  void Reset() { notified_of_update_ = false; }
+  bool notified_of_refresh() const { return notified_of_refresh_; }
+  void Reset() {
+    notified_of_update_ = false;
+    notified_of_refresh_ = false;
+  }
  private:
   content::NotificationRegistrar registrar_;
   bool notified_of_update_;
+  bool notified_of_refresh_;
 };
 }  // namespace
 
 TEST_F(SessionsSyncManagerTest, NotifiedOfUpdates) {
   SessionNotificationObserver observer;
+  ASSERT_FALSE(observer.notified_of_update());
   InitWithNoSyncData();
 
   SessionID::id_type n[] = {5};
@@ -1175,4 +1304,19 @@ TEST_F(SessionsSyncManagerTest, NotifiedOfUpdates) {
   EXPECT_TRUE(observer.notified_of_update());
 }
 
+#if defined(OS_ANDROID) || defined(OS_IOS)
+// Tests that opening the other devices page triggers a session sync refresh.
+// This page only exists on mobile platforms today; desktop has a
+// search-enhanced NTP without other devices.
+TEST_F(SessionsSyncManagerTest, NotifiedOfRefresh) {
+  SessionNotificationObserver observer;
+  ASSERT_FALSE(observer.notified_of_refresh());
+  InitWithNoSyncData();
+  AddTab(browser(), GURL("http://foo1"));
+  EXPECT_FALSE(observer.notified_of_refresh());
+  NavigateAndCommitActiveTab(GURL("chrome://newtab/#open_tabs"));
+  EXPECT_TRUE(observer.notified_of_refresh());
+}
+#endif  // defined(OS_ANDROID) || defined(OS_IOS)
+
 }  // namespace browser_sync
diff --git a/chrome/browser/ui/sync/tab_contents_synced_tab_delegate.cc b/chrome/browser/ui/sync/tab_contents_synced_tab_delegate.cc
index 38e3fde..c0b3e05 100644
--- a/chrome/browser/ui/sync/tab_contents_synced_tab_delegate.cc
+++ b/chrome/browser/ui/sync/tab_contents_synced_tab_delegate.cc
@@ -71,7 +71,7 @@ NavigationEntry* TabContentsSyncedTabDelegate::GetEntryAtIndex(int i) const {
 }
 
 NavigationEntry* TabContentsSyncedTabDelegate::GetActiveEntry() const {
-  return web_contents_->GetController().GetActiveEntry();
+  return web_contents_->GetController().GetVisibleEntry();
 }
 
 bool TabContentsSyncedTabDelegate::ProfileIsManaged() const {
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 88de04f..62f7ad0 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -2340,6 +2340,8 @@
         'browser/sync/profile_sync_service_observer.h',
         'browser/sync/retry_verifier.cc',
         'browser/sync/retry_verifier.h',
+        'browser/sync/sessions2/notification_service_sessions_router.cc',
+        'browser/sync/sessions2/notification_service_sessions_router.h',
         'browser/sync/sessions2/session_data_type_controller2.cc',
         'browser/sync/sessions2/session_data_type_controller2.h',
         'browser/sync/sessions2/sessions_sync_manager.cc',