Add support for subresource request fallback in AppCache for the network service..

content/browser/appcache/appcache_url_loader_job.cc

 // Copyright (c) 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 "content/browser/appcache/appcache_url_loader_job.h"
 
 #include "base/strings/string_number_conversions.h"
 #include "content/browser/appcache/appcache_histograms.h"
 #include "content/browser/appcache/appcache_subresource_url_factory.h"
+#include "content/browser/appcache/appcache_url_loader_request.h"
 #include "content/browser/url_loader_factory_getter.h"
 #include "content/common/net_adapters.h"
 #include "content/public/common/resource_type.h"
 #include "net/http/http_status_code.h"
 
 namespace content {
 
 SubresourceLoadInfo::SubresourceLoadInfo()
     : routing_id(-1), request_id(-1), options(0) {}
 
@@ skipping 55 matching lines @@
     // need to inform the loader callback that we are not going to handle this
     // request. The loader callback is valid only for navigation requests.
     std::move(main_resource_loader_callback_).Run(StartLoaderCallback());
   } else {
     default_url_loader_factory_getter_->GetNetworkFactory()
         ->get()
         ->CreateLoaderAndStart(
             mojo::MakeRequest(&network_loader_request_),
             subresource_load_info_->routing_id,
             subresource_load_info_->request_id, subresource_load_info_->options,
-            subresource_load_info_->request, std::move(client_info_),
+            subresource_load_info_->request, CreateLoaderClientProxy(),
             subresource_load_info_->traffic_annotation);
   }
 }
 
 void AppCacheURLLoaderJob::DeliverErrorResponse() {
   delivery_type_ = ERROR_DELIVERY;
 
   // We expect the URLLoaderClient pointer to be valid at this point.
   DCHECK(client_info_);
 
@@ skipping 27 matching lines @@
   if (network_loader_request_)
     network_loader_request_->FollowRedirect();
 }
 
 void AppCacheURLLoaderJob::SetPriority(net::RequestPriority priority,
                                        int32_t intra_priority_value) {
   if (network_loader_request_)
     network_loader_request_->SetPriority(priority, intra_priority_value);
 }
 
+void AppCacheURLLoaderJob::OnReceiveResponse(
+    const ResourceResponseHead& response_head,
+    const base::Optional<net::SSLInfo>& ssl_info,
+    mojom::DownloadedTempFilePtr downloaded_file) {
+  url_loader_request_instance_->set_response(response_head);
+  // The MaybeLoadFallbackForResponse() call below can pass a fallback
+  // response to us. Reset the delivery_type_ to ensure that we can
+  // receive it
+  delivery_type_ = AWAITING_DELIVERY_ORDERS;
+  if (!sub_resource_handler_->MaybeLoadFallbackForResponse(nullptr)) {
+    client_info_->OnReceiveResponse(response_head, ssl_info,
+                                    std::move(downloaded_file));
+  } else {
+    // Close the pipe to the network loader as we are delivering a fallback
+    // response to the client.
+    loader_client_proxy_binding_.Close();
+  }
+}
+
+void AppCacheURLLoaderJob::OnReceiveRedirect(
+    const net::RedirectInfo& redirect_info,
+    const ResourceResponseHead& response_head) {
+  url_loader_request_instance_->set_response(response_head);
+  // The MaybeLoadFallbackForRedirect() call below can pass a fallback
+  // response to us. Reset the delivery_type_ to ensure that we can
+  // receive it
+  delivery_type_ = AWAITING_DELIVERY_ORDERS;
+  if (!sub_resource_handler_->MaybeLoadFallbackForRedirect(
+          nullptr, redirect_info.new_url)) {
+    client_info_->OnReceiveRedirect(redirect_info, response_head);
+  } else {
+    // Close the pipe to the network loader as we are delivering a fallback
+    // response to the client.
+    loader_client_proxy_binding_.Close();
+  }
+}
+
+void AppCacheURLLoaderJob::OnDataDownloaded(int64_t data_len,
+                                            int64_t encoded_data_len) {
+  client_info_->OnDataDownloaded(data_len, encoded_data_len);
+}
+
+void AppCacheURLLoaderJob::OnUploadProgress(
+    int64_t current_position,
+    int64_t total_size,
+    OnUploadProgressCallback ack_callback) {
+  client_info_->OnUploadProgress(current_position, total_size,
+                                 std::move(ack_callback));
+}
+
+void AppCacheURLLoaderJob::OnReceiveCachedMetadata(
+    const std::vector<uint8_t>& data) {
+  client_info_->OnReceiveCachedMetadata(data);
+}
+
+void AppCacheURLLoaderJob::OnTransferSizeUpdated(int32_t transfer_size_diff) {
+  client_info_->OnTransferSizeUpdated(transfer_size_diff);
+}
+
+void AppCacheURLLoaderJob::OnStartLoadingResponseBody(
+    mojo::ScopedDataPipeConsumerHandle body) {
+  client_info_->OnStartLoadingResponseBody(std::move(body));
+}
+
+void AppCacheURLLoaderJob::OnComplete(
+    const ResourceRequestCompletionStatus& status) {
+  delivery_type_ = AWAITING_DELIVERY_ORDERS;
+  if (!sub_resource_handler_->MaybeLoadFallbackForResponse(nullptr)) {
+    client_info_->OnComplete(status);
+  } else {
+    // Close the pipe to the network loader as we are delivering a fallback
+    // response to the client.
+    loader_client_proxy_binding_.Close();

[michaeln, 2017/07/18 19:19:22]

This turns off the network loaders delegate callbacks (so to speak), but should
we also reset the actual loader to really kill it and free up all resources
associated with it?

network_loader_client_binding_.Close();
network_loader_.reset(); 
or
network_loader_ = nullptr;

[ananta, 2017/07/18 19:52:40]

Thanks. Done. I moved this to a function DisconnectFromNetworkLoader().

+  }
+}
+
 void AppCacheURLLoaderJob::SetSubresourceLoadInfo(
     std::unique_ptr<SubresourceLoadInfo> subresource_load_info,
     URLLoaderFactoryGetter* default_url_loader) {
   subresource_load_info_ = std::move(subresource_load_info);
 
   associated_binding_.reset(new mojo::AssociatedBinding<mojom::URLLoader>(
       this, std::move(subresource_load_info_->url_loader_request)));
   associated_binding_->set_connection_error_handler(base::Bind(
       &AppCacheURLLoaderJob::OnConnectionError, StaticAsWeakPtr(this)));
 
   client_info_ = std::move(subresource_load_info_->client);
   default_url_loader_factory_getter_ = default_url_loader;
 }
 
 void AppCacheURLLoaderJob::Start(mojom::URLLoaderRequest request,
                                  mojom::URLLoaderClientPtr client) {
   DCHECK(!binding_.is_bound());
   binding_.Bind(std::move(request));
 
   binding_.set_connection_error_handler(base::Bind(
       &AppCacheURLLoaderJob::OnConnectionError, StaticAsWeakPtr(this)));
 
   client_info_ = std::move(client);
 
   // Send the cached AppCacheResponse if any.
   if (info_.get())
     SendResponseInfo();
 }
 
