[Sync] Componentize synced_tab_delegate

chrome/browser/sync/sessions/sessions_sync_manager.cc

 // Copyright 2014 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/sessions/sessions_sync_manager.h"
 
 #include "base/metrics/field_trial.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/search.h"
-#include "chrome/browser/sync/glue/synced_tab_delegate.h"
-#include "chrome/common/url_constants.h"
-#include "components/sessions/content/content_serialized_navigation_builder.h"
 #include "components/sync_driver/glue/synced_window_delegate.h"
 #include "components/sync_driver/local_device_info_provider.h"
 #include "components/sync_driver/sessions/synced_window_delegates_getter.h"
-#include "content/public/browser/favicon_status.h"
-#include "content/public/browser/navigation_entry.h"
+#include "components/sync_sessions/sync_sessions_client.h"
+#include "components/sync_sessions/synced_tab_delegate.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/common/url_constants.h"
 #include "sync/api/sync_error.h"
 #include "sync/api/sync_error_factory.h"
 #include "sync/api/sync_merge_result.h"
 #include "sync/api/time.h"
 
-using content::NavigationEntry;
-using sessions::ContentSerializedNavigationBuilder;
 using sessions::SerializedNavigationEntry;
 using sync_driver::DeviceInfo;
 using sync_driver::LocalDeviceInfoProvider;
 using syncer::SyncChange;
 using syncer::SyncData;
 
 namespace browser_sync {
 
 namespace {
 
@@ skipping 19 matching lines @@
   return t1->timestamp > t2->timestamp;
 }
 
 // Comparator function for use with std::sort that will sort sessions by
 // descending modified_time (i.e., most recent first).
 bool SessionsRecencyComparator(const sync_driver::SyncedSession* s1,
                            const sync_driver::SyncedSession* s2) {
   return s1->modified_time > s2->modified_time;
 }
 
+// Helper to check if this is a valid tab that should be synced.
+bool ShouldSyncTab(sync_sessions::SyncSessionsClient* sessions_client,
+                   SyncedTabDelegate* tab_delegate) {
+  if (tab_delegate->GetSyncedWindowDelegate() == nullptr)
+    return false;
+
+  // Is there a valid NavigationEntry?
+  if (tab_delegate->ProfileIsSupervised() &&
+      tab_delegate->GetBlockedNavigations()->size() > 0)
+    return true;
+
+  if (tab_delegate->IsInitialBlankNavigation())
+    return false;  // This deliberately ignores a new pending entry.
+
+  int entry_count = tab_delegate->GetEntryCount();
+  for (int i = 0; i < entry_count; ++i) {
+    const GURL& virtual_url = tab_delegate->GetVirtualURLAtIndex(i);
+    if (!virtual_url.is_valid())
+      continue;
+
+    if (sessions_client->ShouldSyncURL(virtual_url))
+      return true;
+  }
+  return false;
+}
+
 }  // namespace
 
 // |local_device| is owned by ProfileSyncService, its lifetime exceeds
 // lifetime of SessionSyncManager.
 SessionsSyncManager::SessionsSyncManager(
+    sync_sessions::SyncSessionsClient* sessions_client,
     Profile* profile,
     LocalDeviceInfoProvider* local_device,
     scoped_ptr<LocalSessionEventRouter> router,
     scoped_ptr<SyncedWindowDelegatesGetter> synced_window_getter)
