Adding the Copresence RpcHandler and HttpPost helper.

components/copresence/handlers/directive_handler.h

Index: components/copresence/handlers/directive_handler.h
diff --git a/components/copresence/handlers/directive_handler.h b/components/copresence/handlers/directive_handler.h
index 8a415f1dbb77cda9b70dfce35cd6ad2550d1e822..457d74af9daefd6b77c56e9c9682f331a5ec7e11 100644
--- a/components/copresence/handlers/directive_handler.h
+++ b/components/copresence/handlers/directive_handler.h
@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef COMPONENTS_COPRESENCE_HANDLERS_DIRECTIVE_HANDLER_
-#define COMPONENTS_COPRESENCE_HANDLERS_DIRECTIVE_HANDLER_
+#ifndef COMPONENTS_COPRESENCE_HANDLERS_DIRECTIVE_HANDLER_H_
+#define COMPONENTS_COPRESENCE_HANDLERS_DIRECTIVE_HANDLER_H_
 
 #include <string>
 
@@ -18,18 +18,23 @@ namespace copresence {
 class AudioDirectiveHandler;
 class Directive;
 
-// The directive handler manages transmit and receive directives given to it
-// by the client.
+// The directive handler manages transmit and receive directives
+// given to it by the client.
 class DirectiveHandler {
  public:
-  DirectiveHandler(const AudioRecorder::DecodeSamplesCallback& decode_cb,
-                   const AudioDirectiveList::EncodeTokenCallback& encode_cb);
-  ~DirectiveHandler();
+  DirectiveHandler();
+  virtual ~DirectiveHandler();
+
+  // Initialize the |audio_handler_| with the appropriate callbacks.
+  // This function must be called before any others.

[willchan no longer on Chromium, 2014/08/08 01:05:59]

If it's not safe to use this object before Initialize() is called, why not
combine this code with the constructor?

[Charlie, 2014/08/08 21:57:52]

We explicitly don't want these to be constructor parameters so that subclassing
is simplified.

[willchan no longer on Chromium, 2014/08/08 22:53:34]

Can you explain further? Can't subclasses just call the parent constructor?

[Charlie, 2014/08/08 23:38:30]

Yes, but this is easier than passing in a bunch of empty callbacks.

In the client we already have a Create() method instead of a public constructor,
which would take the place of Initialize(). Added a TODO to do that here too.

+  virtual void Initialize(
+      const AudioRecorder::DecodeSamplesCallback& decode_cb,
+      const AudioDirectiveList::EncodeTokenCallback& encode_cb);
 
   // Adds a directive to handle.
-  void AddDirective(const copresence::Directive& directive);
+  virtual void AddDirective(const copresence::Directive& directive);

[willchan no longer on Chromium, 2014/08/08 01:05:58]

Why are these virtual?

[Charlie, 2014/08/08 21:57:52]

So the FakeDirectiveHandler can override them. See rpc_handler_unittest.cc.

[willchan no longer on Chromium, 2014/08/08 22:53:33]

Got it, that makes sense.

My personal suggestion, which I admit is very debatable, is if you're going to
override for testing purposes, to make the base class an abstract interface.
Otherwise, it becomes a bit difficult to understand how your test subclass will
interact with existing behavior in the production class. In other words:
1) DirectiveHandler - abstract interface
2) DirectiveHandlerImpl - has all the production implementation
3) MockDirectiveHandler - lets you mock out the DirectiveHandler dependency

[Charlie, 2014/08/08 23:38:30]

Not a bad idea. This is also what we do in the client. Added a TODO.

   // Removes any directives associated with the given operation id.
-  void RemoveDirectives(const std::string& op_id);
+  virtual void RemoveDirectives(const std::string& op_id);
 
  private:
   scoped_ptr<AudioDirectiveHandler> audio_handler_;
@@ -39,4 +44,4 @@ class DirectiveHandler {
 
 }  // namespace copresence
 
-#endif  // COMPONENTS_COPRESENCE_HANDLERS_DIRECTIVE_HANDLER_
+#endif  // COMPONENTS_COPRESENCE_HANDLERS_DIRECTIVE_HANDLER_H_