sync: Implement per-type commit interface

sync/engine/sync_directory_commit_contributor.h

Index: sync/engine/sync_directory_commit_contributor.h
diff --git a/sync/engine/sync_directory_commit_contributor.h b/sync/engine/sync_directory_commit_contributor.h
new file mode 100644
index 0000000000000000000000000000000000000000..527644cdaf2e57d0973ba6b662d71989eea06afe
--- /dev/null
+++ b/sync/engine/sync_directory_commit_contributor.h
@@ -0,0 +1,38 @@
+// Copyright (c) 2013 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.
+
+#ifndef SYNC_ENGINE_SYNC_DIRECTORY_COMMIT_CONTRIBUTOR_H_
+#define SYNC_ENGINE_SYNC_DIRECTORY_COMMIT_CONTRIBUTOR_H_
+
+#include <map>
+
+#include "sync/internal_api/public/base/model_type.h"
+
+namespace syncer {
+
+class SyncDirectoryCommitContribution;
+
+namespace syncable {
+class Directory;
+}
+
+class SyncDirectoryCommitContributor {
+ public:
+  SyncDirectoryCommitContributor(syncable::Directory* dir, ModelType type);
+  ~SyncDirectoryCommitContributor();
+
+  SyncDirectoryCommitContribution* GetContribution(size_t max_entries);
+
+ private:
+  syncable::Directory* dir_;
+  ModelType type_;
+};
+
+// TODO(rlarocque): Find a better place for this definition.
+typedef std::map<ModelType, SyncDirectoryCommitContributor*>

[Nicolas Zea, 2013/10/09 00:17:39]

Would using linked_ptrs make sense here? (So you don't need to have
STLValueDeleters)

[rlarocque, 2013/10/09 20:00:19]

I think I prefer the STLValueDeleters.  They're not perfect, but they're better
than reference counting.

[tim (not reviewing), 2013/10/10 17:22:36]

There's been some debate about this on chromium-dev.  For maps in particular
there was no consensus reached because linked_ptr offers a few advantages (and
note, afaik, there's no 'ScopedVector' analog, although if you wanted to
investigate it I'm sure you could find support :P). 

I think if you're declaring a truly internal type (in a private: section, only
possibly instantiated by one class ever) and opt for using an STLValueDeleter,
that makes sense, but if you're declaring a generic container and expecting
people who use it to remember to add one of these too, and ensure destruction
ordering, it seems more error prone.  

Also, could any code ever (now-or-future) remove from this map?  The Deleter
won't help this case.  

[rlarocque, 2013/10/10 19:05:00]

Yeah, I re-read that thread when I saw this comment.

I think this use case are simple enough that we can get away with using the
STLValueDeleter.  Eventually, the contributors will be owned by the
SyncSessionContext and modified only by a small handful of its member functions.
 It should be possible to implement it correctly, and we can write unit tests
that will break (under valgrind) if we mess it up.  

+    CommitContributorMap;
+
+}  // namespace
+
+#endif  // SYNC_ENGINE_SYNC_DIRECTORY_COMMIT_CONTRIBUTOR_H_