Using API key specified from js

components/copresence/rpc/http_post.h

Index: components/copresence/rpc/http_post.h
diff --git a/components/copresence/rpc/http_post.h b/components/copresence/rpc/http_post.h
index c4c6e7f46943f137869f207f884c652daeabe26f..0df9585239fc821a5d149da0f463a788e54df289 100644
--- a/components/copresence/rpc/http_post.h
+++ b/components/copresence/rpc/http_post.h
@@ -11,6 +11,7 @@
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "net/url_request/url_fetcher_delegate.h"
+#include "url/gurl.h"
 
 namespace google {
 namespace protobuf {
@@ -36,33 +37,44 @@ class HttpPost : public net::URLFetcherDelegate {
   typedef base::Callback<void(int, const std::string&)>
       ResponseCallback;
 
-  // An id for testing url fetching.
-  static const int kUrlFetcherId = 1;
-
-  // The query parameter to send Apiary tracing tokens.
-  static const char kTracingTokenField[];
-
-  // Create a request to the Copresence server. |url_context_getter|
-  // is owned by the caller, and the context it provides must be available
-  // until the request completes.
+  // Create a request to the Copresence server.
+  // |url_context_getter| is owned by the caller,
+  // and the context it provides must be available until the request completes.
   HttpPost(net::URLRequestContextGetter* url_context_getter,
            const std::string& server_host,
-           const std::string& rpc_name,
-           const std::string& tracing_token,

[rkc, 2014/08/13 21:45:31]

Why is this taken out of the constructor?

[Charlie, 2014/08/13 22:53:37]

All parameters to the constructor are now required. If clients want to add a
tracing token, they should use a separate mutator.

The request proto has been taken out as part of the refactoring, at xiyuan's
suggestion. I can put it back in, but I don't think it's needed.

[rkc, 2014/08/13 23:11:36]

I am not happy about using mutators instead of passing arguments through the
constructor. In general, the internal details of a class should be abstracted
out and anything it needs for its operation should be given to the constructor.

In case this class decides to not save the API key but rather to simply setup
the URL Request in the constructor, now any code that is setting the API key
would now be non-functional. The same argument goes for the tracing token.

In fact, a majority of the implementation had to be moved to the Start method
because we actually now need this space between construction and 'real'
construction where a consumer of this class can set these values.

We do an empty string check on both values anyway (the consumer can just pass in
an empty string to the constructor for those values). I guess I am seeing cons
but I am not seeing any pros of having this as a mutator.

[xiyuan, 2014/08/13 23:15:59]

If we decided to add back |request_proto| to ctor, I would prefer to extract
info out of it instead of storing the pointer, e.g. store the serialized request
data.

-           const google::protobuf::MessageLite& request_proto);
+           const std::string& rpc_name);
 
   // HTTP requests are cancelled on delete.
   virtual ~HttpPost();
 
+  // Add an Apiary tracing token to the request.
+  void set_tracing_token(const std::string& tracing_token);
+
+  // Add an API key to the request. If none is specified,
+  // we attempt to use Chrome's key.
+  void set_api_key(const std::string& api_key) { api_key_ = api_key; }
+
   // Send an HttpPost request.
-  void Start(const ResponseCallback& response_callback);
+  void Start(const ResponseCallback& response_callback,
+             const google::protobuf::MessageLite& request_proto);
 
  private:
+  static const int kUrlFetcherId = 1;
+  static const char kApiKeyField[];
+  static const char kTracingTokenField[];
+
+  friend class HttpPostTest;
+
   // Overridden from net::URLFetcherDelegate.
   virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
 
+  net::URLRequestContextGetter* url_context_getter_;
+  GURL url_;
+
   ResponseCallback response_callback_;
+
   scoped_ptr<net::URLFetcher> url_fetcher_;
+  std::string api_key_;
 
   DISALLOW_COPY_AND_ASSIGN(HttpPost);
 };