-AppCacheURLLoaderJob::AppCacheURLLoaderJob(const ResourceRequest& request,
-                                           AppCacheStorage* storage)
+AppCacheURLLoaderJob::AppCacheURLLoaderJob(
+    const ResourceRequest& request,
+    AppCacheURLLoaderRequest* url_loader_request,

[michaeln, 2017/07/18 19:19:22]

Calling this 'url_loader_request' is misleading, it makes it sounds like a
specific kind of mojom::URLLoaderRequest which is something completely
different. This is a straight c++ AppCacheRequest derivative.

Maybe just 'appcache_request' and 'appcache_request_' wdyt?

[ananta, 2017/07/18 19:52:40]

Makes sense. The names are confusing. Done.

+    AppCacheStorage* storage)
     : request_(request),
       storage_(storage->GetWeakPtr()),
       start_time_tick_(base::TimeTicks::Now()),
       cache_id_(kAppCacheNoCacheId),
       is_fallback_(false),
       binding_(this),
       writable_handle_watcher_(FROM_HERE,
-                               mojo::SimpleWatcher::ArmingPolicy::MANUAL) {}
+                               mojo::SimpleWatcher::ArmingPolicy::MANUAL),
+      loader_client_proxy_binding_(this),
+      url_loader_request_instance_(url_loader_request) {}
 
 void AppCacheURLLoaderJob::OnResponseInfoLoaded(
     AppCacheResponseInfo* response_info,
     int64_t response_id) {
   DCHECK(IsDeliveringAppCacheResponse());
 
   if (!storage_.get()) {
     DeliverErrorResponse();
     return;
   }
@@ skipping 104 matching lines @@
                          : info_->response_data_size();
 
   response_head.connection_info = http_info->connection_info;
   response_head.socket_address = http_info->socket_address;
   response_head.was_fetched_via_spdy = http_info->was_fetched_via_spdy;
   response_head.was_alpn_negotiated = http_info->was_alpn_negotiated;
   response_head.alpn_negotiated_protocol = http_info->alpn_negotiated_protocol;
 
   response_head.load_timing = load_timing_info_;
 
+  url_loader_request_instance_->set_response(response_head);
+
   client_info_->OnReceiveResponse(response_head, http_info->ssl_info,
                                   mojom::DownloadedTempFilePtr());
 
   client_info_->OnStartLoadingResponseBody(
       std::move(data_pipe_.consumer_handle));
 }
 
 void AppCacheURLLoaderJob::ReadMore() {
   DCHECK(!pending_write_.get());
 
@@ skipping 53 matching lines @@
     request_complete_data.completion_time = base::TimeTicks::Now();
     request_complete_data.encoded_body_length =
         is_range_request() ? range_response_info_->headers->GetContentLength()
                            : info_->response_data_size();
     request_complete_data.decoded_body_length =
         request_complete_data.encoded_body_length;
   }
   client_info_->OnComplete(request_complete_data);
 }
 
+mojom::URLLoaderClientPtr AppCacheURLLoaderJob::CreateLoaderClientProxy() {

[michaeln, 2017/07/18 19:19:22]

Consider moving the body of this method directly into the
DeliverNetworkResponse() method. I think there is no other callsite and hoisting
into that method would make the relationship to the network_loader more clear.

Some readbility/naming nits:

Its not obvious that we're setting up 'client' of the network_loader here,
consider network_loader_client_binding_.

About the name 'network_loader_request_', for all practical purposes, this is
the network_loader from appcache job's point of view. I don't think having
'request' in the name helps and I think stripping request from this name would
be more consistent with how mojom interface ptrs are named elsewhere in the
project.

I put naming nit suggestions for a handful of data members in the .h file, wdyt?

[ananta, 2017/07/18 19:52:40]

Thanks. done.

+  mojom::URLLoaderClientPtr client_ptr;
+  loader_client_proxy_binding_.Bind(mojo::MakeRequest(&client_ptr));
+  return client_ptr;
+}
+
 }  // namespace content