[Sync] Preserve old SessionSpecifics data when overwriting tab.

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 369 matching lines @@
           tab.GetWindowId());
   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.
   SyncedSessionTab* session_tab =
       synced_session_tracker_.GetTab(GetCurrentMachineTag(),
                                      tab.GetSessionId());
 
-  // We build a clean session specifics directly from the tab data.
-  sync_pb::SessionSpecifics session_s;
-  session_s.set_session_tag(GetCurrentMachineTag());
-  sync_pb::SessionTab* tab_s = session_s.mutable_tab();
+  // We build a clean session tab specifics directly from the tab data.
+  sync_pb::SessionTab tab_s;

[rlarocque, 2012/07/31 21:15:56]

That's an unusual variable name, though I guess it's no more unusual than what
it was before.

 
   GURL new_url;
-  AssociateTabContents(window, tab, session_tab, tab_s, &new_url);
+  AssociateTabContents(window, tab, session_tab, &tab_s, &new_url);
 
   // 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::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.
-    const sync_pb::SessionSpecifics old_specifics =
-        tab_node.GetSessionSpecifics();
-    tab_s->set_favicon(old_specifics.tab().favicon());
-    tab_s->set_favicon_source(old_specifics.tab().favicon_source());
-    tab_s->set_favicon_type(old_specifics.tab().favicon_type());
+    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());
   }
-
-  // Note: we don't need to preserve unknown fields since we're the only ones
-  // who can write to this node (other clients can only delete).
+  // 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(session_s);
+  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
@@ skipping 1148 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 =
       syncer::GetEncryptedTypes(&trans);
   return !encrypted_types.Has(SESSIONS) ||
          sync_service_->IsCryptographerReady(&trans);
 }
 
 }  // namespace browser_sync