[Sync] Fix bug in JsSyncManagerObserver leading to erroneous change info.

chrome/browser/sync/js_sync_manager_observer_unittest.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/js_sync_manager_observer.h"
 
 #include <cstddef>
 
 #include "base/basictypes.h"
 #include "base/values.h"
@@ skipping 161 matching lines @@
 
   TestDirectorySetterUpper setter_upper;
   sync_api::UserShare share;
   setter_upper.SetUp();
   share.dir_manager.reset(setter_upper.manager());
   share.name = setter_upper.name();
 
   // We don't test with passwords as that requires additional setup.
 
   // Build a list of example ChangeRecords.
-  sync_api::SyncManager::ChangeRecord changes[syncable::PASSWORDS];
-  for (int i = syncable::FIRST_REAL_MODEL_TYPE;
-       i < syncable::PASSWORDS; ++i) {
+  sync_api::SyncManager::ChangeRecord changes[syncable::MODEL_TYPE_COUNT];
+  for (int i = syncable::AUTOFILL_PROFILE;
+       i < syncable::MODEL_TYPE_COUNT; ++i) {
     changes[i].id = MakeNode(&share, syncable::ModelTypeFromInt(i));
     switch (i % 3) {
       case 0:
         changes[i].action =
             sync_api::SyncManager::ChangeRecord::ACTION_ADD;
         break;
       case 1:
         changes[i].action =
             sync_api::SyncManager::ChangeRecord::ACTION_UPDATE;
         break;
       default:
         changes[i].action =
             sync_api::SyncManager::ChangeRecord::ACTION_DELETE;
         break;
     }
     {
       sync_api::ReadTransaction trans(&share);
       sync_api::ReadNode node(&trans);
       EXPECT_TRUE(node.InitByIdLookup(changes[i].id));
       changes[i].specifics = node.GetEntry()->Get(syncable::SPECIFICS);
     }
   }
 
   // Set expectations for each data type.
-  for (int i = syncable::FIRST_REAL_MODEL_TYPE;
-       i < syncable::PASSWORDS; ++i) {
+  for (int i = syncable::AUTOFILL_PROFILE;
+       i < syncable::MODEL_TYPE_COUNT; ++i) {
     const std::string& model_type_str =
         syncable::ModelTypeToString(syncable::ModelTypeFromInt(i));
     ListValue expected_args;
     expected_args.Append(Value::CreateStringValue(model_type_str));
     ListValue* expected_changes = new ListValue();
     expected_args.Append(expected_changes);
-    for (int j = i; j < syncable::PASSWORDS; ++j) {
+    for (int j = i; j < syncable::MODEL_TYPE_COUNT; ++j) {
       sync_api::ReadTransaction trans(&share);
-      expected_changes->Append(changes[i].ToValue(&trans));
+      expected_changes->Append(changes[j].ToValue(&trans));
     }
     EXPECT_CALL(mock_router_,
                 RouteJsEvent("onChangesApplied",
                              HasArgsAsList(expected_args), NULL));
   }
 
   // Fire OnChangesApplied() for each data type.

[Nicolas Zea, 2011/02/18 06:09:52]

A comment here about the fact that you're building a list of changes _starting_
with that specified datatype and including all datatypes higher than it would be
useful. It took me a bit to figure out why the expectations involved multiple
datatypes (since OnChangesApplied refers to only one datatype).

[akalin, 2011/02/18 08:12:29]

Added comment, but above the expectations too since it involves that too.

-  for (int i = syncable::FIRST_REAL_MODEL_TYPE;
-       i < syncable::PASSWORDS; ++i) {
+  for (int i = syncable::AUTOFILL_PROFILE;
+       i < syncable::MODEL_TYPE_COUNT; ++i) {
     sync_api::ReadTransaction trans(&share);
     sync_manager_observer_.OnChangesApplied(syncable::ModelTypeFromInt(i),
                                             &trans, &changes[i],
-                                            syncable::PASSWORDS - i);
+                                            syncable::MODEL_TYPE_COUNT - i);
   }
 
   // |share.dir_manager| does not actually own its value.
   ignore_result(share.dir_manager.release());
   setter_upper.TearDown();
 }
 
 }  // namespace
 }  // namespace browser_sync