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

content/browser/appcache/appcache_url_loader_job.cc

Index: content/browser/appcache/appcache_url_loader_job.cc
diff --git a/content/browser/appcache/appcache_url_loader_job.cc b/content/browser/appcache/appcache_url_loader_job.cc
index 1d1932f7aa137d40dff546ab3ca0b9a31351ed4b..a08bc623371f03ec315fe91dff2078a0163b0400 100644
--- a/content/browser/appcache/appcache_url_loader_job.cc
+++ b/content/browser/appcache/appcache_url_loader_job.cc
@@ -7,6 +7,7 @@
 #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"
@@ -82,7 +83,7 @@ void AppCacheURLLoaderJob::DeliverNetworkResponse() {
             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);
   }
 }
@@ -130,6 +131,69 @@ void AppCacheURLLoaderJob::SetPriority(net::RequestPriority priority,
     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));
+  }
+}
+
+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);
+  }
+}
+
+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))

[michaeln, 2017/07/17 21:04:55]

is this one really needed, does the code in OnReceiveResponse cover this case

[ananta, 2017/07/18 00:19:03]

It is needed. We don't receive the OnReceiveResponse() call reliably. For e.g.
no network

[michaeln, 2017/07/18 01:40:36]

What happens to the network_loader when we do invoke a fallback on
OnReceiveResponse or OnReceiveRedirect. How does the network_load get killed and
do we get an OnComplete(CANCELLED) notification out of it?

Do we need to delete, or otherwise kill, the network_loader inside of
DeliverAppCachedResponse or DeliverErrorResponse if/when those methods are
called while processing OnReceiveResponse or OnReceiveRedirect.

[ananta, 2017/07/18 02:06:49]

We should not be receiving those calls as they are on the same thread. I added
code to close the mojo binding handle to the loader if we create a fallback job.

[ananta, 2017/07/18 02:11:39]

Thanks for catching this. I was only testing the case when there was no network.
Looking at the code there are cases where fallback could be delivered even for
valid responses.

+    client_info_->OnComplete(status);
+}
+
 void AppCacheURLLoaderJob::SetSubresourceLoadInfo(
     std::unique_ptr<SubresourceLoadInfo> subresource_load_info,
     URLLoaderFactoryGetter* default_url_loader) {
@@ -159,8 +223,10 @@ void AppCacheURLLoaderJob::Start(mojom::URLLoaderRequest request,
     SendResponseInfo();
 }
 
-AppCacheURLLoaderJob::AppCacheURLLoaderJob(const ResourceRequest& request,
-                                           AppCacheStorage* storage)
+AppCacheURLLoaderJob::AppCacheURLLoaderJob(
+    const ResourceRequest& request,
+    AppCacheURLLoaderRequest* url_loader_request,
+    AppCacheStorage* storage)
     : request_(request),
       storage_(storage->GetWeakPtr()),
       start_time_tick_(base::TimeTicks::Now()),
@@ -168,7 +234,9 @@ AppCacheURLLoaderJob::AppCacheURLLoaderJob(const ResourceRequest& request,
       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} {}

[michaeln, 2017/07/17 21:04:55]

are the brackets needed, can we use parens here?

[ananta, 2017/07/18 00:19:03]

eek. typo. Fixed

 
 void AppCacheURLLoaderJob::OnResponseInfoLoaded(
     AppCacheResponseInfo* response_info,
@@ -293,6 +361,8 @@ void AppCacheURLLoaderJob::SendResponseInfo() {
 
   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());
 
@@ -366,4 +436,14 @@ void AppCacheURLLoaderJob::NotifyCompleted(int error_code) {
   client_info_->OnComplete(request_complete_data);
 }
 
+mojom::URLLoaderClientPtr AppCacheURLLoaderJob::CreateLoaderClientProxy() {
+  mojom::URLLoaderClientPtr client_ptr;
+  loader_client_proxy_binding_.Bind(mojo::MakeRequest(&client_ptr));
+
+  loader_client_proxy_binding_.set_connection_error_handler(base::Bind(
+      &AppCacheURLLoaderJob::OnConnectionError, StaticAsWeakPtr(this)));
+
+  return client_ptr;
+}
+
 }  // namespace content