-    : favicon_cache_(FaviconServiceFactory::GetForProfile(
+    : sessions_client_(sessions_client),
+      session_tracker_(sessions_client),
+      favicon_cache_(FaviconServiceFactory::GetForProfile(
                          profile,
                          ServiceAccessType::EXPLICIT_ACCESS),
                      HistoryServiceFactory::GetForProfile(
                          profile,
                          ServiceAccessType::EXPLICIT_ACCESS),
                      kMaxSyncFavicons),
       local_tab_pool_out_of_sync_(true),
       sync_prefs_(profile->GetPrefs()),
       profile_(profile),
       local_device_(local_device),
       local_session_header_node_id_(TabNodePool::kInvalidTabNodeID),
       stale_session_threshold_days_(kDefaultStaleSessionThresholdDays),
       local_event_router_(router.Pass()),
       synced_window_getter_(synced_window_getter.Pass()),
-      page_revisit_broadcaster_(this, profile) {}
+      page_revisit_broadcaster_(this, sessions_client, profile) {}
 
 LocalSessionEventRouter::~LocalSessionEventRouter() {}
 
 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";
@@ skipping 184 matching lines @@
   syncer::SyncData data = syncer::SyncData::CreateLocalData(
         current_machine_tag(), current_session_name_, entity);
   change_output->push_back(syncer::SyncChange(
         FROM_HERE, syncer::SyncChange::ACTION_UPDATE, data));
 }
 
 void SessionsSyncManager::AssociateTab(SyncedTabDelegate* const tab,
                                        syncer::SyncChangeList* change_output) {
   DCHECK(tab->HasWebContents());
   SessionID::id_type tab_id = tab->GetSessionId();
-  if (tab->profile() != profile_)
-    return;
 
   if (tab->IsBeingDestroyed()) {
     // This tab is closing.
     TabLinksMap::iterator tab_iter = local_tab_map_.find(tab_id);
     if (tab_iter == local_tab_map_.end()) {
       // We aren't tracking this tab (for example, sync setting page).
       return;
     }
     local_tab_pool_.FreeTabNode(tab_iter->second->tab_node_id(),
                                 change_output);
     local_tab_map_.erase(tab_iter);
     return;
   }
 
-  if (!tab->ShouldSync())
+  if (!ShouldSyncTab(sessions_client_, tab))
     return;
 
   TabLinksMap::iterator local_tab_map_iter = local_tab_map_.find(tab_id);
   TabLink* tab_link = NULL;
 
   if (local_tab_map_iter == local_tab_map_.end()) {
     int tab_node_id = tab->GetSyncId();
     // If there is an old sync node for the tab, reuse it.  If this is a new
     // tab, get a sync node for it.
     if (!local_tab_pool_.IsUnassociatedTabNode(tab_node_id)) {
@@ skipping 19 matching lines @@
   sync_pb::EntitySpecifics specifics;
   LocalTabDelegateToSpecifics(*tab, specifics.mutable_session());
   syncer::SyncData data = syncer::SyncData::CreateLocalData(
       TabNodePool::TabIdToTag(current_machine_tag_,
                                tab_link->tab_node_id()),
       current_session_name_,
       specifics);
   change_output->push_back(syncer::SyncChange(
       FROM_HERE, syncer::SyncChange::ACTION_UPDATE, data));
 
-  const GURL new_url = GetCurrentVirtualURL(*tab);
+  int current_index = tab->GetCurrentEntryIndex();
+  const GURL new_url = tab->GetVirtualURLAtIndex(current_index);
   if (new_url != tab_link->url()) {
     tab_link->set_url(new_url);
-    favicon_cache_.OnFaviconVisited(new_url, GetCurrentFaviconURL(*tab));
+    favicon_cache_.OnFaviconVisited(new_url,
+                                    tab->GetFaviconURLAtIndex(current_index));
     page_revisit_broadcaster_.OnPageVisit(
-        new_url, tab->GetCurrentEntryMaybePending()->GetTransitionType());
+        new_url, tab->GetTransitionAtIndex(current_index));
   }
 
   session_tracker_.GetSession(current_machine_tag())->modified_time =
       base::Time::Now();
 }
 
 void SessionsSyncManager::RebuildAssociations() {
   syncer::SyncDataList data(
       sync_processor_->GetAllSyncData(syncer::SESSIONS));
   scoped_ptr<syncer::SyncErrorFactory> error_handler(error_handler_.Pass());
@@ skipping 15 matching lines @@
       const int tab_id = window.tab(j);
       bool success = session_tab_ids.insert(tab_id).second;
       if (!success)
         return false;
     }
   }
 
   return true;
 }
 
