[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();
   } 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 308 matching lines @@
     foreign_session->windows.resize(header.window_size());
   } else if (specifics.has_tab()) {
     const sync_pb::SessionTab& tab_s = specifics.tab();
     SessionID::id_type tab_id = tab_s.tab_id();
     SessionTab* tab =
         synced_session_tracker_.GetSessionTab(foreign_session_tag,
                                               tab_id,
                                               false);
     PopulateSessionTabFromSpecifics(tab_s, modification_time, tab);
   } else {
     NOTREACHED();

[Yaron, 2011/08/23 00:31:54]

Do you still want to leave this in? It seems like we could protect ourselves in
this case. Basically, if it's a corrupt entry that's not a tab or a window, just
ignore it but don't fail association.

     return false;
   }
 
   return true;
 }
 
 void SessionModelAssociator::DisassociateForeignSession(
     const std::string& foreign_session_tag) {
   DCHECK(CalledOnValidThread());
   synced_session_tracker_.DeleteSession(foreign_session_tag);
@@ skipping 244 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