[Sync] Refactor handling of session tabs

chrome/browser/sync/glue/session_model_associator.cc

 // Copyright (c) 2012 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/glue/session_model_associator.h"
 
 #include <algorithm>
 #include <set>
 #include <utility>
 
@@ skipping 354 matching lines @@
     tab_map_iter->second->set_tab(&tab);
   }
   DCHECK(tab_link);
   DCHECK_NE(tab_link->sync_id(), syncer::kInvalidId);
 
   DVLOG(1) << "Reloading tab " << tab_id << " from window "
            << tab.GetWindowId();
   return WriteTabContentsToSyncModel(tab_link, error);
 }
 
+// static
+GURL SessionModelAssociator::GetCurrentVirtualURL(
+    const SyncedTabDelegate& tab_delegate) {
+  GURL new_url;
+  const int current_index = tab_delegate.GetCurrentEntryIndex();
+  const int pending_index = tab_delegate.GetPendingEntryIndex();
+  const NavigationEntry* current_entry =
+      (current_index == pending_index) ?
+      tab_delegate.GetPendingEntry() :
+      tab_delegate.GetEntryAtIndex(current_index);
+  return current_entry->GetVirtualURL();
+}
+
 bool SessionModelAssociator::WriteTabContentsToSyncModel(
     TabLink* tab_link,
     syncer::SyncError* error) {
   DCHECK(CalledOnValidThread());
-  const SyncedTabDelegate& tab = *(tab_link->tab());
-  const SyncedWindowDelegate& window =
-      *SyncedWindowDelegate::FindSyncedWindowDelegateWithId(
-          tab.GetWindowId());
+  const SyncedTabDelegate& tab_delegate = *(tab_link->tab());
   int64 sync_id = tab_link->sync_id();
   GURL old_tab_url = tab_link->url();
 
   // Load the last stored version of this tab so we can compare changes. If this
   // is a new tab, session_tab will be a blank/newly created SessionTab object.
   SessionTab* session_tab =
       synced_session_tracker_.GetTab(GetCurrentMachineTag(),
-                                     tab.GetSessionId());
+                                     tab_delegate.GetSessionId());
 
-  // We build a clean session tab specifics directly from the tab data.
-  sync_pb::SessionTab tab_s;
+  base::Time now = base::Time::Now();
+  UpdateSessionTabFromDelegate(tab_delegate, now, now, session_tab);
 
-  GURL new_url;
-  AssociateTabContents(window, tab, session_tab, &tab_s, &new_url);
+  const GURL new_url = GetCurrentVirtualURL(tab_delegate);
+  DVLOG(1) << "Local tab " << tab_delegate.GetSessionId()
+           << " now has URL " << new_url.spec();
 
   // Trigger the favicon load if needed. We do this before opening the write
   // transaction to avoid jank.
   tab_link->set_url(new_url);
   if (new_url != old_tab_url) {
     LoadFaviconForTab(tab_link);
   }
 
   // Update our last modified time.
   synced_session_tracker_.GetSession(GetCurrentMachineTag())->modified_time =
       base::Time::Now();
 
   syncer::WriteTransaction trans(FROM_HERE, sync_service_->GetUserShare());
   syncer::WriteNode tab_node(&trans);
   if (tab_node.InitByIdLookup(sync_id) != syncer::BaseNode::INIT_OK) {
     if (error) {
       *error = error_handler_->CreateAndUploadError(
           FROM_HERE,
           "Failed to look up local tab node",
           model_type());
     }
     return false;
   }
 
+  sync_pb::SessionTab tab_s = session_tab->ToSyncData();
   sync_pb::SessionSpecifics specifics = tab_node.GetSessionSpecifics();
   if (new_url == old_tab_url) {
     // Load the old specifics and copy over the favicon data if needed.
     // TODO(zea): store local favicons in the |synced_favicons_| map and use
     // that instead of reading from sync. This will be necessary to switch to
     // the new api.
     tab_s.set_favicon(specifics.tab().favicon());
     tab_s.set_favicon_source(specifics.tab().favicon_source());
     tab_s.set_favicon_type(specifics.tab().favicon_type());
   }
   // Retain the base SessionSpecifics data (tag, tab_node_id, etc.), and just
   // write the new SessionTabSpecifics.
   specifics.mutable_tab()->CopyFrom(tab_s);
 
   // Write into the actual sync model.
   tab_node.SetSessionSpecifics(specifics);
 
   return true;
 }
 
