[Sync] Check if bookmark exists before attempting favicon update

chrome/browser/sync/glue/bookmark_change_processor.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/bookmark_change_processor.h"
 
 #include <stack>
 #include <vector>
 
 #include "base/location.h"
 #include "base/string16.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/bookmarks/bookmark_utils.h"
 #include "chrome/browser/favicon/favicon_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "content/public/browser/browser_thread.h"
 #include "sync/internal_api/public/change_record.h"
 #include "sync/internal_api/public/read_node.h"
 #include "sync/internal_api/public/write_node.h"
 #include "sync/internal_api/public/write_transaction.h"
-#include "sync/syncable/entry.h"  // TODO(tim): Investigating bug 121587.
 #include "ui/gfx/image/image_util.h"
 
 using content::BrowserThread;
 
 namespace browser_sync {
 
 static const char kMobileBookmarksTag[] = "synced_bookmarks";
 
 BookmarkChangeProcessor::BookmarkChangeProcessor(
     BookmarkModelAssociator* model_associator,
@@ skipping 172 matching lines @@
     NOTREACHED() << "Saw update to permanent node!";
     return;
   }
 
   // Acquire a scoped write lock via a transaction.
   syncer::WriteTransaction trans(FROM_HERE, share_handle());
 
   // Lookup the sync node that's associated with |node|.
   syncer::WriteNode sync_node(&trans);
   if (!model_associator_->InitSyncNodeFromChromeId(node->id(), &sync_node)) {
-    // TODO(tim): Investigating bug 121587.
-    if (model_associator_->GetSyncIdFromChromeId(node->id()) ==
-                                                 syncer::kInvalidId) {
-      error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-          "Bookmark id not found in model associator on BookmarkNodeChanged");
-      LOG(ERROR) << "Bad id.";
-    } else if (!sync_node.GetEntry()->good()) {
-      error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-          "Could not InitByIdLookup on BookmarkNodeChanged, good() failed");
-      LOG(ERROR) << "Bad entry.";
-    } else if (sync_node.GetEntry()->Get(syncer::syncable::IS_DEL)) {
-      error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-          "Could not InitByIdLookup on BookmarkNodeChanged, is_del true");
-      LOG(ERROR) << "Deleted entry.";
-    } else {
-      syncer::Cryptographer* crypto = trans.GetCryptographer();
-      syncer::ModelTypeSet encrypted_types(crypto->GetEncryptedTypes());
-      const sync_pb::EntitySpecifics& specifics =
-          sync_node.GetEntry()->Get(syncer::syncable::SPECIFICS);
-      CHECK(specifics.has_encrypted());
-      const bool can_decrypt = crypto->CanDecrypt(specifics.encrypted());
-      const bool agreement = encrypted_types.Has(syncer::BOOKMARKS);
-      if (!agreement && !can_decrypt) {
-        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-            "Could not InitByIdLookup on BookmarkNodeChanged, "
-            " Cryptographer thinks bookmarks not encrypted, and CanDecrypt"
-            " failed.");
-        LOG(ERROR) << "Case 1.";
-      } else if (agreement && can_decrypt) {
-        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-            "Could not InitByIdLookup on BookmarkNodeChanged, "
-            " Cryptographer thinks bookmarks are encrypted, and CanDecrypt"
-            " succeeded (?!), but DecryptIfNecessary failed.");
-        LOG(ERROR) << "Case 2.";
-      } else if (agreement) {
-        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-            "Could not InitByIdLookup on BookmarkNodeChanged, "
-            " Cryptographer thinks bookmarks are encrypted, but CanDecrypt"
-            " failed.");
-        LOG(ERROR) << "Case 3.";
-      } else {
-        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-            "Could not InitByIdLookup on BookmarkNodeChanged, "
-            " Cryptographer thinks bookmarks not encrypted, but CanDecrypt"
-            " succeeded (super weird, btw)");
-        LOG(ERROR) << "Case 4.";
-      }
-    }
+    error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
+          "Failed to load sync node for updated bookmark.");
     return;
   }
 
   UpdateSyncNodeProperties(node, model, &sync_node);
 
   DCHECK_EQ(sync_node.GetIsFolder(), node->is_folder());
   DCHECK_EQ(model_associator_->GetChromeNodeFromSyncId(
             sync_node.GetParentId()),
             node->parent());
   // This node's index should be one more than the predecessor's index.
@@ skipping 30 matching lines @@
     error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
                                                         std::string());
     return;
   }
 }
 
 void BookmarkChangeProcessor::BookmarkNodeFaviconChanged(
     BookmarkModel* model,
     const BookmarkNode* node) {
   DCHECK(running());
+  // Because favicon changes come from a different thread, it's possible we're
+  // being notified of a favicon change for a url whose bookmark was recently
+  // deleted. Check if we have a sync node for this bookmark, and if not ignore
+  // the favicon change.
+  if (model_associator_->GetSyncIdFromChromeId(node->id()) ==
+          syncer::kInvalidId)
+    return;
   BookmarkNodeChanged(model, node);
 }
 
 void BookmarkChangeProcessor::BookmarkNodeChildrenReordered(
     BookmarkModel* model, const BookmarkNode* node) {
 
   // Acquire a scoped write lock via a transaction.
   syncer::WriteTransaction trans(FROM_HERE, share_handle());
 
   // The given node's children got reordered. We need to reorder all the
@@ skipping 325 matching lines @@
     const BookmarkNode* bookmark_node,
     BookmarkModel* model,
     syncer::WriteNode* sync_node) {
   std::vector<unsigned char> favicon_bytes;
   EncodeFavicon(bookmark_node, model, &favicon_bytes);
   if (!favicon_bytes.empty())
     sync_node->SetFaviconBytes(favicon_bytes);
 }
 
 }  // namespace browser_sync