Hook up client side of per-datatype GetUpdates.

Which datatypes are fetched is dictated by the ModelSafeRoutingInfo.  We change the semantic of the ModelSafeRoutingInfo so that datatypes which should not be synced are not in the map.  We will do GetUpdates for GROUP_PASSIVE datatypes.

BUG=29905
TEST=included unit tests

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

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

21 files changed, 417 insertions, 80 deletions

diff --git a/chrome/browser/sync/engine/build_commit_command.cc b/chrome/browser/sync/engine/build_commit_command.cc
index b945fd7..1cf0ae6 100755
--- a/chrome/browser/sync/engine/build_commit_command.cc
+++ b/chrome/browser/sync/engine/build_commit_command.cc
@@ -20,9 +20,11 @@ using std::set;
 using std::string;
 using std::vector;
 using syncable::ExtendedAttribute;
+using syncable::IS_DEL;
 using syncable::Id;
 using syncable::MutableEntry;
 using syncable::SPECIFICS;
+using syncable::UNSPECIFIED;
 
 namespace browser_sync {
 
@@ -96,6 +98,9 @@ void BuildCommitCommand::ExecuteImpl(SyncSession* session) {
     // This is the only change we make to the entry in this function.
     meta_entry.Put(syncable::SYNCING, true);
 
+    DCHECK(0 != session->routing_info().count(meta_entry.GetModelType()))
+        << "Committing change to datatype that's not actively enabled.";
+
     string name = meta_entry.Get(syncable::NON_UNIQUE_NAME);
     CHECK(!name.empty());  // Make sure this isn't an update.
     sync_entry->set_name(name);
@@ -159,18 +164,21 @@ void BuildCommitCommand::ExecuteImpl(SyncSession* session) {
     }
 
     // Deletion is final on the server, let's move things and then delete them.
-    if (meta_entry.Get(syncable::IS_DEL)) {
+    if (meta_entry.Get(IS_DEL)) {
       sync_entry->set_deleted(true);
-    } else if (meta_entry.Get(SPECIFICS).HasExtension(sync_pb::bookmark)) {
-      // Common data in both new and old protocol.
-      const Id& prev_id = meta_entry.Get(syncable::PREV_ID);
-      string prev_string = prev_id.IsRoot() ? string() : prev_id.GetServerId();
-      sync_entry->set_insert_after_item_id(prev_string);
-
-      // TODO(ncarter): In practice we won't want to send this data twice
-      // over the wire; instead, when deployed servers are able to accept
-      // the new-style scheme, we should abandon the old way.
-      SetOldStyleBookmarkData(&meta_entry, sync_entry);
+    } else {
+      if (meta_entry.Get(SPECIFICS).HasExtension(sync_pb::bookmark)) {
+        // Common data in both new and old protocol.
+        const Id& prev_id = meta_entry.Get(syncable::PREV_ID);
+        string prev_id_string =
+            prev_id.IsRoot() ? string() : prev_id.GetServerId();
+        sync_entry->set_insert_after_item_id(prev_id_string);
+
+        // TODO(ncarter): In practice we won't want to send this data twice
+        // over the wire; instead, when deployed servers are able to accept
+        // the new-style scheme, we should abandon the old way.
+        SetOldStyleBookmarkData(&meta_entry, sync_entry);
+      }
       SetEntrySpecifics(&meta_entry, sync_entry);
     }
   }
diff --git a/chrome/browser/sync/engine/download_updates_command.cc b/chrome/browser/sync/engine/download_updates_command.cc
index 9b75ec2..cc58874 100644
--- a/chrome/browser/sync/engine/download_updates_command.cc
+++ b/chrome/browser/sync/engine/download_updates_command.cc
@@ -42,6 +42,15 @@ void DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) {
   LOG(INFO) << "Getting updates from ts " << dir->last_sync_timestamp();
   get_updates->set_from_timestamp(dir->last_sync_timestamp());
 
+  // We want folders for our associated types, always.  If we were to set
+  // this to false, the server would send just the non-container items
+  // (e.g. Bookmark URLs but not their containing folders).
+  get_updates->set_fetch_folders(true);
+
+  // Set the requested_types protobuf field so that we fetch all enabled types.
+  SetRequestedTypes(session->routing_info(),
+                    get_updates->mutable_requested_types());
+
   // Set GetUpdatesMessage.GetUpdatesCallerInfo information.
   get_updates->mutable_caller_info()->set_source(session->TestAndSetSource());
   get_updates->mutable_caller_info()->set_notifications_enabled(
@@ -62,4 +71,20 @@ void DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) {
   status->mutable_updates_response()->CopyFrom(update_response);
 }
 
+void DownloadUpdatesCommand::SetRequestedTypes(
+    const ModelSafeRoutingInfo& routing_info,
+    sync_pb::EntitySpecifics* types) {
+  // The datatypes which should be synced are dictated by the value of the
+  // ModelSafeRoutingInfo.  If a datatype is in the routing info map, it
+  // should be synced (even if it's GROUP_PASSIVE).
+  int requested_type_count = 0;
+  for (ModelSafeRoutingInfo::const_iterator i = routing_info.begin();
+       i != routing_info.end();
+       ++i) {
+    requested_type_count++;
+    syncable::AddDefaultExtensionValue(i->first, types);
+  }
+  DCHECK_LT(0, requested_type_count) << "Doing GetUpdates with empty filter.";
+}
+
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/engine/download_updates_command.h b/chrome/browser/sync/engine/download_updates_command.h
index 6169bb7..cef42fb 100644
--- a/chrome/browser/sync/engine/download_updates_command.h
+++ b/chrome/browser/sync/engine/download_updates_command.h
@@ -6,8 +6,13 @@
 #define CHROME_BROWSER_SYNC_ENGINE_DOWNLOAD_UPDATES_COMMAND_H_
 
 #include "base/basictypes.h"
+#include "chrome/browser/sync/engine/model_safe_worker.h"
 #include "chrome/browser/sync/engine/syncer_command.h"
 
+namespace sync_pb {
+class EntitySpecifics;
+}
+
 namespace browser_sync {
 
 // Downloads updates from the server and places them in the SyncSession.
@@ -19,6 +24,9 @@ class DownloadUpdatesCommand : public SyncerCommand {
   // SyncerCommand implementation.
   virtual void ExecuteImpl(sessions::SyncSession* session);
 
+  void SetRequestedTypes(const ModelSafeRoutingInfo& routing_info,
+                         sync_pb::EntitySpecifics* types);
+
  private:
   DISALLOW_COPY_AND_ASSIGN(DownloadUpdatesCommand);
 };
diff --git a/chrome/browser/sync/engine/download_updates_command_unittest.cc b/chrome/browser/sync/engine/download_updates_command_unittest.cc
new file mode 100755
index 0000000..f66232a
--- /dev/null
+++ b/chrome/browser/sync/engine/download_updates_command_unittest.cc
@@ -0,0 +1,89 @@
+// Copyright (c) 2010 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/engine/download_updates_command.h"
+#include "chrome/browser/sync/protocol/autofill_specifics.pb.h"
+#include "chrome/browser/sync/protocol/bookmark_specifics.pb.h"
+#include "chrome/browser/sync/protocol/preference_specifics.pb.h"
+#include "chrome/browser/sync/protocol/sync.pb.h"
+#include "chrome/test/sync/engine/proto_extension_validator.h"
+#include "chrome/test/sync/engine/syncer_command_test.h"
+
+namespace browser_sync {
+
+// A test fixture for tests exercising DownloadUpdatesCommandTest.
+class DownloadUpdatesCommandTest : public SyncerCommandTest {
+ protected:
+  DownloadUpdatesCommandTest() {}
+  DownloadUpdatesCommand command_;
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DownloadUpdatesCommandTest);
+};
+
+TEST_F(DownloadUpdatesCommandTest, SetRequestedTypes) {
+  {
+    SCOPED_TRACE("Several enabled datatypes, spread out across groups.");
+    ModelSafeRoutingInfo routing_info;
+    routing_info[syncable::BOOKMARKS] = GROUP_UI;
+    routing_info[syncable::AUTOFILL] = GROUP_DB;
+    routing_info[syncable::PREFERENCES] = GROUP_PASSIVE;
+    sync_pb::EntitySpecifics get_updates_filter;
+    command_.SetRequestedTypes(routing_info, &get_updates_filter);
+    ProtoExtensionValidator<sync_pb::EntitySpecifics> v(get_updates_filter);
+    v.ExpectHasExtension(sync_pb::autofill);
+    v.ExpectHasExtension(sync_pb::preference);
+    v.ExpectHasExtension(sync_pb::bookmark);
+    v.ExpectNoOtherFieldsOrExtensions();
+  }
+
+  {
+    SCOPED_TRACE("Several enabled datatypes, one work group.");
+    ModelSafeRoutingInfo routing_info;
+    routing_info[syncable::BOOKMARKS] = GROUP_UI;
+    routing_info[syncable::AUTOFILL] = GROUP_UI;
+    routing_info[syncable::PREFERENCES] = GROUP_UI;
+    sync_pb::EntitySpecifics get_updates_filter;
+    command_.SetRequestedTypes(routing_info, &get_updates_filter);
+    ProtoExtensionValidator<sync_pb::EntitySpecifics> v(get_updates_filter);
+    v.ExpectHasExtension(sync_pb::autofill);
+    v.ExpectHasExtension(sync_pb::preference);
+    v.ExpectHasExtension(sync_pb::bookmark);
+    v.ExpectNoOtherFieldsOrExtensions();
+  }
+
+  {
+    SCOPED_TRACE("Bookmarks only.");
+    ModelSafeRoutingInfo routing_info;
+    routing_info[syncable::BOOKMARKS] = GROUP_PASSIVE;
+    sync_pb::EntitySpecifics get_updates_filter;
+    command_.SetRequestedTypes(routing_info, &get_updates_filter);
+    ProtoExtensionValidator<sync_pb::EntitySpecifics> v(get_updates_filter);
+    v.ExpectHasExtension(sync_pb::bookmark);
+    v.ExpectNoOtherFieldsOrExtensions();
+  }
+
+  {
+    SCOPED_TRACE("Autofill only.");
+    ModelSafeRoutingInfo routing_info;
+    routing_info[syncable::AUTOFILL] = GROUP_PASSIVE;
+    sync_pb::EntitySpecifics get_updates_filter;
+    command_.SetRequestedTypes(routing_info, &get_updates_filter);
+    ProtoExtensionValidator<sync_pb::EntitySpecifics> v(get_updates_filter);
+    v.ExpectHasExtension(sync_pb::autofill);
+    v.ExpectNoOtherFieldsOrExtensions();
+  }
+
+  {
+    SCOPED_TRACE("Preferences only.");
+    ModelSafeRoutingInfo routing_info;
+    routing_info[syncable::PREFERENCES] = GROUP_PASSIVE;
+    sync_pb::EntitySpecifics get_updates_filter;
+    command_.SetRequestedTypes(routing_info, &get_updates_filter);
+    ProtoExtensionValidator<sync_pb::EntitySpecifics> v(get_updates_filter);
+    v.ExpectHasExtension(sync_pb::preference);
+    v.ExpectNoOtherFieldsOrExtensions();
+  }
+}
+
+}  // namespace browser_sync
diff --git a/chrome/browser/sync/engine/model_safe_worker.h b/chrome/browser/sync/engine/model_safe_worker.h
index 84a6a2f..e44e342 100644
--- a/chrome/browser/sync/engine/model_safe_worker.h
+++ b/chrome/browser/sync/engine/model_safe_worker.h
@@ -74,7 +74,9 @@ class ModelSafeWorkerRegistrar {
   // Get the current list of active ModelSafeWorkers.  Should be threadsafe.
   virtual void GetWorkers(std::vector<ModelSafeWorker*>* out) = 0;
 
-  // Get the current routing information for all model types.
+  // Get the current routing information for all enabled model types.
+  // If a model type is not enabled (that is, if the syncer should not
+  // be trying to sync it), it is not in this map.
   virtual void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) = 0;
  protected:
   virtual ~ModelSafeWorkerRegistrar() {}
diff --git a/chrome/browser/sync/engine/syncapi.cc b/chrome/browser/sync/engine/syncapi.cc
index 0fdf3a2..4ae26a2 100755
--- a/chrome/browser/sync/engine/syncapi.cc
+++ b/chrome/browser/sync/engine/syncapi.cc
@@ -568,17 +568,9 @@ bool WriteNode::InitByClientTagLookup(const std::string& tag) {
 void WriteNode::PutModelType(syncable::ModelType model_type) {
   // Set an empty specifics of the appropriate datatype.  The presence
   // of the specific extension will identify the model type.
-  switch (model_type) {
-    case syncable::BOOKMARKS:
-      PutBookmarkSpecificsAndMarkForSyncing(
-          sync_pb::BookmarkSpecifics::default_instance());
-      break;
-    case syncable::PREFERENCES:
-      PutPreferenceSpecificsAndMarkForSyncing(
-          sync_pb::PreferenceSpecifics::default_instance());
-    default:
-      NOTREACHED();
-  }
+  sync_pb::EntitySpecifics specifics;
+  syncable::AddDefaultExtensionValue(model_type, &specifics);
+  PutSpecificsAndMarkForSyncing(specifics);
   DCHECK(GetModelType() == model_type);
 }
 
diff --git a/chrome/browser/sync/engine/syncer_proto_util.h b/chrome/browser/sync/engine/syncer_proto_util.h
index af31251..5bf7931 100755
--- a/chrome/browser/sync/engine/syncer_proto_util.h
+++ b/chrome/browser/sync/engine/syncer_proto_util.h
@@ -8,6 +8,7 @@
 #include <string>
 
 #include "chrome/browser/sync/syncable/blob.h"
+#include "chrome/browser/sync/syncable/model_type.h"
 #include "chrome/browser/sync/util/sync_types.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"  // For FRIEND_TEST
 
@@ -20,6 +21,7 @@ class SyncName;
 
 namespace sync_pb {
 class ClientToServerResponse;
+class EntitySpecifics;
 }  // namespace sync_pb
 
 namespace browser_sync {
@@ -70,6 +72,12 @@ class SyncerProtoUtil {
   static const std::string& NameFromCommitEntryResponse(
       const CommitResponse_EntryResponse& entry);
 
+  // EntitySpecifics is used as a filter for the GetUpdates message to tell
+  // the server which datatypes to send back.  This adds a datatype so that
+  // it's included in the filter.
+  static void AddToEntitySpecificDatatypesFilter(syncable::ModelType datatype,
+      sync_pb::EntitySpecifics* filter);
+
  private:
   SyncerProtoUtil() {}
 
diff --git a/chrome/browser/sync/engine/syncer_thread_unittest.cc b/chrome/browser/sync/engine/syncer_thread_unittest.cc
index d9da657..5487e07 100644
--- a/chrome/browser/sync/engine/syncer_thread_unittest.cc
+++ b/chrome/browser/sync/engine/syncer_thread_unittest.cc
@@ -43,6 +43,9 @@ class SyncerThreadWithSyncerTest : public testing::Test,
             &SyncerThreadWithSyncerTest::HandleSyncerEvent));
     allstatus_->WatchSyncerThread(syncer_thread_);
     syncer_thread_->SetConnected(true);
+    std::bitset<syncable::MODEL_TYPE_COUNT> expected_types;
+    expected_types[syncable::BOOKMARKS] = true;
+    connection_->ExpectGetUpdatesRequestTypes(expected_types);
   }
   virtual void TearDown() {
     syncer_thread_ = NULL;
diff --git a/chrome/browser/sync/engine/syncer_unittest.cc b/chrome/browser/sync/engine/syncer_unittest.cc
index 0d78832..a2956c5 100755
--- a/chrome/browser/sync/engine/syncer_unittest.cc
+++ b/chrome/browser/sync/engine/syncer_unittest.cc
@@ -129,8 +129,12 @@ class SyncerTest : public testing::Test,
 
   virtual void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) {
     // We're just testing the sync engine here, so we shunt everything to
-    // the SyncerThread.
-    (*out)[syncable::BOOKMARKS] = GROUP_PASSIVE;
+    // the SyncerThread.  Datatypes which aren't enabled aren't in the map.
+    for (int i = 0; i < syncable::MODEL_TYPE_COUNT; ++i) {
+      if (enabled_datatypes_[i]) {
+        (*out)[syncable::ModelTypeFromInt(i)] = GROUP_PASSIVE;
+      }
+    }
   }
 
   void HandleSyncerEvent(SyncerEvent event) {
@@ -168,6 +172,7 @@ class SyncerTest : public testing::Test,
 
     mock_server_.reset(
         new MockConnectionManager(syncdb_.manager(), syncdb_.name()));
+    EnableDatatype(syncable::BOOKMARKS);
     worker_ = new ModelSafeWorker();
     context_.reset(new SyncSessionContext(mock_server_.get(), syncdb_.manager(),
        this));
@@ -380,6 +385,16 @@ class SyncerTest : public testing::Test,
     return entry.Get(META_HANDLE);
   }
 
+  void EnableDatatype(syncable::ModelType model_type) {
+    enabled_datatypes_[model_type] = true;
+    mock_server_->ExpectGetUpdatesRequestTypes(enabled_datatypes_);
+  }
+
+  void DisableDatatype(syncable::ModelType model_type) {
+    enabled_datatypes_[model_type] = false;
+    mock_server_->ExpectGetUpdatesRequestTypes(enabled_datatypes_);
+  }
+
   // Some ids to aid tests. Only the root one's value is specific. The rest
   // are named for test clarity.
   // TODO(chron): Get rid of these inbuilt IDs. They only make it
@@ -403,6 +418,8 @@ class SyncerTest : public testing::Test,
   base::TimeDelta last_long_poll_interval_received_;
   scoped_refptr<ModelSafeWorker> worker_;
 
+  std::bitset<syncable::MODEL_TYPE_COUNT> enabled_datatypes_;
+
   DISALLOW_COPY_AND_ASSIGN(SyncerTest);
 };
 
@@ -713,6 +730,7 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
       ASSERT_TRUE(parent.good());
       parent.Put(syncable::IS_UNSYNCED, true);
       parent.Put(syncable::IS_DIR, true);
+      parent.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
       parent.Put(syncable::IS_DEL, true);
       parent.Put(syncable::ID, ids_.FromNumber(103));
       parent.Put(syncable::BASE_VERSION, 1);
@@ -722,6 +740,7 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
       ASSERT_TRUE(child.good());
       child.Put(syncable::IS_UNSYNCED, true);
       child.Put(syncable::IS_DIR, true);
+      child.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
       child.Put(syncable::IS_DEL, true);
       child.Put(syncable::ID, ids_.FromNumber(104));
       child.Put(syncable::BASE_VERSION, 1);
@@ -733,6 +752,7 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
       grandchild.Put(syncable::ID, ids_.FromNumber(105));
       grandchild.Put(syncable::IS_DEL, true);
       grandchild.Put(syncable::IS_DIR, false);
+      grandchild.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
       grandchild.Put(syncable::BASE_VERSION, 1);
       grandchild.Put(syncable::MTIME, now_minus_2h);
     }
