Current code will definitely crash if ACTION_DELETE is passed to ThemeChangeProcessor, since the error will clear the profile_ without returning, and the profile is accessed later. Since the code discusses how to process deletes, I think this is the correct fix.

BUG=none
TEST=sync themes and then delete it

Review URL: http://codereview.chromium.org/2913006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@51976 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 5 insertions, 3 deletions

diff --git a/chrome/browser/sync/glue/theme_change_processor.cc b/chrome/browser/sync/glue/theme_change_processor.cc
index c4e4f68..58c3df3 100644
--- a/chrome/browser/sync/glue/theme_change_processor.cc
+++ b/chrome/browser/sync/glue/theme_change_processor.cc
@@ -155,9 +155,11 @@ void ThemeChangeProcessor::ApplyChangesFromSyncModel(
   }
   const sync_api::SyncManager::ChangeRecord& change =
       changes[change_count - 1];
-  if (change.action != sync_api::SyncManager::ChangeRecord::ACTION_UPDATE) {
+  if (change.action != sync_api::SyncManager::ChangeRecord::ACTION_UPDATE &&
+      change.action != sync_api::SyncManager::ChangeRecord::ACTION_DELETE) {
     std::string err = "strange theme change.action " + change.action;
     error_handler()->OnUnrecoverableError(FROM_HERE, err);
+    return;
   }
   sync_pb::ThemeSpecifics theme_specifics;
   // If the action is a delete, simply use the default values for
@@ -165,8 +167,8 @@ void ThemeChangeProcessor::ApplyChangesFromSyncModel(
   if (change.action != sync_api::SyncManager::ChangeRecord::ACTION_DELETE) {
     sync_api::ReadNode node(trans);
     if (!node.InitByIdLookup(change.id)) {
-    error_handler()->OnUnrecoverableError(FROM_HERE,
-                                          "Theme node lookup failed.");
+      error_handler()->OnUnrecoverableError(FROM_HERE,
+                                            "Theme node lookup failed.");
       return;
     }
     DCHECK_EQ(node.GetModelType(), syncable::THEMES);