[Sync] Sync should run canary cycle after any network related error

Today sync only runs canary cycle after small subset of network errors
that are related to connection/network failure. This leaves sync in
exponential backoff when unexpected error is returned from network
stack.

In this change I make sync translate all network errors to
CONNECTION_UNAVAILABLE therefore making it try canary job after network
change.

BUG=539943
R=zea@chromium.org

Review URL: https://codereview.chromium.org/1553433002

Cr-Commit-Position: refs/heads/master@{#367746}

4 files changed, 71 insertions, 3 deletions

diff --git a/sync/internal_api/http_bridge.cc b/sync/internal_api/http_bridge.cc
index 842adf3..b8d57cc 100644
--- a/sync/internal_api/http_bridge.cc
+++ b/sync/internal_api/http_bridge.cc
@@ -512,7 +512,7 @@ void HttpBridge::OnURLFetchTimedOut() {
   fetch_state_.request_completed = true;
   fetch_state_.request_succeeded = false;
   fetch_state_.http_response_code = -1;
-  fetch_state_.error_code = net::URLRequestStatus::FAILED;
+  fetch_state_.error_code = net::ERR_TIMED_OUT;
 
   // This method is called by the timer, not the url fetcher implementation,
   // so it's safe to delete the fetcher here.
diff --git a/sync/internal_api/syncapi_server_connection_manager.cc b/sync/internal_api/syncapi_server_connection_manager.cc
index 16646ec..a54fcbc 100644
--- a/sync/internal_api/syncapi_server_connection_manager.cc
+++ b/sync/internal_api/syncapi_server_connection_manager.cc
@@ -53,9 +53,9 @@ bool SyncAPIBridgedConnection::Init(const char* path,
   int error_code = 0;
   int response_code = 0;
   if (!http->MakeSynchronousPost(&error_code, &response_code)) {
+    DCHECK_NE(error_code, net::OK);
     DVLOG(1) << "Http POST failed, error returns: " << error_code;
-    response->server_status = HttpResponse::ServerConnectionCodeFromNetError(
-        error_code);
+    response->server_status = HttpResponse::CONNECTION_UNAVAILABLE;
     return false;
   }
 
diff --git a/sync/internal_api/syncapi_server_connection_manager.h b/sync/internal_api/syncapi_server_connection_manager.h
index aa1fe1e..225959b 100644
--- a/sync/internal_api/syncapi_server_connection_manager.h
+++ b/sync/internal_api/syncapi_server_connection_manager.h
@@ -67,6 +67,8 @@ class SYNC_EXPORT SyncAPIServerConnectionManager
                            VeryEarlyAbortPost);
   FRIEND_TEST_ALL_PREFIXES(SyncAPIServerConnectionManagerTest, EarlyAbortPost);
   FRIEND_TEST_ALL_PREFIXES(SyncAPIServerConnectionManagerTest, AbortPost);
+  FRIEND_TEST_ALL_PREFIXES(SyncAPIServerConnectionManagerTest,
+                           FailPostWithTimedOut);
 
   // A factory creating concrete HttpPostProviders for use whenever we need to
   // issue a POST to sync servers.
diff --git a/sync/internal_api/syncapi_server_connection_manager_unittest.cc b/sync/internal_api/syncapi_server_connection_manager_unittest.cc
index 208b3a2..9e67be8 100644
--- a/sync/internal_api/syncapi_server_connection_manager_unittest.cc
+++ b/sync/internal_api/syncapi_server_connection_manager_unittest.cc
@@ -120,4 +120,70 @@ TEST(SyncAPIServerConnectionManagerTest, AbortPost) {
   abort_thread.Stop();
 }
 
+namespace {
+
+class FailingHttpPost : public HttpPostProviderInterface {
+ public:
+  explicit FailingHttpPost(int error_code) : error_code_(error_code) {}
+  ~FailingHttpPost() override {}
+
+  void SetExtraRequestHeaders(const char* headers) override {}
+  void SetURL(const char* url, int port) override {}
+  void SetPostPayload(const char* content_type,
+                      int content_length,
+                      const char* content) override {}
+  bool MakeSynchronousPost(int* error_code, int* response_code) override {
+    *error_code = error_code_;
+    return false;
+  }
+  int GetResponseContentLength() const override { return 0; }
+  const char* GetResponseContent() const override { return ""; }
+  const std::string GetResponseHeaderValue(
+      const std::string& name) const override {
+    return std::string();
+  }
+  void Abort() override {}
+
+ private:
+  int error_code_;
+};
+
+class FailingHttpPostFactory : public HttpPostProviderFactory {
+ public:
+  explicit FailingHttpPostFactory(int error_code) : error_code_(error_code) {}
+  ~FailingHttpPostFactory() override {}
+  void Init(const std::string& user_agent,
+            const BindToTrackerCallback& bind_to_tracker_callback) override {}
+
+  HttpPostProviderInterface* Create() override {
+    return new FailingHttpPost(error_code_);
+  }
+  void Destroy(HttpPostProviderInterface* http) override {
+    delete static_cast<FailingHttpPost*>(http);
+  }
+
+ private:
+  int error_code_;
+};
+
+}  // namespace
+
+// Fail request with TIMED_OUT error. Make sure server status is
+// CONNECTION_UNAVAILABLE and therefore request will be retried after network
+// change.
+TEST(SyncAPIServerConnectionManagerTest, FailPostWithTimedOut) {
+  CancelationSignal signal;
+  SyncAPIServerConnectionManager server(
+      "server", 0, true, new FailingHttpPostFactory(net::ERR_TIMED_OUT),
+      &signal);
+
+  ServerConnectionManager::PostBufferParams params;
+
+  bool result = server.PostBufferToPath(&params, "/testpath", "testauth");
+
+  EXPECT_FALSE(result);
+  EXPECT_EQ(HttpResponse::CONNECTION_UNAVAILABLE,
+            params.response.server_status);
+}
+
 }  // namespace syncer