[CRD iOS] Prevent crash when failed to fetch host list

HostListFetcher returns a -1 error code when it fails to get the HTTP
response (because of timeout?). We map the HTTP code into error message
using net::GetHttpReasonPhrase(), which reaches NOTREACHED() when the
argument is -1 and causes a crash on debug build.

This CL prevents this by explicitly translating -1 into "Connection
failed".

Bug: 832994
Change-Id: I94f5c353de2674500190efb9dc8066ad40aaed8e
Reviewed-on: https://chromium-review.googlesource.com/1012787
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551181}

remoting/ios/facade/host_list_fetcher.cc

@@ -19,6 +19,11 @@
 
 namespace remoting {
 
+static_assert(static_cast<int>(net::URLFetcher::RESPONSE_CODE_INVALID) !=
+                  static_cast<int>(
+                      HostListFetcher::ResponseCode::RESPONSE_CODE_CANCELLED),
+              "RESPONSE_CODE_INVALID collided with RESPONSE_CODE_CANCELLED.");
+
 namespace {
 
 // Used by the HostlistFetcher to make HTTP requests and also by the

remoting/ios/facade/host_list_fetcher.h

@@ -27,9 +27,9 @@ class HostListFetcher : public net::URLFetcherDelegate {
  public:
   // Imposible http response code. Used to signal that the request has been
   // cancelled.
+  // TODO(yuweih): Add all response code values here and make the callback
+  // return this enum instead of int.
   enum ResponseCode {
-    // URLFetcher::RESPONSE_CODE_INVALID = -1. Use -257 to (hopefully) prevent
-    // collisions.
     RESPONSE_CODE_CANCELLED = -257,
   };
 

remoting/ios/facade/host_list_service.mm

@@ -20,13 +20,45 @@
 
 #include "base/i18n/time_formatting.h"
 #include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
+#include "net/url_request/url_fetcher.h"
 #include "remoting/base/string_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace remoting {
 
+namespace {
+
+bool IsValidErrorCode(int error_code) {
+#define HTTP_STATUS(label, code, reason) \
+  if (error_code == code)                \
+    return true;
+#include "net/http/http_status_code_list.h"
+#undef HTTP_STATUS
+  return false;
+}
+
+std::string GetRequestErrorMessage(int error_code) {
+  if (IsValidErrorCode(error_code)) {
+    std::string error_phrase =
+        net::GetHttpReasonPhrase(static_cast<net::HttpStatusCode>(error_code));
+    return l10n_util::GetStringFUTF8(IDS_SERVER_COMMUNICATION_ERROR,
+                                     base::UTF8ToUTF16(error_phrase));
+  }
+
+  switch (error_code) {
+    case net::URLFetcher::RESPONSE_CODE_INVALID:
+      return l10n_util::GetStringUTF8(IDS_ERROR_NETWORK_ERROR);
+    default:
+      return l10n_util::GetStringFUTF8(IDS_SERVER_COMMUNICATION_ERROR,
+                                       base::IntToString16(error_code));
+  }
+}
+
+}  // namespace
+
 HostListService* HostListService::GetInstance() {
   static base::NoDestructor<HostListService> instance;
   return instance.get();
@@ -164,16 +196,16 @@ void HostListService::HandleFetchFailure(FetchFailureReason reason,
           l10n_util::GetStringUTF8(IDS_ERROR_OAUTH_TOKEN_INVALID);
       break;
     case FetchFailureReason::REQUEST_ERROR:
-      last_fetch_failure_->localized_description = l10n_util::GetStringFUTF8(
-          IDS_SERVER_COMMUNICATION_ERROR,
-          base::UTF8ToUTF16(net::GetHttpReasonPhrase(
-              static_cast<net::HttpStatusCode>(error_code))));
+      last_fetch_failure_->localized_description =
+          GetRequestErrorMessage(error_code);
       break;
     default:
       NOTREACHED();
   }
   LOG(WARNING) << "Failed to fetch host list: "
-               << last_fetch_failure_->localized_description;
+               << last_fetch_failure_->localized_description
+               << " reason: " << static_cast<int>(reason)
+               << ", error_code: " << error_code;
   fetch_failure_callbacks_.Notify();
 }