[Sync] Don't attempt to sync windowless tabs.

chrome/browser/sync/glue/session_model_associator.cc

 // Copyright (c) 2011 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 198 matching lines @@
   for (std::vector<SyncedTabDelegate*>::const_iterator i = tabs.begin();
        i != tabs.end();
        ++i) {
     ReassociateTab(**i);
   }
   if (waiting_for_change_) QuitLoopForTest();
 }
 
 void SessionModelAssociator::ReassociateTab(const SyncedTabDelegate& tab) {
   DCHECK(CalledOnValidThread());
-  if (!IsValidTab(tab))
-    return;
-
   int64 sync_id;
   SessionID::id_type id = tab.GetSessionId();
   if (tab.IsBeingDestroyed()) {
     // This tab is closing.
     TabLinksMap::iterator tab_iter = tab_map_.find(id);
     if (tab_iter == tab_map_.end()) {
       // We aren't tracking this tab (for example, sync setting page).
       return;
     }
     tab_pool_.FreeTabNode(tab_iter->second.sync_id());
     tab_map_.erase(tab_iter);
     return;
   }
 
+  if (!IsValidTab(tab))
+    return;
+
   TabLinksMap::const_iterator tablink = tab_map_.find(id);
   if (tablink == tab_map_.end()) {
     // This is a new tab, get a sync node for it.
     sync_id = tab_pool_.GetFreeTabNode();
+    if (sync_id == -1) {

[tim (not reviewing), 2011/08/25 20:25:01]

isn't there a constant for this -1 id?
also you seem to use no braces on single line if below

[Nicolas Zea, 2011/08/25 20:52:01]

Done.

+      return;

[tim (not reviewing), 2011/08/25 20:25:01]

log something, perhaps?

[Nicolas Zea, 2011/08/25 20:52:01]

Logging is done in the GetFreeTabNode call

+    }
   } else {
     // This tab is already associated with a sync node, reuse it.
     sync_id = tablink->second.sync_id();
   }
   Associate(&tab, sync_id);
 }
 
 void SessionModelAssociator::Associate(const SyncedTabDelegate* tab,
                                        int64 sync_id) {
   DCHECK(CalledOnValidThread());
   SessionID::id_type session_id = tab->GetSessionId();
   const SyncedWindowDelegate* window =
       SyncedWindowDelegate::FindSyncedWindowDelegateWithId(
           tab->GetWindowId());
-  if (!window) {  // Can happen for weird things like developer console.
-    tab_pool_.FreeTabNode(sync_id);
-    return;
-  }
+  DCHECK(window);
 
   TabLinks t(sync_id, tab);
   tab_map_[session_id] = t;
 
   sync_api::WriteTransaction trans(FROM_HERE, sync_service_->GetUserShare());
   WriteTabContentsToSyncModel(*window, *tab, sync_id, &trans);
 }
 
 bool SessionModelAssociator::WriteTabContentsToSyncModel(
     const SyncedWindowDelegate& window,
@@ skipping 156 matching lines @@
     sync_api::ReadNode root(&trans);
     if (!root.InitByTagLookup(kSessionsTag)) {
       error->Reset(FROM_HERE, kNoSessionsFolderError, model_type());
       return false;
     }
 
     // Make sure we have a machine tag.
     if (current_machine_tag_.empty())
       InitializeCurrentMachineTag(&trans);
     synced_session_tracker_.SetLocalSessionTag(current_machine_tag_);
-    UpdateAssociationsFromSyncModel(root, &trans);
+    if (!UpdateAssociationsFromSyncModel(root, &trans)) {
+      error->Reset(FROM_HERE,
+                   "Failed to update associations from sync",
+                   model_type());
+      return false;
+    }
 
     if (local_session_syncid_ == sync_api::kInvalidId) {
       // The sync db didn't have a header node for us, we need to create one.
       sync_api::WriteNode write_node(&trans);
       if (!write_node.InitUniqueByCreation(syncable::SESSIONS, root,
           current_machine_tag_)) {
         error->Reset(FROM_HERE,
                      "Failed to create sessions header sync node.",
                      model_type());
         return false;
@@ skipping 61 matching lines @@
     const sync_pb::SessionSpecifics& specifics =
         sync_node.GetSessionSpecifics();
     const int64 modification_time = sync_node.GetModificationTime();
     if (specifics.session_tag() != GetCurrentMachineTag()) {
       if (!AssociateForeignSpecifics(specifics, modification_time)) {
         return false;
       }
     } else if (id != local_session_syncid_) {
       // This is previously stored local session information.
       if (specifics.has_header()) {
-        DCHECK_EQ(sync_api::kInvalidId, local_session_syncid_);
+        if (sync_api::kInvalidId != local_session_syncid_)
+          return false;
 
         // This is our previous header node, reuse it.
         local_session_syncid_ = id;
       } else {
-        DCHECK(specifics.has_tab());
+        if (!specifics.has_tab())
+          return false;
 
         // This is a tab node. We want to track these to reuse them in our free
         // tab node pool. They will be overwritten eventually, so need to do
         // anything else.
         tab_pool_.AddTabNode(id);
       }
     }
 
     id = sync_node.GetSuccessorId();
   }
 
   // After updating from sync model all tabid's should be free.
-  DCHECK(tab_pool_.full());
+  if (!tab_pool_.full())
+    return false;
 
   return true;
 }
 
 bool SessionModelAssociator::AssociateForeignSpecifics(
     const sync_pb::SessionSpecifics& specifics,
     const int64 modification_time) {
   DCHECK(CalledOnValidThread());
   std::string foreign_session_tag = specifics.session_tag();
-  DCHECK(foreign_session_tag != GetCurrentMachineTag() || setup_for_test_);
+  if (foreign_session_tag == GetCurrentMachineTag() && !setup_for_test_)
+    return false;
 
   if (specifics.has_header()) {
     // Read in the header data for this foreign session.
     // Header data contains window information and ordered tab id's for each
     // window.
 
     // Load (or create) the SyncedSession object for this client.
     SyncedSession* foreign_session =
         synced_session_tracker_.GetSession(foreign_session_tag);
 
@@ skipping 206 matching lines @@
 }
 
 int64 SessionModelAssociator::TabNodePool::GetFreeTabNode() {
   DCHECK_GT(machine_tag_.length(), 0U);
   if (tab_pool_fp_ == -1) {
     // Tab pool has no free nodes, allocate new one.
     sync_api::WriteTransaction trans(FROM_HERE, sync_service_->GetUserShare());
     sync_api::ReadNode root(&trans);
     if (!root.InitByTagLookup(kSessionsTag)) {
       LOG(ERROR) << kNoSessionsFolderError;
-      return 0;
+      return -1;
     }
     size_t tab_node_id = tab_syncid_pool_.size();
     std::string tab_node_tag = TabIdToTag(machine_tag_, tab_node_id);
     sync_api::WriteNode tab_node(&trans);
     if (!tab_node.InitUniqueByCreation(syncable::SESSIONS, root,
                                        tab_node_tag)) {
-      LOG(ERROR) << "Could not create new node!";
+      LOG(ERROR) << "Could not create new node with tag "
+                 << tab_node_tag << "!";
       return -1;
     }
     tab_node.SetTitle(UTF8ToWide(tab_node_tag));
 
     // Grow the pool by 1 since we created a new node. We don't actually need
     // to put the node's id in the pool now, since the pool is still empty.
     // The id will be added when that tab is closed and the node is freed.
     tab_syncid_pool_.resize(tab_node_id + 1);
     VLOG(1) << "Adding sync node " << tab_node.GetId() << " to tab syncid pool";
     return tab_node.GetId();
@@ skipping 52 matching lines @@
   }
   if (num_populated == 0)
     return true;
   return false;
 }
 
 // Valid local tab?
 bool SessionModelAssociator::IsValidTab(const SyncedTabDelegate& tab) {
   DCHECK(CalledOnValidThread());
   if ((tab.profile() == sync_service_->profile() ||
-       sync_service_->profile() == NULL)) {
+       sync_service_->profile() == NULL)) {          // For tests.
+    const SyncedWindowDelegate* window =
+        SyncedWindowDelegate::FindSyncedWindowDelegateWithId(
+            tab.GetWindowId());
+    if (!window)
+      return false;
     const NavigationEntry* entry = tab.GetActiveEntry();
     if (!entry)
       return false;
     if (entry->virtual_url().is_valid() &&
         (entry->virtual_url() != GURL(chrome::kChromeUINewTabURL) ||
          tab.GetEntryCount() > 1)) {
       return true;
     }
   }
   return false;
@@ skipping 194 matching lines @@
 bool SessionModelAssociator::CryptoReadyIfNecessary() {
   // We only access the cryptographer while holding a transaction.
   sync_api::ReadTransaction trans(FROM_HERE, sync_service_->GetUserShare());
   syncable::ModelTypeSet encrypted_types;
   encrypted_types = sync_api::GetEncryptedTypes(&trans);
   return encrypted_types.count(syncable::SESSIONS) == 0 ||
          sync_service_->IsCryptographerReady(&trans);
 }
 
 }  // namespace browser_sync