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

Unified Diff: chrome/browser/sync/glue/theme_change_processor.cc

Issue 1622030: Made ThemeChangeProcessor handle multiple updates to work around (Closed)
Patch Set: Added link to bug Created 10 years, 8 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 side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/sync/glue/theme_change_processor.cc
diff --git a/chrome/browser/sync/glue/theme_change_processor.cc b/chrome/browser/sync/glue/theme_change_processor.cc
index 01cebfb1d815175692572c3dfc6385b037baf86e..ec61f656660a10681bcdc5819ae0eaa481832965 100644
--- a/chrome/browser/sync/glue/theme_change_processor.cc
+++ b/chrome/browser/sync/glue/theme_change_processor.cc
@@ -138,28 +138,41 @@ void ThemeChangeProcessor::ApplyChangesFromSyncModel(
if (!running()) {
return;
}
- StopObserving();
- if (change_count != 1) {
- LOG(ERROR) << "Unexpected number of theme changes";
+ // TODO(akalin): Normally, we should only have a single change and
+ // it should be an update. However, the syncapi may occasionally
+ // generates multiple changes. When we fix syncapi to not do that,
+ // we can remove the extra logic below. See:
+ // http://code.google.com/p/chromium/issues/detail?id=41696 .
+ if (change_count < 1) {
+ LOG(ERROR) << "Unexpected change_count: " << change_count;
error_handler()->OnUnrecoverableError();
return;
}
- const sync_api::SyncManager::ChangeRecord& change = changes[0];
+ if (change_count > 1) {
+ LOG(WARNING) << change_count << " theme changes detected; "
+ << "only applying the last one";
+ }
+ const sync_api::SyncManager::ChangeRecord& change =
+ changes[change_count - 1];
if (change.action != sync_api::SyncManager::ChangeRecord::ACTION_UPDATE) {
- LOG(ERROR) << "Unexpected change.action " << change.action;
- error_handler()->OnUnrecoverableError();
- return;
+ LOG(WARNING) << "strange theme change.action: " << change.action;
}
- sync_api::ReadNode node(trans);
- if (!node.InitByIdLookup(change.id)) {
- LOG(ERROR) << "Theme node lookup failed";
- error_handler()->OnUnrecoverableError();
- return;
+ sync_pb::ThemeSpecifics theme_specifics;
+ // If the action is a delete, simply use the default values for
+ // ThemeSpecifics, which would cause the default theme to be set.
+ if (change.action != sync_api::SyncManager::ChangeRecord::ACTION_DELETE) {
+ sync_api::ReadNode node(trans);
+ if (!node.InitByIdLookup(change.id)) {
+ LOG(ERROR) << "Theme node lookup failed";
+ error_handler()->OnUnrecoverableError();
+ return;
+ }
+ DCHECK_EQ(node.GetModelType(), syncable::THEMES);
+ DCHECK(profile_);
+ theme_specifics = node.GetThemeSpecifics();
}
- DCHECK_EQ(node.GetModelType(), syncable::THEMES);
- DCHECK(profile_);
- SetCurrentThemeFromThemeSpecificsIfNecessary(
- node.GetThemeSpecifics(), profile_);
+ StopObserving();
+ SetCurrentThemeFromThemeSpecificsIfNecessary(theme_specifics, profile_);
StartObserving();
}
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698