@@ -838,10 +858,13 @@ TEST_F(SyncerTest, TestCommitListOrderingCounterexample) {
     ASSERT_TRUE(child1.good());
     child1.Put(syncable::IS_UNSYNCED, true);
     child1.Put(syncable::ID, child_id_);
+    child1.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     MutableEntry child2(&wtrans, syncable::CREATE, parent_id_, "2");
     ASSERT_TRUE(child2.good());
     child2.Put(syncable::IS_UNSYNCED, true);
+    child2.Put(syncable::SPECIFICS, DefaultBookmarkSpecifics());
     child2.Put(syncable::ID, child2_id);
+
     parent.Put(syncable::BASE_VERSION, 1);
     child1.Put(syncable::BASE_VERSION, 1);
     child2.Put(syncable::BASE_VERSION, 1);
@@ -3366,6 +3389,7 @@ TEST_F(SyncerTest, UpdateFlipsTheFolderBit) {
     local_deleted.Put(IS_DEL, true);
     local_deleted.Put(IS_DIR, true);
     local_deleted.Put(IS_UNSYNCED, true);
+    local_deleted.Put(SPECIFICS, DefaultBookmarkSpecifics());
   }
 
   // Server update: entry-type object (not a container), revision 10.
@@ -4118,6 +4142,36 @@ TEST_F(SyncerTest, UniqueServerTagUpdates) {
   }
 }
 