-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) {
+bool SessionsSyncManager::OnLocalTabModified(SyncedTabDelegate* modified_tab) {
+  if (!ShouldSyncTab(sessions_client_, modified_tab))

[skym, 2015/10/16 16:55:47]

Won't this miss things like if you have a tab

www.google.com -> chrome://1 -> chrome://2 -> chrome://3 -> chrome://4 ->
chrome://5

As you transition to the next 'uninteresting' entry, say chrome://6, and the
last entry rolls off, foreign clients should remove this tab, but instead the
behavior will be it kind of freezes/gets stuck. Admittedly, this case probably
isn't very important.

I think the existing code had a similar problem, it wasn't flaring in this case.
But at least it was still processing the change.

[Nicolas Zea, 2015/10/20 23:14:42]

Good catch! You're right, previously we'd have removed the tab from the header
node, but this bypasses the AssociateWindows call. Fixed, and added a test case
for it.

[skym, 2015/10/21 00:38:36]

I'm a little sad to see this optimization go away, but it does keep things
easier/simpler. I think when we convert this to USS we're going to have to solve
this problem, since the directory won't be doing bit-wise diffs for us anymore.

+    return false;
+
+  GURL virtual_url =
+      modified_tab->GetVirtualURLAtIndex(modified_tab->GetCurrentEntryIndex());
+  if (!modified_tab->IsBeingDestroyed() && virtual_url.is_valid() &&
+      virtual_url.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));
   }
 
   if (local_tab_pool_out_of_sync_) {
     // If our tab pool is corrupt, pay the price of a full re-association to
     // fix things up.  This takes care of the new tab modification as well.
     RebuildAssociations();
     DCHECK(!local_tab_pool_out_of_sync_);
-    return;
+    return true;
   }
 
   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

[skym, 2015/10/16 16:55:47]

It seems AssociateWindows gets called every time to solve a similar problem as
my comment above. We don't seem to have a way to tell if it was previously
synced. I would think though, this kind of logic is exactly what the session
tracker is supposed to do. I wonder if it already does this.

[Nicolas Zea, 2015/10/20 23:14:42]

I'm not sure I understand your comment. What do you mean by previously synced?

[skym, 2015/10/21 00:38:36]

Oooh, good point, this is more complex than I realized. I guess, did the last
version that was sent to the change processor of the containing window for this
tab, refer to this tab or not... Definitely out of scope of your changes, I was
just thinking.

   // "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, syncer::SyncDataList(), &changes);
   sync_processor_->ProcessSyncChanges(FROM_HERE, changes);
