Prohibit blink::ResourceRequest implicit copy.

This CL deletes the default copy constructor and introduce
a move constructor and CopyFrom() method for preparing
ResourceRequest copy removal.
This is a preparation to introduce PendingRemote,
which is non-copiable, into ResourceRequest for streaming body upload
(design doc: http://bit.ly/2tSWsIH).

Bug: 787704
Change-Id: I9a4d564a85f8a2723f581726362a64082f99a726
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1948393Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732302}

third_party/blink/renderer/core/html/lazy_load_frame_observer.cc

@@ -93,11 +93,13 @@ int GetLazyFrameLoadingViewportDistanceThresholdPx(const Document& document) {
 }  // namespace
 
 struct LazyLoadFrameObserver::LazyLoadRequestInfo {
-  LazyLoadRequestInfo(const ResourceRequest& resource_request,
+  LazyLoadRequestInfo(const ResourceRequest& passed_resource_request,
                       WebFrameLoadType frame_load_type)
-      : resource_request(resource_request), frame_load_type(frame_load_type) {}
+      : frame_load_type(frame_load_type) {
+    resource_request.CopyFrom(passed_resource_request);
+  }
 
-  const ResourceRequest resource_request;
+  ResourceRequest resource_request;
   const WebFrameLoadType frame_load_type;
 };
 

third_party/blink/renderer/core/loader/frame_load_request.cc

@@ -43,11 +43,10 @@ static void SetReferrerForRequest(Document* origin_document,
 
 FrameLoadRequest::FrameLoadRequest(Document* origin_document,
                                    const ResourceRequest& resource_request)
-
     : origin_document_(origin_document),
-      resource_request_(resource_request),
       should_send_referrer_(kMaybeSendReferrer) {
   // These flags are passed to a service worker which controls the page.
+  resource_request_.CopyFrom(resource_request);
   resource_request_.SetMode(network::mojom::RequestMode::kNavigate);
   resource_request_.SetCredentialsMode(
       network::mojom::CredentialsMode::kInclude);

third_party/blink/renderer/core/loader/threadable_loader.cc

@@ -274,7 +274,8 @@ void ThreadableLoader::Start(const ResourceRequest& request) {
   request_headers_ = request.HttpHeaderFields();
   report_upload_progress_ = request.ReportUploadProgress();
 
-  ResourceRequest new_request(request);
+  ResourceRequest new_request;
+  new_request.CopyFrom(request);
 
   // Set the service worker mode to none if "bypass for network" in DevTools is
   // enabled.
@@ -319,7 +320,7 @@ void ThreadableLoader::Start(const ResourceRequest& request) {
     // Save the request to fallback_request_for_service_worker to use when the
     // service worker doesn't handle (call respondWith()) a CORS enabled
     // request.
-    fallback_request_for_service_worker_ = ResourceRequest(request);
+    fallback_request_for_service_worker_.CopyFrom(request);
     // Skip the service worker for the fallback request.
     fallback_request_for_service_worker_.SetSkipServiceWorker(true);
   }
@@ -356,7 +357,7 @@ void ThreadableLoader::LoadPreflightRequest(
   std::unique_ptr<ResourceRequest> preflight_request =
       CreateAccessControlPreflightRequest(actual_request, GetSecurityOrigin());
 
-  actual_request_ = actual_request;
+  actual_request_.CopyFrom(actual_request);
   actual_options_ = actual_options;
 
   // Explicitly set |skip_service_worker| to true here. Although the page is
@@ -405,7 +406,8 @@ void ThreadableLoader::MakeCrossOriginAccessRequest(
     return;
   }
 
-  ResourceRequest cross_origin_request(request);
+  ResourceRequest cross_origin_request;
+  cross_origin_request.CopyFrom(request);
   ResourceLoaderOptions cross_origin_options(resource_loader_options_);
 
   cross_origin_request.RemoveUserAndPassFromURL();
@@ -678,7 +680,8 @@ bool ThreadableLoader::RedirectReceived(
   // We're initiating a new request (for redirect), so update
   // |last_request_url_| by destroying |assign_on_scope_exit|.
 
-  ResourceRequest cross_origin_request(new_request);
+  ResourceRequest cross_origin_request;
+  cross_origin_request.CopyFrom(new_request);
   cross_origin_request.SetInitialUrlForResourceTiming(initial_request_url_);
 
   // Remove any headers that may have been added by the network layer that cause
@@ -949,15 +952,17 @@ void ThreadableLoader::LoadFallbackRequestForServiceWorker() {
   if (GetResource())
     checker_.WillRemoveClient();
   ClearResource();
-  ResourceRequest fallback_request(fallback_request_for_service_worker_);
-  fallback_request_for_service_worker_ = ResourceRequest();
+  ResourceRequest fallback_request;
+  fallback_request.CopyFrom(fallback_request_for_service_worker_);
+  fallback_request_for_service_worker_.CopyFrom(ResourceRequest());
   DispatchInitialRequest(fallback_request);
 }
 
 void ThreadableLoader::LoadActualRequest() {
-  ResourceRequest actual_request = actual_request_;
+  ResourceRequest actual_request;
+  actual_request.CopyFrom(actual_request_);
   ResourceLoaderOptions actual_options = actual_options_;
-  actual_request_ = ResourceRequest();
+  actual_request_.CopyFrom(ResourceRequest());
   actual_options_ = ResourceLoaderOptions();
 
   if (GetResource())

third_party/blink/renderer/platform/exported/web_url_request.cc

@@ -58,8 +58,9 @@ WebURLRequest::ExtraData::ExtraData() : render_frame_id_(MSG_ROUTING_NONE) {}
 // TODO(keishi): Replace with GCWrapper<ResourceRequest>
 struct WebURLRequest::ResourceRequestContainer {
   ResourceRequestContainer() = default;
-  explicit ResourceRequestContainer(const ResourceRequest& r)
-      : resource_request(r) {}
+  explicit ResourceRequestContainer(const ResourceRequest& r) {
+    resource_request.CopyFrom(r);
+  }
 
   ResourceRequest resource_request;
 };
@@ -84,8 +85,9 @@ WebURLRequest& WebURLRequest::operator=(const WebURLRequest& r) {
   // semantics via this operator is just not supported.
   DCHECK(owned_resource_request_);
   DCHECK(resource_request_);
-  if (&r != this)
-    *resource_request_ = *r.resource_request_;
+  if (&r != this) {
+    resource_request_->CopyFrom(*r.resource_request_);
+  }
   return *this;
 }
 

third_party/blink/renderer/platform/loader/fetch/fetch_parameters.cc

@@ -34,20 +34,22 @@
 namespace blink {
 
 FetchParameters::FetchParameters(const ResourceRequest& resource_request)
-    : resource_request_(resource_request),
-      decoder_options_(TextResourceDecoderOptions::kPlainTextContent),
+    : decoder_options_(TextResourceDecoderOptions::kPlainTextContent),
       speculative_preload_type_(SpeculativePreloadType::kNotSpeculative),
       defer_(kNoDefer),
-      image_request_optimization_(kNone) {}
+      image_request_optimization_(kNone) {
+  resource_request_.CopyFrom(resource_request);
+}
 
 FetchParameters::FetchParameters(const ResourceRequest& resource_request,
                                  const ResourceLoaderOptions& options)
-    : resource_request_(resource_request),
-      decoder_options_(TextResourceDecoderOptions::kPlainTextContent),
+    : decoder_options_(TextResourceDecoderOptions::kPlainTextContent),
       options_(options),
       speculative_preload_type_(SpeculativePreloadType::kNotSpeculative),
       defer_(kNoDefer),
-      image_request_optimization_(kNone) {}
+      image_request_optimization_(kNone) {
+  resource_request_.CopyFrom(resource_request);
+}
 
 FetchParameters::FetchParameters(FetchParameters&&) = default;
 

third_party/blink/renderer/platform/loader/fetch/memory_cache_correctness_test.cc

@@ -318,7 +318,7 @@ TEST_F(MemoryCacheCorrectnessTest, RequestWithNoCache) {
   no_cache_request.SetHttpHeaderField(http_names::kCacheControl, "no-cache");
   no_cache_request.SetRequestorOrigin(GetSecurityOrigin());
   MockResource* no_cache_resource =
-      ResourceFromResourceRequest(no_cache_request);
+      ResourceFromResourceRequest(std::move(no_cache_request));
   MockResource* fetched = FetchMockResource();
   EXPECT_NE(no_cache_resource, fetched);
 }
@@ -349,7 +349,7 @@ TEST_F(MemoryCacheCorrectnessTest, RequestWithNoStore) {
   no_store_request.SetHttpHeaderField(http_names::kCacheControl, "no-store");
   no_store_request.SetRequestorOrigin(GetSecurityOrigin());
   MockResource* no_store_resource =
-      ResourceFromResourceRequest(no_store_request);
+      ResourceFromResourceRequest(std::move(no_store_request));
   MockResource* fetched = FetchMockResource();
   EXPECT_NE(no_store_resource, fetched);
 }

third_party/blink/renderer/platform/loader/fetch/resource.cc

@@ -162,8 +162,8 @@ Resource::Resource(const ResourceRequest& request,
       integrity_disposition_(ResourceIntegrityDisposition::kNotChecked),
       options_(options),
       response_timestamp_(Now()),
-      resource_request_(request),
       overhead_size_(CalculateOverheadSize()) {
+  resource_request_.CopyFrom(request);
   InstanceCounters::IncrementCounter(InstanceCounters::kResourceCounter);
 
   if (IsMainThread())
@@ -490,7 +490,7 @@ void Resource::SetRevalidatingRequest(const ResourceRequest& request) {
   DCHECK(!request.IsNull());
   CHECK(!is_revalidation_start_forbidden_);
   is_revalidating_ = true;
-  resource_request_ = request;
+  resource_request_.CopyFrom(request);
   status_ = ResourceStatus::kNotStarted;
 }
 

third_party/blink/renderer/platform/loader/fetch/resource.h

@@ -378,7 +378,7 @@ class PLATFORM_EXPORT Resource : public GarbageCollected<Resource>,
   // in ResourceFetcher::StartLoad() for retry in cache-aware loading, remove
   // once ResourceRequest is not modified in StartLoad(). crbug.com/632580
   void SetResourceRequest(const ResourceRequest& resource_request) {
-    resource_request_ = resource_request;
+    resource_request_.CopyFrom(resource_request);
   }
 
   // Used by the MemoryCache to reduce the memory consumption of the entry.
@@ -475,7 +475,9 @@ class PLATFORM_EXPORT Resource : public GarbageCollected<Resource>,
    public:
     explicit RedirectPair(const ResourceRequest& request,
                           const ResourceResponse& redirect_response)
-        : request_(request), redirect_response_(redirect_response) {}
+        : redirect_response_(redirect_response) {
+      request_.CopyFrom(request);
+    }
 
     ResourceRequest request_;
     ResourceResponse redirect_response_;

third_party/blink/renderer/platform/loader/fetch/resource_loader.cc

@@ -389,7 +389,7 @@ ResourceLoader::ResourceLoader(ResourceFetcher* fetcher,
   // If they are keepalive request && their responses are not observable to web
   // content, we can have them survive without breaking web content when the
   // page is put into BackForwardCache.
-  auto request = resource_->GetResourceRequest();
+  auto& request = resource_->GetResourceRequest();
   if (!RequestContextObserveResponse(request.GetRequestContext())) {
     if (FrameScheduler* frame_scheduler = fetcher->GetFrameScheduler()) {
       feature_handle_for_scheduler_ = frame_scheduler->RegisterFeature(
@@ -570,7 +570,8 @@ void ResourceLoader::StartWith(const ResourceRequest& request) {
   if (is_cache_aware_loading_activated_) {
     // Override cache policy for cache-aware loading. If this request fails, a
     // reload with original request will be triggered in DidFail().
-    ResourceRequest cache_aware_request(request);
+    ResourceRequest cache_aware_request;
+    cache_aware_request.CopyFrom(request);
     cache_aware_request.SetCacheMode(
         mojom::FetchCacheMode::kUnspecifiedOnlyIfCachedStrict);
     RequestAsynchronously(cache_aware_request);
@@ -962,7 +963,8 @@ void ResourceLoader::DidReceiveResponseInternal(
             kEnableCorsHandlingByResourceFetcher &&
         request_mode == network::mojom::RequestMode::kCors &&
         response.WasFallbackRequiredByServiceWorker()) {
-      ResourceRequest last_request = resource_->LastResourceRequest();
+      ResourceRequest last_request;
+      last_request.CopyFrom(resource_->LastResourceRequest());
       DCHECK(!last_request.GetSkipServiceWorker());
       // This code handles the case when a controlling service worker doesn't
       // handle a cross origin request.

third_party/blink/renderer/platform/loader/fetch/resource_request.cc

@@ -82,12 +82,18 @@ ResourceRequest::ResourceRequest(const KURL& url)
           network::mojom::CorsPreflightPolicy::kConsiderPreflight),
       redirect_status_(RedirectStatus::kNoRedirect) {}
 
-ResourceRequest::ResourceRequest(const ResourceRequest&) = default;
-
 ResourceRequest::~ResourceRequest() = default;
 
 ResourceRequest& ResourceRequest::operator=(const ResourceRequest&) = default;
 
+ResourceRequest::ResourceRequest(ResourceRequest&&) = default;
+
+ResourceRequest& ResourceRequest::operator=(ResourceRequest&&) = default;
+
+void ResourceRequest::CopyFrom(const ResourceRequest& src) {
+  *this = src;
+}
+
 std::unique_ptr<ResourceRequest> ResourceRequest::CreateRedirectRequest(
     const KURL& new_url,
     const AtomicString& new_method,

third_party/blink/renderer/platform/loader/fetch/resource_request.h

@@ -70,13 +70,16 @@ class PLATFORM_EXPORT ResourceRequest final {
   explicit ResourceRequest(const String& url_string);
   explicit ResourceRequest(const KURL&);
 
-  // TODO(toyoshim): Use std::unique_ptr as much as possible, and hopefully
-  // make ResourceRequest DISALLOW_COPY_AND_ASSIGN. See crbug.com/787704.
-  ResourceRequest(const ResourceRequest&);
-  ResourceRequest& operator=(const ResourceRequest&);
+  ResourceRequest(const ResourceRequest&) = delete;
+  ResourceRequest(ResourceRequest&&);
+  ResourceRequest& operator=(ResourceRequest&&);
 
   ~ResourceRequest();
 
+  // TODO(yoichio): Use move semantics as much as possible.
+  // See crbug.com/787704.
+  void CopyFrom(const ResourceRequest&);
+
   // Constructs a new ResourceRequest for a redirect from this instance.
   std::unique_ptr<ResourceRequest> CreateRedirectRequest(
       const KURL& new_url,
@@ -457,6 +460,8 @@ class PLATFORM_EXPORT ResourceRequest final {
   using SharableExtraData =
       base::RefCountedData<std::unique_ptr<WebURLRequest::ExtraData>>;
 
+  ResourceRequest& operator=(const ResourceRequest&);
+
   const CacheControlHeader& GetCacheControlHeader() const;
 
   bool NeedsHTTPOrigin() const;