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

content/browser/appcache/appcache_request_handler.cc

Index: content/browser/appcache/appcache_request_handler.cc
diff --git a/content/browser/appcache/appcache_request_handler.cc b/content/browser/appcache/appcache_request_handler.cc
index 652eff24a7acf59b8edd790ac25b29a544e3b5f2..236b6ea5a26091f7f134fc78f1cfa122a5e1c584 100644
--- a/content/browser/appcache/appcache_request_handler.cc
+++ b/content/browser/appcache/appcache_request_handler.cc
@@ -127,19 +127,18 @@ AppCacheJob* AppCacheRequestHandler::MaybeLoadFallbackForRedirect(
   if (request_->GetURL().GetOrigin() == location.GetOrigin())
     return NULL;
 
-  DCHECK(!job_.get());  // our jobs never generate redirects
-
   std::unique_ptr<AppCacheJob> job;
+
   if (found_fallback_entry_.has_response_id()) {
+    job = MaybeCreateJobForFallback(network_delegate);
     // 6.9.6, step 4: If this results in a redirect to another origin,
     // get the resource of the fallback entry.
-    job = CreateJob(network_delegate);
     DeliverAppCachedResponse(found_fallback_entry_, found_cache_id_,
                              found_manifest_url_, true,
                              found_namespace_entry_url_);
   } else if (!found_network_namespace_) {
     // 6.9.6, step 6: Fail the resource load.
-    job = CreateJob(network_delegate);
+    job = MaybeCreateJobForFallback(network_delegate);
     DeliverErrorResponse();
   } else {
     // 6.9.6 step 3 and 5: Fetch the resource normally.
@@ -163,7 +162,7 @@ AppCacheJob* AppCacheRequestHandler::MaybeLoadFallbackForResponse(
   }
 
   // We don't fallback for responses that we delivered.
-  if (job_.get()) {
+  if (job_.get() && !base::FeatureList::IsEnabled(features::kNetworkService)) {
     DCHECK(!job_->IsDeliveringNetworkResponse());
     return NULL;
   }
@@ -186,7 +185,12 @@ AppCacheJob* AppCacheRequestHandler::MaybeLoadFallbackForResponse(
 
   // 6.9.6, step 4: If this results in a 4xx or 5xx status code
   // or there were network errors, get the resource of the fallback entry.
-  std::unique_ptr<AppCacheJob> job = CreateJob(network_delegate);
+
+  // In network service land, the job initiates a fallback request. We reuse
+  // the existing job to deliver the fallback response.
+  std::unique_ptr<AppCacheJob> job = MaybeCreateJobForFallback(
+      network_delegate);
+
   DeliverAppCachedResponse(found_fallback_entry_, found_cache_id_,
                            found_manifest_url_, true,
                            found_namespace_entry_url_);
@@ -342,6 +346,20 @@ std::unique_ptr<AppCacheJob> AppCacheRequestHandler::CreateJob(
   return job;
 }
 
+std::unique_ptr<AppCacheJob> AppCacheRequestHandler::MaybeCreateJobForFallback(
+    net::NetworkDelegate* network_delegate) {
+  std::unique_ptr<AppCacheJob> job;
+  // In network service land, the job initiates a fallback request. We reuse
+  // the existing job to deliver the fallback response.
+  if (!base::FeatureList::IsEnabled(features::kNetworkService)) {
+    job = CreateJob(network_delegate);

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

maybe rearrange, early return, for readiability, the comment applies more to the
else clause than the if

// We create a new job with the net::URLRequest impl.
if (!networkServiceEnabled)
  return CreateJob();

// But reuse the existing job with the networkService impl.
DCHECK(job.get());
return job.get();

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

Done.

+  } else {
+    DCHECK(!job_.get());  // our jobs never generate redirects

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

should this check for not-nullness?

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

Yes. Done

+    job.reset(job_.get());
+  }
+  return job;
+}
+
 // Main-resource handling ----------------------------------------------
 
 std::unique_ptr<AppCacheJob> AppCacheRequestHandler::MaybeLoadMainResource(