Remove notion of ServerConnectionCode from fake server

HttpResponse::ServerConnectionCode (poorly named, because it includes
network status too) should not be exposed to the fake server (or any
server).

Instead, let the "client" construct that information based on the
network error and http response code. Specifically, the logic is moved
to LoopbackConnectionManager, which becomes more analogous to the
"production" ServerConnectionManager.

This removes some weirdness like FakeServer::SendToLoopbackServer()
dropping |server_status| altogether.

Bug: 774180
Change-Id: I52e51925a63a08e592c40d840a9319ff2244616a
Reviewed-on: https://chromium-review.googlesource.com/c/1347286Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610402}

components/sync/engine_impl/loopback_server/loopback_connection_manager.cc

@@ -3,6 +3,9 @@
 // found in the LICENSE file.
 #include "components/sync/engine_impl/loopback_server/loopback_connection_manager.h"
 
+#include "components/sync/engine_impl/net/server_connection_manager.h"
+#include "net/http/http_status_code.h"
+
 namespace syncer {
 
 LoopbackConnectionManager::LoopbackConnectionManager(
@@ -18,9 +21,15 @@ bool LoopbackConnectionManager::PostBufferToPath(
     const std::string& path,
     const std::string& auth_token) {
   loopback_server_.HandleCommand(
-      params->buffer_in, &params->response.server_status,
-      &params->response.response_code, &params->buffer_out);
-  return params->response.server_status == HttpResponse::SERVER_CONNECTION_OK;
+      params->buffer_in, &params->response.response_code, &params->buffer_out);
+
+  if (params->response.response_code != net::HTTP_OK) {
+    params->response.server_status = HttpResponse::SYNC_SERVER_ERROR;
+    return false;
+  }
+
+  params->response.server_status = HttpResponse::SERVER_CONNECTION_OK;
+  return true;
 }
 
 }  // namespace syncer

components/sync/engine_impl/loopback_server/loopback_server.cc

@@ -233,11 +233,9 @@ void LoopbackServer::SaveEntity(std::unique_ptr<LoopbackServerEntity> entity) {
   entities_[entity->GetId()] = std::move(entity);
 }
 
-void LoopbackServer::HandleCommand(
-    const string& request,
-    HttpResponse::ServerConnectionCode* server_status,
-    int64_t* response_code,
-    std::string* response) {
+void LoopbackServer::HandleCommand(const string& request,
+                                   int* response_code,
+                                   std::string* response) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   sync_pb::ClientToServerMessage message;
@@ -267,14 +265,12 @@ void LoopbackServer::HandleCommand(
         success = true;
         break;
       default:
-        *server_status = HttpResponse::SYNC_SERVER_ERROR;
         *response_code = net::ERR_NOT_IMPLEMENTED;
         *response = string();
         return;
     }
 
     if (!success) {
-      *server_status = HttpResponse::SYNC_SERVER_ERROR;
       *response_code = net::ERR_FAILED;
       *response = string();
       UMA_HISTOGRAM_ENUMERATION(
@@ -288,7 +284,6 @@ void LoopbackServer::HandleCommand(
 
   response_proto.set_store_birthday(GetStoreBirthday());
 
-  *server_status = HttpResponse::SERVER_CONNECTION_OK;
   *response_code = net::HTTP_OK;
   *response = response_proto.SerializeAsString();
 

components/sync/engine_impl/loopback_server/loopback_server.h

@@ -18,7 +18,6 @@
 #include "base/values.h"
 #include "components/sync/base/model_type.h"
 #include "components/sync/engine_impl/loopback_server/loopback_server_entity.h"
-#include "components/sync/engine_impl/net/server_connection_manager.h"
 #include "components/sync/protocol/loopback_server.pb.h"
 #include "components/sync/protocol/sync.pb.h"
 
@@ -49,8 +48,7 @@ class LoopbackServer {
   // used to pass data back to the caller. The command has failed if the value
   // pointed to by |error_code| is nonzero.
   void HandleCommand(const std::string& request,
-                     HttpResponse::ServerConnectionCode* server_status,
-                     int64_t* response_code,
+                     int* response_code,
                      std::string* response);
 
   // Enables strong consistency model (i.e. server detects conflicts).

components/sync/engine_impl/net/server_connection_manager.h

@@ -55,7 +55,7 @@ struct HttpResponse {
   };
 
   // The HTTP Status code.
-  int64_t response_code;
+  int response_code;
 
   // The value of the Content-length header.
   int64_t content_length;

components/sync/test/fake_server/fake_server.cc

@@ -239,12 +239,10 @@ void FakeServer::HandleWalletRequest(
 
 int FakeServer::SendToLoopbackServer(const std::string& request,
                                      std::string* response) {
-  int64_t response_code;
-  syncer::HttpResponse::ServerConnectionCode server_status;
+  int response_code;
   base::ThreadRestrictions::SetIOAllowed(true);
-  loopback_server_->HandleCommand(request, &server_status, &response_code,
-                                  response);
-  return static_cast<int>(response_code);
+  loopback_server_->HandleCommand(request, &response_code, response);
+  return response_code;
 }
 
 void FakeServer::InjectClientCommand(std::string* response) {