[Offline Prefetech] Send GetOperationReqest to the server

components/offline_pages/core/prefetch/prefetch_request_fetcher.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/offline_pages/core/prefetch/prefetch_request_fetcher.h"
 
 #include "base/logging.h"
 #include "net/base/load_flags.h"
+#include "net/http/http_request_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_status.h"
 #include "url/gurl.h"
 
 namespace offline_pages {
 
 namespace {
+
+const char kPrefetchSever[] =
+    "http://staging-offlinepages-pa.sandbox.googleapis.com/";
+
+// Content type needed in order to communicate with the server in binary
+// proto format.
 const char kRequestContentType[] = "application/x-protobuf";
+
 }  // namespace
 
 PrefetchRequestFetcher::PrefetchRequestFetcher(
-    const GURL& url,
+    const std::string& url_path,
     const std::string& message,
     net::URLRequestContextGetter* request_context_getter,
     const FinishedCallback& callback)
     : request_context_getter_(request_context_getter), callback_(callback) {
   net::NetworkTrafficAnnotationTag traffic_annotation =
       net::DefineNetworkTrafficAnnotation("offline_prefetch", R"(
         semantics {
           sender: "Offline Prefetch"
           description:
             "Chromium interacts with Offline Page Service to prefetch "
             "suggested website resources."
           trigger:
             "When there are suggested website resources to fetch."
           data:
             "URLs of the suggested website resources to fetch."
           destination: GOOGLE_OWNED_SERVICE
         }
         policy {
           cookies_allowed: false
           setting:
             "Users can enable or disable the offline prefetch by toggling"
             "chrome://flags#offline-prefetch in Chromium on Android."
           policy_exception_justification:
             "Not implemented, considered not useful."
         })");
-  url_fetcher_ = net::URLFetcher::Create(url, net::URLFetcher::POST, this,
-                                         traffic_annotation);
+  url_fetcher_ = net::URLFetcher::Create(
+      GURL(kPrefetchSever + url_path),

[dewittj, 2017/05/16 18:30:20]

This feels like it should be more like

GURL::Replacements replacements;
replacements.SetPathString(url_path);

GURL fetch_url = GURL(kPrefetchServer).ReplaceComponents(replacements);
url_fetcher_ = net::URLFetcher::Create(fetch_url, ...);

[jianli, 2017/05/16 22:44:40]

Since both server and path are hard coded constant strings, concatenating them
seems to be good enough. I like your proposal, but it seems to a bit too
complicated for our simple case.

[dewittj, 2017/05/16 23:06:22]

URL path is not hard coded in the GET case, correct?  Replacements encodes
components correctly, in case the operation name needs it.  Also, in the future
if we need to alter the URL string it won't be a risk, just the path will be
changed.

[jianli, 2017/05/16 23:52:33]

Make sense. Done.

+      message.empty() ? net::URLFetcher::GET : net::URLFetcher::POST, this,
+      traffic_annotation);
   url_fetcher_->SetRequestContext(request_context_getter_.get());
   url_fetcher_->SetAutomaticallyRetryOn5xx(false);
   url_fetcher_->SetAutomaticallyRetryOnNetworkChanges(0);
-  url_fetcher_->SetUploadData(kRequestContentType, message);
+  if (message.empty()) {
+    std::string extra_header(net::HttpRequestHeaders::kContentType);
+    extra_header += ": ";
+    extra_header += kRequestContentType;
+    url_fetcher_->AddExtraRequestHeader(extra_header);
+  } else {
+    url_fetcher_->SetUploadData(kRequestContentType, message);
+  }
   url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
                              net::LOAD_DO_NOT_SAVE_COOKIES);
   url_fetcher_->Start();
 }
 
 PrefetchRequestFetcher::~PrefetchRequestFetcher() {}
 
 void PrefetchRequestFetcher::OnURLFetchComplete(const net::URLFetcher* source) {
   std::string data;
   PrefetchRequestStatus status = ParseResponse(source, &data);
@@ skipping 25 matching lines @@
 
   if (!source->GetResponseAsString(data) || data->empty()) {
     DVLOG(1) << "Failed to get response or empty response";
     return PrefetchRequestStatus::SHOULD_RETRY_WITH_BACKOFF;
   }
 
   return PrefetchRequestStatus::SUCCESS;
 }
 
 }  // offline_pages