Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(758)

Side by Side Diff: chrome/browser/sync/glue/bookmark_change_processor.cc

Issue 10694126: Revert 146032 - [Sync] Check if bookmark exists before attempting favicon update (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src/
Patch Set: Created 8 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
« no previous file with comments | « no previous file | chrome/browser/sync/profile_sync_service_bookmark_unittest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "chrome/browser/sync/glue/bookmark_change_processor.h" 5 #include "chrome/browser/sync/glue/bookmark_change_processor.h"
6 6
7 #include <stack> 7 #include <stack>
8 #include <vector> 8 #include <vector>
9 9
10 #include "base/location.h" 10 #include "base/location.h"
11 #include "base/string16.h" 11 #include "base/string16.h"
12 #include "base/string_util.h" 12 #include "base/string_util.h"
13 #include "base/utf_string_conversions.h" 13 #include "base/utf_string_conversions.h"
14 #include "chrome/browser/bookmarks/bookmark_model.h" 14 #include "chrome/browser/bookmarks/bookmark_model.h"
15 #include "chrome/browser/bookmarks/bookmark_utils.h" 15 #include "chrome/browser/bookmarks/bookmark_utils.h"
16 #include "chrome/browser/favicon/favicon_service.h" 16 #include "chrome/browser/favicon/favicon_service.h"
17 #include "chrome/browser/profiles/profile.h" 17 #include "chrome/browser/profiles/profile.h"
18 #include "chrome/browser/sync/profile_sync_service.h" 18 #include "chrome/browser/sync/profile_sync_service.h"
19 #include "content/public/browser/browser_thread.h" 19 #include "content/public/browser/browser_thread.h"
20 #include "sync/internal_api/public/change_record.h" 20 #include "sync/internal_api/public/change_record.h"
21 #include "sync/internal_api/public/read_node.h" 21 #include "sync/internal_api/public/read_node.h"
22 #include "sync/internal_api/public/write_node.h" 22 #include "sync/internal_api/public/write_node.h"
23 #include "sync/internal_api/public/write_transaction.h" 23 #include "sync/internal_api/public/write_transaction.h"
24 #include "sync/syncable/entry.h" // TODO(tim): Investigating bug 121587.
24 #include "ui/gfx/image/image_util.h" 25 #include "ui/gfx/image/image_util.h"
25 26
26 using content::BrowserThread; 27 using content::BrowserThread;
27 28
28 namespace browser_sync { 29 namespace browser_sync {
29 30
30 static const char kMobileBookmarksTag[] = "synced_bookmarks"; 31 static const char kMobileBookmarksTag[] = "synced_bookmarks";
31 32
32 BookmarkChangeProcessor::BookmarkChangeProcessor( 33 BookmarkChangeProcessor::BookmarkChangeProcessor(
33 BookmarkModelAssociator* model_associator, 34 BookmarkModelAssociator* model_associator,
(...skipping 172 matching lines...) Expand 10 before | Expand all | Expand 10 after
206 NOTREACHED() << "Saw update to permanent node!"; 207 NOTREACHED() << "Saw update to permanent node!";
207 return; 208 return;
208 } 209 }
209 210
210 // Acquire a scoped write lock via a transaction. 211 // Acquire a scoped write lock via a transaction.
211 syncer::WriteTransaction trans(FROM_HERE, share_handle()); 212 syncer::WriteTransaction trans(FROM_HERE, share_handle());
212 213
213 // Lookup the sync node that's associated with |node|. 214 // Lookup the sync node that's associated with |node|.
214 syncer::WriteNode sync_node(&trans); 215 syncer::WriteNode sync_node(&trans);
215 if (!model_associator_->InitSyncNodeFromChromeId(node->id(), &sync_node)) { 216 if (!model_associator_->InitSyncNodeFromChromeId(node->id(), &sync_node)) {
216 error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE, 217 // TODO(tim): Investigating bug 121587.
217 "Failed to load sync node for updated bookmark."); 218 if (model_associator_->GetSyncIdFromChromeId(node->id()) ==
219 syncer::kInvalidId) {
220 error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
221 "Bookmark id not found in model associator on BookmarkNodeChanged");
222 LOG(ERROR) << "Bad id.";
223 } else if (!sync_node.GetEntry()->good()) {
224 error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
225 "Could not InitByIdLookup on BookmarkNodeChanged, good() failed");
226 LOG(ERROR) << "Bad entry.";
227 } else if (sync_node.GetEntry()->Get(syncer::syncable::IS_DEL)) {
228 error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
229 "Could not InitByIdLookup on BookmarkNodeChanged, is_del true");
230 LOG(ERROR) << "Deleted entry.";
231 } else {
232 syncer::Cryptographer* crypto = trans.GetCryptographer();
233 syncer::ModelTypeSet encrypted_types(crypto->GetEncryptedTypes());
234 const sync_pb::EntitySpecifics& specifics =
235 sync_node.GetEntry()->Get(syncer::syncable::SPECIFICS);
236 CHECK(specifics.has_encrypted());
237 const bool can_decrypt = crypto->CanDecrypt(specifics.encrypted());
238 const bool agreement = encrypted_types.Has(syncer::BOOKMARKS);
239 if (!agreement && !can_decrypt) {
240 error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
241 "Could not InitByIdLookup on BookmarkNodeChanged, "
242 " Cryptographer thinks bookmarks not encrypted, and CanDecrypt"
243 " failed.");
244 LOG(ERROR) << "Case 1.";
245 } else if (agreement && can_decrypt) {
246 error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
247 "Could not InitByIdLookup on BookmarkNodeChanged, "
248 " Cryptographer thinks bookmarks are encrypted, and CanDecrypt"
249 " succeeded (?!), but DecryptIfNecessary failed.");
250 LOG(ERROR) << "Case 2.";
251 } else if (agreement) {
252 error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
253 "Could not InitByIdLookup on BookmarkNodeChanged, "
254 " Cryptographer thinks bookmarks are encrypted, but CanDecrypt"
255 " failed.");
256 LOG(ERROR) << "Case 3.";
257 } else {
258 error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
259 "Could not InitByIdLookup on BookmarkNodeChanged, "
260 " Cryptographer thinks bookmarks not encrypted, but CanDecrypt"
261 " succeeded (super weird, btw)");
262 LOG(ERROR) << "Case 4.";
263 }
264 }
218 return; 265 return;
219 } 266 }
220 267
221 UpdateSyncNodeProperties(node, model, &sync_node); 268 UpdateSyncNodeProperties(node, model, &sync_node);
222 269
223 DCHECK_EQ(sync_node.GetIsFolder(), node->is_folder()); 270 DCHECK_EQ(sync_node.GetIsFolder(), node->is_folder());
224 DCHECK_EQ(model_associator_->GetChromeNodeFromSyncId( 271 DCHECK_EQ(model_associator_->GetChromeNodeFromSyncId(
225 sync_node.GetParentId()), 272 sync_node.GetParentId()),
226 node->parent()); 273 node->parent());
227 // This node's index should be one more than the predecessor's index. 274 // This node's index should be one more than the predecessor's index.
(...skipping 30 matching lines...) Expand all
258 error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE, 305 error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
259 std::string()); 306 std::string());
260 return; 307 return;
261 } 308 }
262 } 309 }
263 310
264 void BookmarkChangeProcessor::BookmarkNodeFaviconChanged( 311 void BookmarkChangeProcessor::BookmarkNodeFaviconChanged(
265 BookmarkModel* model, 312 BookmarkModel* model,
266 const BookmarkNode* node) { 313 const BookmarkNode* node) {
267 DCHECK(running()); 314 DCHECK(running());
268 // Because favicon changes come from a different thread, it's possible we're
269 // being notified of a favicon change for a url whose bookmark was recently
270 // deleted. Check if we have a sync node for this bookmark, and if not ignore
271 // the favicon change.
272 if (model_associator_->GetSyncIdFromChromeId(node->id()) ==
273 syncer::kInvalidId)
274 return;
275 BookmarkNodeChanged(model, node); 315 BookmarkNodeChanged(model, node);
276 } 316 }
277 317
278 void BookmarkChangeProcessor::BookmarkNodeChildrenReordered( 318 void BookmarkChangeProcessor::BookmarkNodeChildrenReordered(
279 BookmarkModel* model, const BookmarkNode* node) { 319 BookmarkModel* model, const BookmarkNode* node) {
280 320
281 // Acquire a scoped write lock via a transaction. 321 // Acquire a scoped write lock via a transaction.
282 syncer::WriteTransaction trans(FROM_HERE, share_handle()); 322 syncer::WriteTransaction trans(FROM_HERE, share_handle());
283 323
284 // The given node's children got reordered. We need to reorder all the 324 // The given node's children got reordered. We need to reorder all the
(...skipping 325 matching lines...) Expand 10 before | Expand all | Expand 10 after
610 const BookmarkNode* bookmark_node, 650 const BookmarkNode* bookmark_node,
611 BookmarkModel* model, 651 BookmarkModel* model,
612 syncer::WriteNode* sync_node) { 652 syncer::WriteNode* sync_node) {
613 std::vector<unsigned char> favicon_bytes; 653 std::vector<unsigned char> favicon_bytes;
614 EncodeFavicon(bookmark_node, model, &favicon_bytes); 654 EncodeFavicon(bookmark_node, model, &favicon_bytes);
615 if (!favicon_bytes.empty()) 655 if (!favicon_bytes.empty())
616 sync_node->SetFaviconBytes(favicon_bytes); 656 sync_node->SetFaviconBytes(favicon_bytes);
617 } 657 }
618 658
619 } // namespace browser_sync 659 } // namespace browser_sync
OLDNEW
« no previous file with comments | « no previous file | chrome/browser/sync/profile_sync_service_bookmark_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698