+TEST_F(SyncerTest, GetUpdatesSetsRequestedTypes) {
+  // The expectations of this test happen in the MockConnectionManager's
+  // GetUpdates handler.  EnableDatatype sets the expectation value from our
+  // set of enabled/disabled datatypes.
+  EnableDatatype(syncable::BOOKMARKS);
+  syncer_->SyncShare(this);
+  EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
+
+  EnableDatatype(syncable::AUTOFILL);
+  syncer_->SyncShare(this);
+  EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
+
+  EnableDatatype(syncable::PREFERENCES);
+  syncer_->SyncShare(this);
+  EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
+
+  DisableDatatype(syncable::BOOKMARKS);
+  syncer_->SyncShare(this);
+  EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
+
+  DisableDatatype(syncable::AUTOFILL);
+  syncer_->SyncShare(this);
+  EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
+
+  DisableDatatype(syncable::PREFERENCES);
+  EnableDatatype(syncable::AUTOFILL);
+  syncer_->SyncShare(this);
+  EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
+}
+
 class SyncerPositionUpdateTest : public SyncerTest {
  public:
   SyncerPositionUpdateTest() : next_update_id_(1), next_revision_(1) {}
diff --git a/chrome/browser/sync/engine/syncer_util.cc b/chrome/browser/sync/engine/syncer_util.cc
index d42305f..6ade37b 100755
--- a/chrome/browser/sync/engine/syncer_util.cc
+++ b/chrome/browser/sync/engine/syncer_util.cc
@@ -838,19 +838,4 @@ syncable::Id SyncerUtil::ComputePrevIdFromServerPosition(
   return closest_sibling;
 }
 
-syncable::ModelType SyncerUtil::GetModelType(
-    const SyncEntity& entry) {
-
-  const bool is_bookmark =
-      (entry.has_specifics() &&
-       entry.specifics().HasExtension(sync_pb::bookmark)) ||
-       entry.has_bookmarkdata();
-
-  if (is_bookmark) {
-    return syncable::BOOKMARKS;
-  }
-
-  return syncable::UNSPECIFIED;
-}
-
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/engine/syncer_util.h b/chrome/browser/sync/engine/syncer_util.h
index eaba33e..134f7b7 100755
--- a/chrome/browser/sync/engine/syncer_util.h
+++ b/chrome/browser/sync/engine/syncer_util.h
@@ -152,8 +152,6 @@ class SyncerUtil {
   // if they match.  For an up-to-date item, this should be the case.
   static bool ServerAndLocalOrdersMatch(syncable::Entry* entry);
 
-  static syncable::ModelType GetModelType(const SyncEntity& entry);
-
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(SyncerUtil);
 };
diff --git a/chrome/browser/sync/engine/syncproto.h b/chrome/browser/sync/engine/syncproto.h
index c34a709..b412bea 100644
--- a/chrome/browser/sync/engine/syncproto.h
+++ b/chrome/browser/sync/engine/syncproto.h
@@ -53,29 +53,8 @@ class SyncEntity : public IdWrapper<sync_pb::SyncEntity> {
             (has_bookmarkdata() && bookmarkdata().bookmark_folder()));
   }
 
