Sync engine_impl: Return HttpResponse by value, not out param

ServerConnectionManager (and its implementations, plus some related
helper classes) returned an HttpResponse as an output parameter.
Output parameters are generally discouraged, and here in particular,
they made the logic hard to follow since the whole thing went through
many layers.

Before this CL, the methods in question returned a bool that roughly
meant "success", but it was inconsistent (in some layers, an HTTP error
would be counted as success, in others as failure). Now, they return
the HttpResponse instead. Clients now query the "success" they're
interested in from the HttpResponse.

Bug: 951350
Change-Id: Idd7867246506caf069287e4dc34c8b59a1da59fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2465910
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816613}

components/sync/engine_impl/loopback_server/loopback_connection_manager.cc

@@ -13,14 +13,13 @@ LoopbackConnectionManager::LoopbackConnectionManager(
     const base::FilePath& persistent_file)
     : loopback_server_(persistent_file) {}
 
-LoopbackConnectionManager::~LoopbackConnectionManager() {}
+LoopbackConnectionManager::~LoopbackConnectionManager() = default;
 
-bool LoopbackConnectionManager::PostBufferToPath(
+HttpResponse LoopbackConnectionManager::PostBufferToPath(
     const std::string& buffer_in,
     const std::string& path,
     const std::string& access_token,
-    std::string* buffer_out,
-    HttpResponse* http_response) {
+    std::string* buffer_out) {
   buffer_out->clear();
 
   sync_pb::ClientToServerMessage message;
@@ -28,22 +27,23 @@ bool LoopbackConnectionManager::PostBufferToPath(
   DCHECK(parsed) << "Unable to parse the ClientToServerMessage.";
 
   sync_pb::ClientToServerResponse client_to_server_response;
-  http_response->http_status_code =
+  HttpResponse http_response = HttpResponse::Uninitialized();
+  http_response.http_status_code =
       loopback_server_.HandleCommand(message, &client_to_server_response);
 
   if (client_to_server_response.IsInitialized()) {
     *buffer_out = client_to_server_response.SerializeAsString();
   }
 
-  DCHECK_GE(http_response->http_status_code, 0);
+  DCHECK_GE(http_response.http_status_code, 0);
 
-  if (http_response->http_status_code != net::HTTP_OK) {
-    http_response->server_status = HttpResponse::SYNC_SERVER_ERROR;
-    return false;
+  if (http_response.http_status_code != net::HTTP_OK) {
+    http_response.server_status = HttpResponse::SYNC_SERVER_ERROR;
+  } else {
+    http_response.server_status = HttpResponse::SERVER_CONNECTION_OK;
   }
 
-  http_response->server_status = HttpResponse::SERVER_CONNECTION_OK;
-  return true;
+  return http_response;
 }
 
 }  // namespace syncer

components/sync/engine_impl/loopback_server/loopback_connection_manager.h

@@ -23,11 +23,10 @@ class LoopbackConnectionManager : public ServerConnectionManager {
 
  private:
   // Overridden ServerConnectionManager functions.
-  bool PostBufferToPath(const std::string& buffer_in,
-                        const std::string& path,
-                        const std::string& access_token,
-                        std::string* buffer_out,
-                        HttpResponse* http_response) override;
+  HttpResponse PostBufferToPath(const std::string& buffer_in,
+                                const std::string& path,
+                                const std::string& access_token,
+                                std::string* buffer_out) override;
 
   // The loopback server that will handle the requests locally.
   LoopbackServer loopback_server_;

components/sync/engine_impl/net/server_connection_manager.cc

@@ -75,9 +75,13 @@ HttpResponse HttpResponse::ForIoError() {
 // static
 HttpResponse HttpResponse::ForHttpError(int http_status_code) {
   HttpResponse response;
-  response.server_status = http_status_code == net::HTTP_UNAUTHORIZED
-                               ? SYNC_AUTH_ERROR
-                               : SYNC_SERVER_ERROR;
+  if (http_status_code == net::HTTP_OK) {
+    response.server_status = SERVER_CONNECTION_OK;
+  } else if (http_status_code == net::HTTP_UNAUTHORIZED) {
+    response.server_status = SYNC_AUTH_ERROR;
+  } else {
+    response.server_status = SYNC_SERVER_ERROR;
+  }
   response.http_status_code = http_status_code;
   return response;
 }
@@ -140,17 +144,16 @@ void ServerConnectionManager::NotifyStatusChanged() {
         ServerConnectionEvent(server_response_.server_status));
 }
 
-bool ServerConnectionManager::PostBufferWithCachedAuth(
+HttpResponse ServerConnectionManager::PostBufferWithCachedAuth(
     const std::string& buffer_in,
-    std::string* buffer_out,
-    HttpResponse* http_response) {
+    std::string* buffer_out) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   std::string path =
       MakeSyncServerPath(proto_sync_path(), MakeSyncQueryString(client_id_));
-  bool result = PostBufferToPath(buffer_in, path, access_token_, buffer_out,
-                                 http_response);
-  SetServerResponse(*http_response);
-  return result;
+  HttpResponse http_response =
+      PostBufferToPath(buffer_in, path, access_token_, buffer_out);
+  SetServerResponse(http_response);
+  return http_response;
 }
 
 void ServerConnectionManager::AddListener(

components/sync/engine_impl/net/server_connection_manager.h

@@ -64,6 +64,7 @@ struct HttpResponse {
   static HttpResponse Uninitialized();
   static HttpResponse ForNetError(int net_error_code);
   static HttpResponse ForIoError();
+  // TODO(crbug.com/951350): Rename to ForHttpStatusCode.
   static HttpResponse ForHttpError(int http_status_code);
   static HttpResponse ForSuccess();
 
@@ -95,13 +96,10 @@ class ServerConnectionManager {
   ServerConnectionManager();
   virtual ~ServerConnectionManager();
 
-  // POSTS buffer_in and reads a http_response into buffer_out.
-  // Uses our currently set access token in our headers.
-  //
-  // Returns true if executed successfully.
-  bool PostBufferWithCachedAuth(const std::string& buffer_in,
-                                std::string* buffer_out,
-                                HttpResponse* http_response);
+  // POSTs |buffer_in| and reads the body of the response into |buffer_out|.
+  // Uses the currently set access token in the headers.
+  HttpResponse PostBufferWithCachedAuth(const std::string& buffer_in,
+                                        std::string* buffer_out);
 
   void AddListener(ServerConnectionEventListener* listener);
   void RemoveListener(ServerConnectionEventListener* listener);
@@ -145,11 +143,10 @@ class ServerConnectionManager {
 
   // Internal PostBuffer base function which subclasses are expected to
   // implement.
-  virtual bool PostBufferToPath(const std::string& buffer_in,
-                                const std::string& path,
-                                const std::string& access_token,
-                                std::string* buffer_out,
-                                HttpResponse* http_response) = 0;
+  virtual HttpResponse PostBufferToPath(const std::string& buffer_in,
+                                        const std::string& path,
+                                        const std::string& access_token,
+                                        std::string* buffer_out) = 0;
 
   void ClearAccessToken();
 

components/sync/engine_impl/net/sync_server_connection_manager.cc

@@ -49,17 +49,11 @@ class Connection : public CancelationObserver {
              CancelationSignal* cancelation_signal);
   ~Connection() override;
 
-  // TODO(crbug.com/951350): Return the HttpResponse by value. It's not
-  // obvious what the boolean return value means. (True means success or HTTP
-  // error, false means canceled or network error.)
-  bool Init(const std::string& connection_url,
-            int sync_server_port,
-            const std::string& access_token,
-            const std::string& payload,
-            HttpResponse* response);
-  bool ReadBufferResponse(std::string* buffer_out,
-                          HttpResponse* response,
-                          bool require_response);
+  HttpResponse Init(const std::string& connection_url,
+                    int sync_server_port,
+                    const std::string& access_token,
+                    const std::string& payload);
+  bool ReadBufferResponse(std::string* buffer_out, HttpResponse* response);
   bool ReadDownloadResponse(HttpResponse* response, std::string* buffer_out);
 
   // CancelationObserver overrides.
@@ -95,11 +89,10 @@ Connection::Connection(HttpPostProviderFactory* factory,
 
 Connection::~Connection() = default;
 
-bool Connection::Init(const std::string& connection_url,
-                      int sync_server_port,
-                      const std::string& access_token,
-                      const std::string& payload,
-                      HttpResponse* response) {
+HttpResponse Connection::Init(const std::string& connection_url,
+                              int sync_server_port,
+                              const std::string& access_token,
+                              const std::string& payload) {
   post_provider_->SetURL(connection_url.c_str(), sync_server_port);
 
   if (!access_token.empty()) {
@@ -113,55 +106,45 @@ bool Connection::Init(const std::string& connection_url,
                                  payload.data());
 
   // Issue the POST, blocking until it finishes.
-  int net_error_code = 0;
-  int http_status_code = 0;
   if (!cancelation_signal_->TryRegisterHandler(this)) {
     // Return early because cancelation signal was signaled.
     // TODO(crbug.com/951350): Introduce an extra status code for canceled?
-    response->server_status = HttpResponse::CONNECTION_UNAVAILABLE;
-    return false;
+    return HttpResponse::ForNetError(0);
   }
   base::ScopedClosureRunner auto_unregister(base::BindOnce(
       &CancelationSignal::UnregisterHandler,
       base::Unretained(cancelation_signal_), base::Unretained(this)));
 
+  int net_error_code = 0;
+  int http_status_code = 0;
   if (!post_provider_->MakeSynchronousPost(&net_error_code,
                                            &http_status_code)) {
     DCHECK_NE(net_error_code, net::OK);
     DVLOG(1) << "Http POST failed, error returns: " << net_error_code;
-    response->server_status = HttpResponse::CONNECTION_UNAVAILABLE;
-    response->net_error_code = net_error_code;
-    return false;
+    return HttpResponse::ForNetError(net_error_code);
   }
 
   // We got a server response, copy over response codes and content.
-  response->http_status_code = http_status_code;
-  response->content_length =
+  HttpResponse response = HttpResponse::ForHttpError(http_status_code);
+  response.content_length =
       static_cast<int64_t>(post_provider_->GetResponseContentLength());
-  response->payload_length =
+  response.payload_length =
       static_cast<int64_t>(post_provider_->GetResponseContentLength());
-  if (response->http_status_code == net::HTTP_OK)
-    response->server_status = HttpResponse::SERVER_CONNECTION_OK;
-  else if (response->http_status_code == net::HTTP_UNAUTHORIZED)
-    response->server_status = HttpResponse::SYNC_AUTH_ERROR;
-  else
-    response->server_status = HttpResponse::SYNC_SERVER_ERROR;
 
-  // Write the content into our buffer.
+  // Write the content into the buffer.
   buffer_.assign(post_provider_->GetResponseContent(),
                  post_provider_->GetResponseContentLength());
-  return true;
+  return response;
 }
 
 bool Connection::ReadBufferResponse(std::string* buffer_out,
-                                    HttpResponse* response,
-                                    bool require_response) {
+                                    HttpResponse* response) {
   if (net::HTTP_OK != response->http_status_code) {
     response->server_status = HttpResponse::SYNC_SERVER_ERROR;
     return false;
   }
 
-  if (require_response && (1 > response->content_length))
+  if (response->content_length <= 0)
     return false;
 
   const int64_t bytes_read =
@@ -218,23 +201,20 @@ SyncServerConnectionManager::SyncServerConnectionManager(
 
 SyncServerConnectionManager::~SyncServerConnectionManager() = default;
 
-bool SyncServerConnectionManager::PostBufferToPath(
+HttpResponse SyncServerConnectionManager::PostBufferToPath(
     const std::string& buffer_in,
     const std::string& path,
     const std::string& access_token,
-    std::string* buffer_out,
-    HttpResponse* http_response) {
+    std::string* buffer_out) {
   if (access_token.empty()) {
-    http_response->server_status = HttpResponse::SYNC_AUTH_ERROR;
     // Print a log to distinguish this "known failure" from others.
     DVLOG(1) << "ServerConnectionManager forcing SYNC_AUTH_ERROR due to missing"
                 " access token";
-    return false;
+    return HttpResponse::ForHttpError(net::HTTP_UNAUTHORIZED);
   }
 
   if (cancelation_signal_->IsSignalled()) {
-    http_response->server_status = HttpResponse::CONNECTION_UNAVAILABLE;
-    return false;
+    return HttpResponse::ForNetError(0);
   }
 
   auto connection = std::make_unique<Connection>(post_provider_factory_.get(),
@@ -243,21 +223,19 @@ bool SyncServerConnectionManager::PostBufferToPath(
 
   // Note that |post| may be aborted by now, which will just cause Init to fail
   // with CONNECTION_UNAVAILABLE.
-  bool ok = connection->Init(connection_url, sync_server_port_, access_token,
-                             buffer_in, http_response);
+  HttpResponse http_response = connection->Init(
+      connection_url, sync_server_port_, access_token, buffer_in);
 
-  if (http_response->server_status == HttpResponse::SYNC_AUTH_ERROR) {
+  if (http_response.server_status == HttpResponse::SYNC_AUTH_ERROR) {
     ClearAccessToken();
   }
 
-  if (!ok || net::HTTP_OK != http_response->http_status_code)
-    return false;
-
-  if (connection->ReadBufferResponse(buffer_out, http_response, true)) {
-    http_response->server_status = HttpResponse::SERVER_CONNECTION_OK;
-    return true;
+  if (http_response.server_status != HttpResponse::SERVER_CONNECTION_OK) {
+    return http_response;
   }
-  return false;
+
+  connection->ReadBufferResponse(buffer_out, &http_response);
+  return http_response;
 }
 
 }  // namespace syncer

components/sync/engine_impl/net/sync_server_connection_manager.h

@@ -32,11 +32,10 @@ class SyncServerConnectionManager : public ServerConnectionManager {
   ~SyncServerConnectionManager() override;
 
  protected:
-  bool PostBufferToPath(const std::string& buffer_in,
-                        const std::string& path,
-                        const std::string& access_token,
-                        std::string* buffer_out,
-                        HttpResponse* http_response) override;
+  HttpResponse PostBufferToPath(const std::string& buffer_in,
+                                const std::string& path,
+                                const std::string& access_token,
+                                std::string* buffer_out) override;
 
  private:
   FRIEND_TEST_ALL_PREFIXES(SyncServerConnectionManagerTest, VeryEarlyAbortPost);

components/sync/engine_impl/net/sync_server_connection_manager_unittest.cc

@@ -72,12 +72,9 @@ TEST(SyncServerConnectionManagerTest, VeryEarlyAbortPost) {
       "server", 0, true, std::make_unique<BlockingHttpPostFactory>(), &signal);
 
   std::string buffer_out;
-  HttpResponse http_response = HttpResponse::Uninitialized();
+  HttpResponse http_response =
+      server.PostBufferToPath("", "/testpath", "testauth", &buffer_out);
 
-  bool result = server.PostBufferToPath("", "/testpath", "testauth",
-                                        &buffer_out, &http_response);
-
-  EXPECT_FALSE(result);
   EXPECT_EQ(HttpResponse::CONNECTION_UNAVAILABLE, http_response.server_status);
 }
 
@@ -87,14 +84,12 @@ TEST(SyncServerConnectionManagerTest, EarlyAbortPost) {
   SyncServerConnectionManager server(
       "server", 0, true, std::make_unique<BlockingHttpPostFactory>(), &signal);
 
-  std::string buffer_out;
-  HttpResponse http_response = HttpResponse::Uninitialized();
 
   signal.Signal();
-  bool result = server.PostBufferToPath("", "/testpath", "testauth",
-                                        &buffer_out, &http_response);
+  std::string buffer_out;
+  HttpResponse http_response =
+      server.PostBufferToPath("", "/testpath", "testauth", &buffer_out);
 
-  EXPECT_FALSE(result);
   EXPECT_EQ(HttpResponse::CONNECTION_UNAVAILABLE, http_response.server_status);
 }
 
@@ -112,12 +107,9 @@ TEST(SyncServerConnectionManagerTest, AbortPost) {
       TestTimeouts::tiny_timeout());
 
   std::string buffer_out;
-  HttpResponse http_response = HttpResponse::Uninitialized();
+  HttpResponse http_response =
+      server.PostBufferToPath("", "/testpath", "testauth", &buffer_out);
 
-  bool result = server.PostBufferToPath("", "/testpath", "testauth",
-                                        &buffer_out, &http_response);
-
-  EXPECT_FALSE(result);
   EXPECT_EQ(HttpResponse::CONNECTION_UNAVAILABLE, http_response.server_status);
   abort_thread.Stop();
 }
@@ -179,12 +171,9 @@ TEST(SyncServerConnectionManagerTest, FailPostWithTimedOut) {
       std::make_unique<FailingHttpPostFactory>(net::ERR_TIMED_OUT), &signal);
 
   std::string buffer_out;
-  HttpResponse http_response = HttpResponse::Uninitialized();
-
-  bool result = server.PostBufferToPath("", "/testpath", "testauth",
-                                        &buffer_out, &http_response);
+  HttpResponse http_response =
+      server.PostBufferToPath("", "/testpath", "testauth", &buffer_out);
 
-  EXPECT_FALSE(result);
   EXPECT_EQ(HttpResponse::CONNECTION_UNAVAILABLE, http_response.server_status);
 }
 

components/sync/engine_impl/syncer_proto_util.cc

@@ -356,11 +356,11 @@ bool SyncerProtoUtil::PostAndProcessHeaders(ServerConnectionManager* scm,
 
   const base::Time start_time = base::Time::Now();
 
+  // Fills in buffer_out.
   std::string buffer_out;
-  HttpResponse http_response = HttpResponse::Uninitialized();
-
-  // Fills in buffer_out and http_response.
-  if (!scm->PostBufferWithCachedAuth(buffer_in, &buffer_out, &http_response)) {
+  HttpResponse http_response =
+      scm->PostBufferWithCachedAuth(buffer_in, &buffer_out);
+  if (http_response.server_status != HttpResponse::SERVER_CONNECTION_OK) {
     LOG(WARNING) << "Error posting from syncer:" << http_response;
     return false;
   }

components/sync/engine_impl/syncer_proto_util_unittest.cc

@@ -214,27 +214,26 @@ TEST_F(SyncerProtoUtilTest, VerifyEncryptionObsolete) {
 
 class DummyConnectionManager : public ServerConnectionManager {
  public:
-  DummyConnectionManager() : send_error_(false) {}
+  DummyConnectionManager() = default;
 
-  bool PostBufferToPath(const std::string& buffer_in,
-                        const std::string& path,
-                        const std::string& access_token,
-                        std::string* buffer_out,
-                        HttpResponse* response) override {
+  HttpResponse PostBufferToPath(const std::string& buffer_in,
+                                const std::string& path,
+                                const std::string& access_token,
+                                std::string* buffer_out) override {
     if (send_error_) {
-      return false;
+      return HttpResponse::ForIoError();
     }
 
     sync_pb::ClientToServerResponse client_to_server_response;
     client_to_server_response.SerializeToString(buffer_out);
 
-    return true;
+    return HttpResponse::ForSuccess();
   }
 
   void set_send_error(bool send) { send_error_ = send; }
 
  private:
-  bool send_error_;
+  bool send_error_ = false;
 };
 
 TEST_F(SyncerProtoUtilTest, PostAndProcessHeaders) {

components/sync/test/engine/mock_connection_manager.cc

@@ -13,6 +13,7 @@
 #include "components/sync/engine_impl/syncer_proto_util.h"
 #include "components/sync/protocol/bookmark_specifics.pb.h"
 #include "net/base/net_errors.h"
+#include "net/http/http_status_code.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using std::find;
@@ -65,27 +66,29 @@ void MockConnectionManager::SetMidCommitObserver(
   mid_commit_observer_ = observer;
 }
 
-bool MockConnectionManager::PostBufferToPath(const std::string& buffer_in,
-                                             const std::string& path,
-                                             const std::string& access_token,
-                                             std::string* buffer_out,
-                                             HttpResponse* http_response) {
+HttpResponse MockConnectionManager::PostBufferToPath(
+    const std::string& buffer_in,
+    const std::string& path,
+    const std::string& access_token,
+    std::string* buffer_out) {
   ClientToServerMessage post;
   if (!post.ParseFromString(buffer_in)) {
     ADD_FAILURE();
-    return false;
+    // Note: Here and below, ForIoError() is chosen somewhat arbitrarily, since
+    // HttpResponse doesn't have any better-fitting type of error.
+    return HttpResponse::ForIoError();
   }
   if (!post.has_protocol_version()) {
     ADD_FAILURE();
-    return false;
+    return HttpResponse::ForIoError();
   }
   if (!post.has_api_key()) {
     ADD_FAILURE();
-    return false;
+    return HttpResponse::ForIoError();
   }
   if (!post.has_bag_of_chips()) {
     ADD_FAILURE();
-    return false;
+    return HttpResponse::ForIoError();
   }
 
   requests_.push_back(post);
@@ -94,29 +97,25 @@ bool MockConnectionManager::PostBufferToPath(const std::string& buffer_in,
   client_to_server_response.Clear();
 
   if (access_token.empty()) {
-    http_response->server_status = HttpResponse::SYNC_AUTH_ERROR;
-    return false;
+    return HttpResponse::ForNetError(net::HTTP_UNAUTHORIZED);
   }
 
   if (access_token != kValidAccessToken) {
     // Simulate server-side auth failure.
-    http_response->server_status = HttpResponse::SYNC_AUTH_ERROR;
     ClearAccessToken();
+    return HttpResponse::ForNetError(net::HTTP_UNAUTHORIZED);
   }
 
   if (--countdown_to_postbuffer_fail_ == 0) {
     // Fail as countdown hits zero.
-    http_response->server_status = HttpResponse::SYNC_SERVER_ERROR;
-    return false;
+    return HttpResponse::ForHttpError(net::HTTP_BAD_REQUEST);
   }
 
   if (!server_reachable_) {
-    http_response->server_status = HttpResponse::CONNECTION_UNAVAILABLE;
-    return false;
+    return HttpResponse::ForNetError(net::ERR_FAILED);
   }
 
   // Default to an ok connection.
-  http_response->server_status = HttpResponse::SERVER_CONNECTION_OK;
   client_to_server_response.set_error_code(SyncEnums::SUCCESS);
   const string current_store_birthday = store_birthday();
   client_to_server_response.set_store_birthday(current_store_birthday);
@@ -126,9 +125,8 @@ bool MockConnectionManager::PostBufferToPath(const std::string& buffer_in,
     client_to_server_response.set_error_message("Merry Unbirthday!");
     client_to_server_response.SerializeToString(buffer_out);
     store_birthday_sent_ = true;
-    return true;
+    return HttpResponse::ForSuccess();
   }
-  bool result = true;
   EXPECT_TRUE(!store_birthday_sent_ || post.has_store_birthday() ||
               post.message_contents() ==
                   ClientToServerMessage::CLEAR_SERVER_DATA);
@@ -136,21 +134,21 @@ bool MockConnectionManager::PostBufferToPath(const std::string& buffer_in,
 
   if (post.message_contents() == ClientToServerMessage::COMMIT) {
     if (!ProcessCommit(&post, &client_to_server_response)) {
-      return false;
+      return HttpResponse::ForIoError();
     }
 
   } else if (post.message_contents() == ClientToServerMessage::GET_UPDATES) {
     if (!ProcessGetUpdates(&post, &client_to_server_response)) {
-      return false;
+      return HttpResponse::ForIoError();
     }
   } else if (post.message_contents() ==
              ClientToServerMessage::CLEAR_SERVER_DATA) {
     if (!ProcessClearServerData(&post, &client_to_server_response)) {
-      return false;
+      return HttpResponse::ForIoError();
     }
   } else {
     EXPECT_TRUE(false) << "Unknown/unsupported ClientToServerMessage";
-    return false;
+    return HttpResponse::ForIoError();
   }
 
   {
@@ -187,7 +185,7 @@ bool MockConnectionManager::PostBufferToPath(const std::string& buffer_in,
     mid_commit_observer_->Observe();
   }
 
-  return result;
+  return HttpResponse::ForSuccess();
 }
 
 sync_pb::GetUpdatesResponse* MockConnectionManager::GetUpdateResponse() {

components/sync/test/engine/mock_connection_manager.h

@@ -39,11 +39,10 @@ class MockConnectionManager : public ServerConnectionManager {
   ~MockConnectionManager() override;
 
   // Overridden ServerConnectionManager functions.
-  bool PostBufferToPath(const std::string& buffer_in,
-                        const std::string& path,
-                        const std::string& access_token,
-                        std::string* buffer_out,
-                        HttpResponse* http_response) override;
+  HttpResponse PostBufferToPath(const std::string& buffer_in,
+                                const std::string& path,
+                                const std::string& access_token,
+                                std::string* buffer_out) override;
 
   // Control of commit response.
   // NOTE: Commit callback is invoked only once then reset.