sync: Route local sessions events to SessionsSyncManager

chrome/browser/sync/sessions2/sessions_sync_manager.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/sync/sessions2/sessions_sync_manager.h"
 
 #include "chrome/browser/chrome_notification_types.h"
 #if !defined(OS_ANDROID)
 #include "chrome/browser/network_time/navigation_time_helper.h"
 #endif
@@ skipping 19 matching lines @@
 
 namespace browser_sync {
 
 // Maximum number of favicons to sync.
 // TODO(zea): pull this from the server.
 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<LocalEventRouter> 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()) {
 }
 
 SessionsSyncManager::~SessionsSyncManager() {
 }
 
 // Returns the GUID-based string that should be used for
 // |SessionsSyncManager::current_machine_tag_|.
 static std::string BuildMachineTag(const std::string& cache_guid) {
   std::string machine_tag = "session_sync";
   machine_tag.append(cache_guid);
@@ skipping 51 matching lines @@
       delegate_->GetLocalSyncCacheGUID()));
   if (current_machine_tag_.compare(sync_machine_tag) != 0)
     DeleteForeignSessionInternal(sync_machine_tag, &new_changes);
 #endif
 
   // Check if anything has changed on the local client side.
   AssociateWindows(RELOAD_TABS, &new_changes);
 
   merge_result.set_error(
       sync_processor_->ProcessSyncChanges(FROM_HERE, new_changes));
+
+  local_event_router_->StartRoutingTo(this);
   return merge_result;
 }
 
 void SessionsSyncManager::AssociateWindows(
     ReloadTabsOption option,
     syncer::SyncChangeList* change_output) {
   const std::string local_tag = current_machine_tag();
   sync_pb::SessionSpecifics specifics;
   specifics.set_session_tag(local_tag);
   sync_pb::SessionHeader* header_s = specifics.mutable_header();
@@ skipping 157 matching lines @@
   const GURL new_url = GetCurrentVirtualURL(*tab);
   if (new_url != tab_link->url()) {
     tab_link->set_url(new_url);
     favicon_cache_.OnFaviconVisited(new_url, GetCurrentFaviconURL(*tab));
   }
 
   session_tracker_.GetSession(current_machine_tag())->modified_time =
       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(

[rlarocque, 2013/11/27 22:47:51]

I've always thought that this would be better implemented as a WebUI Javascript
callback.  That way we could:
- Clean up the sessions code.
- Not require that sessions sync be enabled for this to work.
- Allow more complicated fetch logic (eg. Refresh every time the user activates
an "other tabs" UI element, rather than on every NTP open).

No need to change it now, though.

[tim (not reviewing), 2013/12/02 18:29:49]

That does seem nice. I actually wrestled around with this code a fair bit
because as it turns out, it's no longer reachable on desktop (hence the
decorations around my test in the _unittest.cc file).  It is reachable on
Android though.

To your point about not requiring sessions to be enabled, can you explain? It
occurs to me that we could do this filtering entirely within the
LocalEventRouter now, which exists unless sessions sync is explicitly disabled,
and probably makes a lot more sense; That could also be an action that
explicitly sends a flare to sync to start.

For now I'd like to maintain as much parity as I can though.  But interested in
changing this down the road.

[rlarocque, 2013/12/02 18:53:53]

By "not requiring sessions to be enabled", I mean that the JavaScript callback
would work regardless of whether or not sessions sync has been explicitly
disabled by the user.  I'm not sure that we care about that, since (I think)
this was implemented to help update the "other devices" UI, so we'd probably
want to disable it if sessions sync were disabled anyway.  Still, though, a
solution that works even when sessions sync is disabled is more flexible and
could be nice to have in the future.

+        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 {
   if (tab.profile() != profile_)
     return false;
 
   if (SyncedWindowDelegate::FindSyncedWindowDelegateWithId(
           tab.GetWindowId()) == NULL) {
     return false;
   }
@@ skipping 25 matching lines @@
 }
 
 // static.
 bool SessionsSyncManager::ShouldSyncWindow(
     const SyncedWindowDelegate* window) {
   if (window->IsApp())
     return false;
   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();
 }
 
 syncer::SyncDataList SessionsSyncManager::GetAllSyncData(
     syncer::ModelType type) const {
   NOTIMPLEMENTED();
   return syncer::SyncDataList();
 }
 
@@ skipping 16 matching lines @@
         it->sync_data().GetSpecifics().session();
     switch (it->change_type()) {
       case syncer::SyncChange::ACTION_DELETE:
         // Deletions are all or nothing (since we only ever delete entire
         // sessions). Therefore we don't care if it's a tab node or meta node,
         // and just ensure we've disassociated.
         if (current_machine_tag() == session.session_tag()) {
           // 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()) {
           // Disassociate only when header node is deleted. For tab node
           // deletions, the header node will be updated and foreign tab will
           // get deleted.
           DisassociateForeignSession(session.session_tag());
         }
         continue;
       case syncer::SyncChange::ACTION_ADD:
@@ skipping 452 matching lines @@
           SerializedNavigationEntry::FromNavigationEntry(
               i + offset, *blocked_navigations[i]));
       session_tab->navigations.back().set_blocked_state(
           SerializedNavigationEntry::STATE_BLOCKED);
       // TODO(bauerb): Add categories
     }
   }
   session_tab->session_storage_persistent_id.clear();
 }
 
-
 FaviconCache* SessionsSyncManager::GetFaviconCache() {
   return &favicon_cache_;
 }
 
 };  // namespace browser_sync