-  // Note: keep this consistent with GetModelType in syncable.cc!
   syncable::ModelType GetModelType() const {
-    DCHECK(!id().IsRoot());  // Root shouldn't ever go over the wire.
-
-    if (deleted())
-      return syncable::UNSPECIFIED;
-
-    if (specifics().HasExtension(sync_pb::bookmark) || has_bookmarkdata())
-      return syncable::BOOKMARKS;
-
-    if (specifics().HasExtension(sync_pb::preference))
-      return syncable::PREFERENCES;
-
-    // Loose check for server-created top-level folders that aren't
-    // bound to a particular model type.
-    if (!server_defined_unique_tag().empty() && IsFolder())
-      return syncable::TOP_LEVEL_FOLDER;
-
-    // This is an item of a datatype we can't understand. Maybe it's
-    // from the future?  Either we mis-encoded the object, or the
-    // server sent us entries it shouldn't have.
-    NOTREACHED() << "Unknown datatype in sync proto.";
-    return syncable::UNSPECIFIED;
+    return syncable::GetModelType(*this);
   }
 };
 
diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index c154137..7206b1c 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -31,12 +31,6 @@ SyncBackendHost::SyncBackendHost(SyncFrontend* frontend,
       sync_data_folder_path_(profile_path.Append(kSyncDataFolderName)),
       last_auth_error_(AuthError::None()) {
 
-  // Init our registrar state.
-  for (int i = 0; i < syncable::MODEL_TYPE_COUNT; i++) {
-    syncable::ModelType t(syncable::ModelTypeFromInt(i));
-    registrar_.routing_info[t] = GROUP_PASSIVE;  // Init to syncing 0 types.
-  }
-
   core_ = new Core(this);
 }
 
@@ -62,6 +56,12 @@ void SyncBackendHost::Initialize(
   // need to update routing_info_.
   registrar_.workers[GROUP_UI] = new UIModelWorker(frontend_loop_);
 
+  // Any datatypes that we want the syncer to pull down must
+  // be in the routing_info map.  We set them to group passive, meaning that
+  // updates will be applied, but not dispatched to the UI thread yet.
+  // TODO(ncarter): Wire this up to some external control.
+  registrar_.routing_info[syncable::BOOKMARKS] = GROUP_PASSIVE;
+
   core_thread_.message_loop()->PostTask(FROM_HERE,
       NewRunnableMethod(core_.get(), &SyncBackendHost::Core::DoInitialize,
                         sync_service_url, true,
diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h
index 731a172..c7c145b 100644
--- a/chrome/browser/sync/glue/sync_backend_host.h
+++ b/chrome/browser/sync/glue/sync_backend_host.h
@@ -137,6 +137,7 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar {
     if (!core_thread_.Start())
       return;
     registrar_.workers[GROUP_UI] = new UIModelWorker(frontend_loop_);
+    registrar_.routing_info[syncable::BOOKMARKS] = GROUP_PASSIVE;
 
     core_thread_.message_loop()->PostTask(FROM_HERE,
         NewRunnableMethod(core_.get(),
diff --git a/chrome/browser/sync/protocol/sync.proto b/chrome/browser/sync/protocol/sync.proto
index 0b25a23..2ca8b4c 100755
--- a/chrome/browser/sync/protocol/sync.proto
+++ b/chrome/browser/sync/protocol/sync.proto
@@ -282,6 +282,20 @@ message GetUpdatesMessage {
 
   // Indicates the reason for the GetUpdatesMessage.
   optional GetUpdatesCallerInfo caller_info = 2;
+
+  // Indicates whether related folders should be fetched.
+  optional bool fetch_folders = 3 [default = true];
+
+  // The presence of an individual EntitySpecifics extension indicates that the
+  // client requests sync object types associated with that extension.  This
+  // determination depends only on the presence of the extension field, not its
+  // contents -- thus clients should send empty extension messages.  For 
+  // backwards compatibility only bookmark objects will be sent to the client 
+  // should requested_types not be present.
+  //
+  // requested_types may contain multiple EntitySpecifics extensions -- in this
+  // event, the server will return items of all the indicated types.
+  optional EntitySpecifics requested_types = 4;
 };
 
 message AuthenticateMessage {
diff --git a/chrome/browser/sync/sessions/sync_session.cc b/chrome/browser/sync/sessions/sync_session.cc
index 5a0a3e8..27e2235 100644
--- a/chrome/browser/sync/sessions/sync_session.cc
+++ b/chrome/browser/sync/sessions/sync_session.cc
@@ -15,9 +15,10 @@ SyncSession::SyncSession(SyncSessionContext* context, Delegate* delegate)
       delegate_(delegate),
       auth_failure_occurred_(false) {
 
-  std::vector<ModelSafeWorker*>* s =
-      const_cast<std::vector<ModelSafeWorker*>* >(&workers_);
-  context_->registrar()->GetWorkers(s);
+  context_->registrar()->GetWorkers(
+      const_cast<std::vector<ModelSafeWorker*>*>(&workers_));
+  context_->registrar()->GetModelSafeRoutingInfo(
+      const_cast<ModelSafeRoutingInfo*>(&routing_info_));
 
   // TODO(tim): Use ModelSafeRoutingInfo to silo parts of the session status by
   // ModelSafeGroup;
diff --git a/chrome/browser/sync/sessions/sync_session.h b/chrome/browser/sync/sessions/sync_session.h
index f84db4f..196d256 100644
--- a/chrome/browser/sync/sessions/sync_session.h
+++ b/chrome/browser/sync/sessions/sync_session.h
@@ -103,6 +103,7 @@ class SyncSession {
   }
 
   const std::vector<ModelSafeWorker*>& workers() const { return workers_; }
+  const ModelSafeRoutingInfo& routing_info() const { return routing_info_; }
 
  private:
   // Extend the encapsulation boundary to utilities for internal member
@@ -135,6 +136,11 @@ class SyncSession {
   // The set of active ModelSafeWorkers for the duration of this session.
   const std::vector<ModelSafeWorker*> workers_;
 
+  // The routing info for the duration of this session, dictating which
+  // datatypes should be synced and which workers should be used when working
+  // on those datatypes.
+  const ModelSafeRoutingInfo routing_info_;
+
   // Used to fail read/write operations on this SyncSession that don't obey the
   // currently active ModelSafeWorker contract.
   bool group_restriction_in_effect_;
diff --git a/chrome/browser/sync/syncable/model_type.cc b/chrome/browser/sync/syncable/model_type.cc
new file mode 100755
index 0000000..10ff451
--- /dev/null
+++ b/chrome/browser/sync/syncable/model_type.cc
@@ -0,0 +1,66 @@
+// Copyright (c) 2010 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/syncable/model_type.h"
+
+#include "chrome/browser/sync/engine/syncproto.h"
+#include "chrome/browser/sync/protocol/autofill_specifics.pb.h"
+#include "chrome/browser/sync/protocol/bookmark_specifics.pb.h"
+#include "chrome/browser/sync/protocol/preference_specifics.pb.h"
+#include "chrome/browser/sync/protocol/sync.pb.h"
+
+namespace syncable {
+
+void AddDefaultExtensionValue(syncable::ModelType datatype,
+                              sync_pb::EntitySpecifics* specifics) {
+  switch (datatype) {
+    case BOOKMARKS:
+      specifics->MutableExtension(sync_pb::bookmark);
+      break;
+    case PREFERENCES:
+      specifics->MutableExtension(sync_pb::preference);
+      break;
+    case AUTOFILL:
+      specifics->MutableExtension(sync_pb::autofill);
+      break;
+    default:
+      NOTREACHED() << "No known extension for model type.";
+  }
+}
+
+// Note: keep this consistent with GetModelType in syncable.cc!
+ModelType GetModelType(const sync_pb::SyncEntity& sync_pb_entity) {
+  const browser_sync::SyncEntity& sync_entity =
+      static_cast<const browser_sync::SyncEntity&>(sync_pb_entity);
+  DCHECK(!sync_entity.id().IsRoot());  // Root shouldn't ever go over the wire.
+
+  if (sync_entity.deleted())
+    return syncable::UNSPECIFIED;
+
+  if (sync_entity.specifics().HasExtension(sync_pb::bookmark) ||
+      sync_entity.has_bookmarkdata()) {
+    return syncable::BOOKMARKS;
+  }
+
+  if (sync_entity.specifics().HasExtension(sync_pb::preference))
+    return syncable::PREFERENCES;
+
+  if (sync_entity.specifics().HasExtension(sync_pb::autofill))
+    return syncable::AUTOFILL;
+
+  // Loose check for server-created top-level folders that aren't
+  // bound to a particular model type.
+  if (!sync_entity.server_defined_unique_tag().empty() &&
+      sync_entity.IsFolder()) {
+    return syncable::TOP_LEVEL_FOLDER;
+  }
+
+  // This is an item of a datatype we can't understand. Maybe it's
+  // from the future?  Either we mis-encoded the object, or the
+  // server sent us entries it shouldn't have.
+  NOTREACHED() << "Unknown datatype in sync proto.";
+  return syncable::UNSPECIFIED;
+}
+
+}  // namespace syncable
diff --git a/chrome/browser/sync/syncable/model_type.h b/chrome/browser/sync/syncable/model_type.h
index 0ec7211..e5dd648 100644
--- a/chrome/browser/sync/syncable/model_type.h
+++ b/chrome/browser/sync/syncable/model_type.h
@@ -11,6 +11,11 @@
 
 #include "base/logging.h"
 
+namespace sync_pb {
+class EntitySpecifics;
+class SyncEntity;
+}
+
 namespace syncable {
 
 enum ModelType {
@@ -22,6 +27,7 @@ enum ModelType {
                      // datatypes (e.g. the "Google Chrome" folder).
   BOOKMARKS,         // A bookmark folder or a bookmark URL object.
   PREFERENCES,       // A preference folder or a preference object.
+  AUTOFILL,          // An autofill folder or an autofill object.
   MODEL_TYPE_COUNT,
 };
 
@@ -31,6 +37,15 @@ inline ModelType ModelTypeFromInt(int i) {
   return static_cast<ModelType>(i);
 }
 
+void AddDefaultExtensionValue(syncable::ModelType datatype,
+                              sync_pb::EntitySpecifics* specifics);
+
+// Extract the model type of a SyncEntity protocol buffer.  ModelType is a
+// local concept: the enum is not in the protocol.  The SyncEntity's ModelType
+// is inferred from the presence of particular datatype extensions in the
+// entity specifics.
+ModelType GetModelType(const sync_pb::SyncEntity& sync_entity);
+
 }  // namespace syncable
 
 #endif  // CHROME_BROWSER_SYNC_SYNCABLE_MODEL_TYPE_H_
diff --git a/chrome/browser/sync/syncable/syncable.cc b/chrome/browser/sync/syncable/syncable.cc
index f656e05..ff0ab77 100755
--- a/chrome/browser/sync/syncable/syncable.cc
+++ b/chrome/browser/sync/syncable/syncable.cc
@@ -35,6 +35,7 @@
 #include "base/time.h"
 #include "chrome/browser/sync/engine/syncer.h"
 #include "chrome/browser/sync/engine/syncer_util.h"
+#include "chrome/browser/sync/protocol/autofill_specifics.pb.h"
 #include "chrome/browser/sync/protocol/bookmark_specifics.pb.h"
 #include "chrome/browser/sync/protocol/preference_specifics.pb.h"
 #include "chrome/browser/sync/protocol/service_constants.h"
@@ -1037,6 +1038,10 @@ void Entry::DeleteAllExtendedAttributes(WriteTransaction *trans) {
 syncable::ModelType Entry::GetServerModelType() const {
   if (Get(SERVER_SPECIFICS).HasExtension(sync_pb::bookmark))
     return BOOKMARKS;
+  if (Get(SERVER_SPECIFICS).HasExtension(sync_pb::preference))
+    return PREFERENCES;
+  if (Get(SERVER_SPECIFICS).HasExtension(sync_pb::autofill))
+    return AUTOFILL;
   if (IsRoot())
     return TOP_LEVEL_FOLDER;
   // Loose check for server-created top-level folders that aren't
@@ -1062,6 +1067,8 @@ syncable::ModelType Entry::GetModelType() const {
     return BOOKMARKS;
   if (Get(SPECIFICS).HasExtension(sync_pb::preference))
     return PREFERENCES;
+  if (Get(SPECIFICS).HasExtension(sync_pb::autofill))
+    return AUTOFILL;
   if (IsRoot())
     return TOP_LEVEL_FOLDER;
   // Loose check for server-created top-level folders that aren't
diff --git a/chrome/browser/sync/syncable/syncable_unittest.cc b/chrome/browser/sync/syncable/syncable_unittest.cc
index d30296a..d94815a 100755
--- a/chrome/browser/sync/syncable/syncable_unittest.cc
+++ b/chrome/browser/sync/syncable/syncable_unittest.cc
@@ -12,13 +12,6 @@
 #include <limits>
 #include <string>
 
-// TODO(ncarter): Winnow down the OS-specific includes from the test
-// file.
-#if defined(OS_WIN)
-#include <tchar.h>
-#include <process.h>
-#endif  // defined(OS_WIN)
-
 #if !defined(OS_WIN)
 #define MAX_PATH PATH_MAX
 #include <ostream>
@@ -35,6 +28,7 @@
 #include "base/platform_thread.h"
 #include "base/scoped_ptr.h"
 #include "base/scoped_temp_dir.h"
+#include "chrome/browser/sync/engine/syncproto.h"
 #include "chrome/browser/sync/protocol/bookmark_specifics.pb.h"
 #include "chrome/browser/sync/syncable/directory_backing_store.h"
 #include "chrome/browser/sync/syncable/directory_manager.h"
@@ -866,6 +860,88 @@ TEST_F(SyncableDirectoryTest, TestSaveChangesFailure) {
   }
 }
 
+// Create items of each model type, and check that GetModelType and
+// GetServerModelType return the right value.
+TEST_F(SyncableDirectoryTest, GetModelType) {
+  TestIdFactory id_factory;
+  for (int i = 0; i < MODEL_TYPE_COUNT; ++i) {
+    ModelType datatype = ModelTypeFromInt(i);
+    SCOPED_TRACE(testing::Message("Testing model type ") << datatype);
+    switch (datatype) {
+      case UNSPECIFIED:
+      case TOP_LEVEL_FOLDER:
+        continue;  // Datatype isn't a function of Specifics.
+      default:
+        break;
+    }
+    sync_pb::EntitySpecifics specifics;
+    AddDefaultExtensionValue(datatype, &specifics);
+
+    WriteTransaction trans(dir_.get(), UNITTEST, __FILE__, __LINE__);
+
+    MutableEntry folder(&trans, CREATE, trans.root_id(), "Folder");
+    ASSERT_TRUE(folder.good());
+    folder.Put(ID, id_factory.NewServerId());
+    folder.Put(SPECIFICS, specifics);
+    folder.Put(BASE_VERSION, 1);
+    folder.Put(IS_DIR, true);
+    folder.Put(IS_DEL, false);
+    ASSERT_EQ(datatype, folder.GetModelType());
+
+    MutableEntry item(&trans, CREATE, trans.root_id(), "Item");
+    ASSERT_TRUE(item.good());
+    item.Put(ID, id_factory.NewServerId());
+    item.Put(SPECIFICS, specifics);
+    item.Put(BASE_VERSION, 1);
+    item.Put(IS_DIR, false);
+    item.Put(IS_DEL, false);
+    ASSERT_EQ(datatype, item.GetModelType());
+
+    // It's critical that deletion records retain their datatype, so that
+    // they can be dispatched to the appropriate change processor.
+    MutableEntry deleted_item(&trans, CREATE, trans.root_id(), "Deleted Item");
+    ASSERT_TRUE(item.good());
+    deleted_item.Put(ID, id_factory.NewServerId());
+    deleted_item.Put(SPECIFICS, specifics);
+    deleted_item.Put(BASE_VERSION, 1);
+    deleted_item.Put(IS_DIR, false);
+    deleted_item.Put(IS_DEL, true);
+    ASSERT_EQ(datatype, deleted_item.GetModelType());
+
+    MutableEntry server_folder(&trans, CREATE_NEW_UPDATE_ITEM,
+        id_factory.NewServerId());
+    ASSERT_TRUE(server_folder.good());
+    server_folder.Put(SERVER_SPECIFICS, specifics);
+    server_folder.Put(BASE_VERSION, 1);
+    server_folder.Put(SERVER_IS_DIR, true);
+    server_folder.Put(SERVER_IS_DEL, false);
+    ASSERT_EQ(datatype, server_folder.GetServerModelType());
+
+    MutableEntry server_item(&trans, CREATE_NEW_UPDATE_ITEM,
+        id_factory.NewServerId());
+    ASSERT_TRUE(server_item.good());
+    server_item.Put(SERVER_SPECIFICS, specifics);
+    server_item.Put(BASE_VERSION, 1);
+    server_item.Put(SERVER_IS_DIR, false);
+    server_item.Put(SERVER_IS_DEL, false);
+    ASSERT_EQ(datatype, server_item.GetServerModelType());
+
+    browser_sync::SyncEntity folder_entity;
+    folder_entity.set_id(id_factory.NewServerId());
+    folder_entity.set_deleted(false);
+    folder_entity.set_folder(true);
+    folder_entity.mutable_specifics()->CopyFrom(specifics);
+    ASSERT_EQ(datatype, folder_entity.GetModelType());
+
+    browser_sync::SyncEntity item_entity;
+    item_entity.set_id(id_factory.NewServerId());
+    item_entity.set_deleted(false);
+    item_entity.set_folder(false);
+    item_entity.mutable_specifics()->CopyFrom(specifics);
+    ASSERT_EQ(datatype, item_entity.GetModelType());
+  }
+}
+
 void SyncableDirectoryTest::ValidateEntry(BaseTransaction* trans, int64 id,
     bool check_name, string name, int64 base_version, int64 server_version,
     bool is_del) {