+  return true;
 }
 
 void SessionsSyncManager::OnFaviconsChanged(
     const std::set<GURL>& page_urls,
     const GURL& /* icon_url */) {
   // 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 (const GURL& page_url : page_urls) {
     for (TabLinksMap::iterator tab_iter = local_tab_map_.begin();
          tab_iter != local_tab_map_.end();
@@ skipping 440 matching lines @@
     const std::string& foreign_session_tag) {
   if (foreign_session_tag == current_machine_tag()) {
     DVLOG(1) << "Local session deleted! Doing nothing until a navigation is "
              << "triggered.";
     return false;
   }
   DVLOG(1) << "Disassociating session " << foreign_session_tag;
   return session_tracker_.DeleteSession(foreign_session_tag);
 }
 
-// static
-GURL SessionsSyncManager::GetCurrentVirtualURL(
-    const SyncedTabDelegate& tab_delegate) {
-  const NavigationEntry* current_entry =
-      tab_delegate.GetCurrentEntryMaybePending();
-  return current_entry->GetVirtualURL();
-}
-
-// static
-GURL SessionsSyncManager::GetCurrentFaviconURL(
-    const SyncedTabDelegate& tab_delegate) {
-  const NavigationEntry* current_entry =
-      tab_delegate.GetCurrentEntryMaybePending();
-  return (current_entry->GetFavicon().valid ?
-          current_entry->GetFavicon().url :
-          GURL());
-}
-
 bool SessionsSyncManager::GetForeignSession(
     const std::string& tag,
     std::vector<const sessions::SessionWindow*>* windows) {
   return session_tracker_.LookupSessionWindows(tag, windows);
 }
 
 bool SessionsSyncManager::GetForeignSessionTabs(
     const std::string& tag,
     std::vector<const sessions::SessionTab*>* tabs) {
   std::vector<const sessions::SessionWindow*> windows;
@@ skipping 114 matching lines @@
   session_tab->user_agent_override.clear();
   session_tab->timestamp = mtime;
   const int current_index = tab_delegate.GetCurrentEntryIndex();
   const int min_index = std::max(0, current_index - kMaxSyncNavigationCount);
   const int max_index = std::min(current_index + kMaxSyncNavigationCount,
                                  tab_delegate.GetEntryCount());
   bool is_supervised = tab_delegate.ProfileIsSupervised();
   session_tab->navigations.clear();
 
   for (int i = min_index; i < max_index; ++i) {
-    const NavigationEntry* entry = tab_delegate.GetEntryAtIndexMaybePending(i);
-    DCHECK(entry);
-    if (!entry->GetVirtualURL().is_valid())
+    if (!tab_delegate.GetVirtualURLAtIndex(i).is_valid())
       continue;
+    sessions::SerializedNavigationEntry serialized_entry;
+    tab_delegate.GetSerializedNavigationAtIndex(i, &serialized_entry);

[pavely, 2015/10/16 21:55:20]

Nit: It would make sense to move obtaining serialized_entry 4 lines down to
where it is pushed into navigations.

 
     // Set current_navigation_index to the index in navigations.
     if (i == current_index)
       session_tab->current_navigation_index = session_tab->navigations.size();
 
-    session_tab->navigations.push_back(
-        ContentSerializedNavigationBuilder::FromNavigationEntry(i, *entry));
+    session_tab->navigations.push_back(serialized_entry);
     if (is_supervised) {
       session_tab->navigations.back().set_blocked_state(
           SerializedNavigationEntry::STATE_ALLOWED);
     }
   }
 
   // If the current navigation is invalid, set the index to the end of the
   // navigation array.
   if (session_tab->current_navigation_index < 0) {
     session_tab->current_navigation_index =
         session_tab->navigations.size() - 1;
   }
 
   if (is_supervised) {
-    const std::vector<const NavigationEntry*>& blocked_navigations =
+    int offset = session_tab->navigations.size();
+    const std::vector<const SerializedNavigationEntry*>& blocked_navigations =
         *tab_delegate.GetBlockedNavigations();
-    int offset = session_tab->navigations.size();
     for (size_t i = 0; i < blocked_navigations.size(); ++i) {
-      session_tab->navigations.push_back(
-          ContentSerializedNavigationBuilder::FromNavigationEntry(
-              i + offset, *blocked_navigations[i]));
+      session_tab->navigations.push_back(*blocked_navigations[i]);
+      session_tab->navigations.back().set_index(offset + i);
       session_tab->navigations.back().set_blocked_state(
           SerializedNavigationEntry::STATE_BLOCKED);
       // TODO(bauerb): Add categories
     }
   }
   session_tab->session_storage_persistent_id.clear();
 }
 
 // static
 void SessionsSyncManager::SetVariationIds(sessions::SessionTab* session_tab) {
@@ skipping 39 matching lines @@
                << " with age " << session_age_in_days << ", deleting.";
       DeleteForeignSessionInternal(session_tag, &changes);
     }
   }
 
   if (!changes.empty())
     sync_processor_->ProcessSyncChanges(FROM_HERE, changes);
 }
 
 };  // namespace browser_sync