-// Builds |sync_tab| by combining data from |prev_tab| and |new_tab|. Updates
-// |prev_tab| to reflect the newest version.
-// Timestamps are chosen from either |prev_tab| or base::Time::Now() based on
-// the following rules:
-// 1. If a navigation exists in both |new_tab| and |prev_tab|, as determined
-//    by the unique id, and the navigation didn't just become the current
-//    navigation, we preserve the old timestamp.
+// Updates |session_tab| by combining existing data and new data from
+// |tab_delegate|.
+//
+// Timestamps are chosen from either |session_tab| or
+// |default_navigation_timestamp| based on the following rules:
+// 1. If a navigation exists in both |tab_delegate| and |session_tab|,
+//    as determined by the unique id, and the navigation didn't just
+//    become the current navigation, we preserve the old timestamp.
 // 2. If the navigation exists in both but just become the current navigation
 //    (e.g. the user went back in history to this navigation), we update the
-//    timestamp to Now().
-// 3. All new navigations not present in |prev_tab| have their timestamps set to
-//    Now().
-void SessionModelAssociator::AssociateTabContents(
-    const SyncedWindowDelegate& window,
-    const SyncedTabDelegate& new_tab,
-    SessionTab* prev_tab,
-    sync_pb::SessionTab* sync_tab,
-    GURL* new_url) {
-  DCHECK(prev_tab);
-  DCHECK(sync_tab);
-  DCHECK(new_url);
-  SessionID::id_type tab_id = new_tab.GetSessionId();
-  sync_tab->set_tab_id(tab_id);
-  sync_tab->set_window_id(new_tab.GetWindowId());
-  const int current_index = new_tab.GetCurrentEntryIndex();
-  sync_tab->set_current_navigation_index(current_index);
+//    timestamp to |default_navigation_timestamp|.
+// 3. All new navigations not present in |session_tab| have their
+//    timestamps set to |default_navigation_timestamp| (if they're the
+//    current one) or nulled out (if not).
+//
+// static
+void SessionModelAssociator::UpdateSessionTabFromDelegate(
+    const SyncedTabDelegate& tab_delegate,
+    base::Time mtime,
+    base::Time default_navigation_timestamp,
+    SessionTab* session_tab) {
+  DCHECK(session_tab);
+  session_tab->window_id.set_id(tab_delegate.GetWindowId());
+  session_tab->tab_id.set_id(tab_delegate.GetSessionId());
+  session_tab->tab_visual_index = 0;
+  session_tab->current_navigation_index = tab_delegate.GetCurrentEntryIndex();
+  session_tab->pinned = tab_delegate.IsPinned();
+  session_tab->extension_app_id = tab_delegate.GetExtensionAppId();
+  session_tab->user_agent_override.clear();
+  session_tab->timestamp = mtime;
+  const int current_index = tab_delegate.GetCurrentEntryIndex();
+  const int pending_index = tab_delegate.GetPendingEntryIndex();
   const int min_index = std::max(0,
                                  current_index - kMaxSyncNavigationCount);
   const int max_index = std::min(current_index + kMaxSyncNavigationCount,
-                                 new_tab.GetEntryCount());
-  const int pending_index = new_tab.GetPendingEntryIndex();
-  sync_tab->set_pinned(window.IsTabPinned(&new_tab));
-  if (new_tab.HasExtensionAppId()) {
-    sync_tab->set_extension_app_id(new_tab.GetExtensionAppId());
-  }
-
-  sync_tab->mutable_navigation()->Clear();
+                                 tab_delegate.GetEntryCount());
+  std::vector<TabNavigation> previous_navigations;
+  previous_navigations.swap(session_tab->navigations);

[Nicolas Zea, 2012/09/26 00:26:47]

is there any reason you modify previous_navigations instead of
session_tab->navigations directly?

[akalin, 2012/09/26 00:32:12]

session_tab->navigations should be cleared and filled in from the new
NavigationEntry's right?  But we need to keep around the old navigations to
compare timestamps, so that's why we put those in previous_navigations.

[Nicolas Zea, 2012/09/26 00:35:16]

Ah, right. Makes sense.

   std::vector<TabNavigation>::const_iterator prev_nav_iter =
-      prev_tab->navigations.begin();
+      previous_navigations.begin();
   for (int i = min_index; i < max_index; ++i) {
     const NavigationEntry* entry = (i == pending_index) ?
-       new_tab.GetPendingEntry() : new_tab.GetEntryAtIndex(i);
+       tab_delegate.GetPendingEntry() : tab_delegate.GetEntryAtIndex(i);
     DCHECK(entry);
     if (i == min_index) {
       // Find the location of the first navigation within the previous list of
       // navigations. We only need to do this once, as all subsequent
       // navigations are either contiguous or completely new.
-      for (;prev_nav_iter != prev_tab->navigations.end();
+      for (;prev_nav_iter != previous_navigations.end();
            ++prev_nav_iter) {
         if (prev_nav_iter->unique_id() == entry->GetUniqueID())
           break;
       }
     }
     if (entry->GetVirtualURL().is_valid()) {
-      if (i == current_index) {
-        *new_url = GURL(entry->GetVirtualURL().spec());
-        DVLOG(1) << "Associating local tab " << new_tab.GetSessionId()
-                 << " with url " << new_url->spec() << " and title "
-                 << entry->GetTitle();
-
-      }
-      sync_pb::TabNavigation* sync_nav = sync_tab->add_navigation();
-      const TabNavigation& navigation =
-          TabNavigation::FromNavigationEntry(i, *entry, base::Time::Now());
-      *sync_nav = navigation.ToSyncData();
-
+      // TODO(akalin): Remove this logic once we have a timestamp in
+      // NavigationEntry (since we can just use those directly).
+      base::Time timestamp;
       // If this navigation is an old one, reuse the old timestamp. Otherwise we
       // leave the timestamp as the current time.
-      if (prev_nav_iter != prev_tab->navigations.end() &&
+      if (prev_nav_iter != previous_navigations.end() &&
           prev_nav_iter->unique_id() == entry->GetUniqueID()) {
-        // Check that we haven't gone back/foward in the nav stack to this page
+        // Check that we haven't gone back/forward in the nav stack to this page
         // (if so, we want to refresh the timestamp).
-        if (!(current_index != prev_tab->current_navigation_index &&
+        if (!(current_index != session_tab->current_navigation_index &&
               current_index == i)) {
-          sync_nav->set_timestamp(
-              syncer::TimeToProtoTime(prev_nav_iter->timestamp()));
-          DVLOG(2) << "Nav to " << sync_nav->virtual_url() << " already known, "
-                   << "reusing old timestamp " << sync_nav->timestamp();
+          timestamp = prev_nav_iter->timestamp();
+          DVLOG(2) << "Nav to " << entry->GetVirtualURL()
+                   << " already known, reusing old timestamp "
+                   << timestamp.ToInternalValue();
         }
         // Even if the user went back in their history, they may have skipped
         // over navigations, so the subsequent navigation entries may need their
         // old timestamps preserved.
         ++prev_nav_iter;
-      } else if (current_index != i &&
-                 prev_tab->navigations.empty()) {
+      } else if (current_index != i && previous_navigations.empty()) {
         // If this is a new tab, and has more than one navigation, we don't
         // actually want to assign the current timestamp to other navigations.
         // Override the timestamp to 0 in that case.
         // Note: this is primarily to handle restoring sessions at restart,
         // opening recently closed tabs, or opening tabs from other devices.
         // Only the current navigation should have a timestamp in those cases.
-        sync_nav->set_timestamp(0);
+        timestamp = base::Time();
+      } else {
+        timestamp = default_navigation_timestamp;
       }
+
+      session_tab->navigations.push_back(
+          TabNavigation::FromNavigationEntry(i, *entry, timestamp));
     }
   }
-
-  // Now update our local version with the newest data.
-  PopulateSessionTabFromSpecifics(*sync_tab,
-                                  base::Time::Now(),
-                                  prev_tab);
+  session_tab->session_storage_persistent_id.clear();
 }
 
 void SessionModelAssociator::LoadFaviconForTab(TabLink* tab_link) {
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
   if (!command_line.HasSwitch(switches::kSyncTabFavicons))
     return;
   FaviconService* favicon_service =
       FaviconServiceFactory::GetForProfile(profile_, Profile::EXPLICIT_ACCESS);
   if (!favicon_service)
     return;
@@ skipping 412 matching lines @@
       if (tab->navigations[selected_index].virtual_url().is_valid())
         previous_url = tab->navigations[selected_index].virtual_url().spec();
       if (synced_favicon_pages_.find(previous_url) ==
           synced_favicon_pages_.end()) {
         // The previous url didn't have a favicon. No need to decrement it.
         previous_url.clear();
       }
     }
 
     // Update SessionTab based on protobuf.
-    PopulateSessionTabFromSpecifics(tab_s, modification_time, tab);
+    tab->SetFromSyncData(tab_s, modification_time);
 
     // Loads the tab favicon, increments the usage counter, and updates
     // synced_favicon_pages_.
     LoadForeignTabFavicon(tab_s);
 
     // Now check to see if the favicon associated with the previous url is no
     // longer in use. This will have no effect if the current url matches the
     // previous url (LoadForeignTabFavicon increments, this decrements, no net
     // change in usage), or if the previous_url was not set (new tab).
     DecrementAndCleanFaviconForURL(previous_url);
@@ skipping 47 matching lines @@
              << "triggered.";
     return false;
   }
   DVLOG(1) << "Disassociating session " << foreign_session_tag;
   return synced_session_tracker_.DeleteSession(foreign_session_tag);
 }
 
 // Static
 void SessionModelAssociator::PopulateSessionHeaderFromSpecifics(
     const sync_pb::SessionHeader& header_specifics,
-    const base::Time& mtime,
+    base::Time mtime,
     SyncedSession* session_header) {
   if (header_specifics.has_client_name()) {
     session_header->session_name = header_specifics.client_name();
   }
   if (header_specifics.has_device_type()) {
     switch (header_specifics.device_type()) {
       case sync_pb::SessionHeader_DeviceType_TYPE_WIN:
         session_header->device_type = SyncedSession::TYPE_WIN;
         break;
       case sync_pb::SessionHeader_DeviceType_TYPE_MAC:
@@ skipping 18 matching lines @@
         break;
     }
   }
   session_header->modified_time = mtime;
 }
 
 // Static
 void SessionModelAssociator::PopulateSessionWindowFromSpecifics(
     const std::string& session_tag,
     const sync_pb::SessionWindow& specifics,
-    const base::Time& mtime,
+    base::Time mtime,
     SessionWindow* session_window,
     SyncedSessionTracker* tracker) {
   if (specifics.has_window_id())
     session_window->window_id.set_id(specifics.window_id());
   if (specifics.has_selected_tab_index())
     session_window->selected_tab_index = specifics.selected_tab_index();
   if (specifics.has_browser_type()) {
     if (specifics.browser_type() ==
         sync_pb::SessionWindow_BrowserType_TYPE_TABBED) {
       session_window->type = 1;
     } else {
       session_window->type = 2;
     }
   }
   session_window->timestamp = mtime;
   session_window->tabs.resize(specifics.tab_size(), NULL);
   for (int i = 0; i < specifics.tab_size(); i++) {
     SessionID::id_type tab_id = specifics.tab(i);
     tracker->PutTabInWindow(session_tag,
                             session_window->window_id.id(),
                             tab_id,
                             i);
   }
 }
 
-// Static
-void SessionModelAssociator::PopulateSessionTabFromSpecifics(
-    const sync_pb::SessionTab& specifics,
-    const base::Time& mtime,
-    SessionTab* tab) {
-  DCHECK_EQ(tab->tab_id.id(), specifics.tab_id());
-  if (specifics.has_tab_id())
-    tab->tab_id.set_id(specifics.tab_id());
-  if (specifics.has_window_id())
-    tab->window_id.set_id(specifics.window_id());
-  if (specifics.has_tab_visual_index())
-    tab->tab_visual_index = specifics.tab_visual_index();
-  if (specifics.has_current_navigation_index())
-    tab->current_navigation_index = specifics.current_navigation_index();
-  if (specifics.has_pinned())
-    tab->pinned = specifics.pinned();
-  if (specifics.has_extension_app_id())
-    tab->extension_app_id = specifics.extension_app_id();
-  tab->timestamp = mtime;
-  // Cleared in case we reuse a pre-existing SyncedSessionTab object.
-  tab->navigations.clear();
-  for (int i = 0; i < specifics.navigation_size(); ++i) {
-    tab->navigations.push_back(
-        TabNavigation::FromSyncData(i, specifics.navigation(i)));
-  }
-}
-
 void SessionModelAssociator::LoadForeignTabFavicon(
     const sync_pb::SessionTab& tab) {
   if (!tab.has_favicon() || tab.favicon().empty())
     return;
   if (!tab.has_favicon_type() ||
       tab.favicon_type() != sync_pb::SessionTab::TYPE_WEB_FAVICON) {
     DVLOG(1) << "Ignoring non-web favicon.";
     return;
   }
   if (tab.navigation_size() == 0)
@@ skipping 278 matching lines @@
 
 bool SessionModelAssociator::CryptoReadyIfNecessary() {
   // We only access the cryptographer while holding a transaction.
   syncer::ReadTransaction trans(FROM_HERE, sync_service_->GetUserShare());
   const syncer::ModelTypeSet encrypted_types = trans.GetEncryptedTypes();
   return !encrypted_types.Has(SESSIONS) ||
          sync_service_->IsCryptographerReady(&trans);
 }
 
 }  // namespace browser_sync