Service Worker: Change last update check location and HTTP cache bypass rule (2/2)

content/browser/service_worker/service_worker_write_to_cache_job_unittest.cc

 // Copyright 2014 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 "base/location.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/thread_task_runner_handle.h"
 #include "content/browser/fileapi/mock_url_request_delegate.h"
 #include "content/browser/service_worker/embedded_worker_test_helper.h"
@@ skipping 42 matching lines @@
 net::URLRequestJob* CreateNormalURLRequestJob(
     net::URLRequest* request,
     net::NetworkDelegate* network_delegate) {
   return new net::URLRequestTestJob(request,
                                     network_delegate,
                                     std::string(kHeaders, arraysize(kHeaders)),
                                     kScriptCode,
                                     true);
 }
 
-net::URLRequestJob* CreateResponseJob(const std::string& response_data,
-                                      net::URLRequest* request,
-                                      net::NetworkDelegate* network_delegate) {
+net::URLRequestJob* CreateCachedResponseJob(
+    const std::string& response_data,
+    net::URLRequest* request,
+    net::NetworkDelegate* network_delegate) {
   return new net::URLRequestTestJob(request, network_delegate,
                                     std::string(kHeaders, arraysize(kHeaders)),
                                     response_data, true);
 }
 
+class NetworkResponseJob : public net::URLRequestTestJob {
+ public:
+  NetworkResponseJob(net::URLRequest* request,
+                     net::NetworkDelegate* network_delegate,
+                     const std::string& response_headers,
+                     const std::string& response_data,
+                     bool auto_advance)
+      : net::URLRequestTestJob(request,
+                               network_delegate,
+                               response_headers,
+                               response_data,
+                               auto_advance) {}
+  void GetResponseInfo(net::HttpResponseInfo* info) override {
+    URLRequestTestJob::GetResponseInfo(info);
+    info->network_accessed = true;
+  }
+
+ protected:
+  ~NetworkResponseJob() override {}
+};
+
+net::URLRequestJob* CreateNetworkResponseJob(
+    const std::string& response_data,
+    net::URLRequest* request,
+    net::NetworkDelegate* network_delegate) {
+  return new NetworkResponseJob(request, network_delegate,
+                                std::string(kHeaders, arraysize(kHeaders)),
+                                response_data, true);
+}
+
 net::URLRequestJob* CreateInvalidMimeTypeJob(
     net::URLRequest* request,
     net::NetworkDelegate* network_delegate) {
   const char kPlainTextHeaders[] =
       "HTTP/1.1 200 OK\0"
       "Content-Type: text/plain\0"
       "Expires: Thu, 1 Jan 2100 20:00:00 GMT\0"
       "\0";
   return new net::URLRequestTestJob(
       request,
       network_delegate,
       std::string(kPlainTextHeaders, arraysize(kPlainTextHeaders)),
       kScriptCode,
       true);
 }
 
+net::URLRequestJob* CreateErrorReponseJob(
+    net::URLRequest* request,
+    net::NetworkDelegate* network_delegate) {
+  const char kErrorHeaders[] = "HTTP/1.1 404 Not Found\0\0";
+  return new net::URLRequestTestJob(
+      request, network_delegate,
+      std::string(kErrorHeaders, arraysize(kErrorHeaders)), kScriptCode, true);
+}
+
 class SSLCertificateErrorJob : public net::URLRequestTestJob {
  public:
   SSLCertificateErrorJob(net::URLRequest* request,
                          net::NetworkDelegate* network_delegate,
                          const std::string& response_headers,
                          const std::string& response_data,
                          bool auto_advance)
       : net::URLRequestTestJob(request,
                                network_delegate,
                                response_headers,
@@ skipping 229 matching lines @@
     mock_protocol_handler_ = nullptr;
     version_ = nullptr;
     registration_ = nullptr;
     helper_.reset();
     // URLRequestJobs may post clean-up tasks on destruction.
     base::RunLoop().RunUntilIdle();
   }
 
   int CreateIncumbent(const std::string& response) {
     mock_protocol_handler_->SetCreateJobCallback(
-        base::Bind(&CreateResponseJob, response));
+        base::Bind(&CreateNetworkResponseJob, response));
     request_->Start();
     base::RunLoop().RunUntilIdle();
     EXPECT_EQ(net::URLRequestStatus::SUCCESS, request_->status().status());
     int incumbent_resource_id =
         version_->script_cache_map()->LookupResourceId(script_url_);
     EXPECT_NE(kInvalidServiceWorkerResponseId, incumbent_resource_id);
 
     registration_->SetActiveVersion(version_);
 
     // Teardown the request.
     request_.reset();
     url_request_context_.reset();
     url_request_job_factory_.reset();
     mock_protocol_handler_ = nullptr;
     base::RunLoop().RunUntilIdle();
 
     return incumbent_resource_id;
   }
 
   int GetResourceId(ServiceWorkerVersion* version) {
     return version->script_cache_map()->LookupResourceId(script_url_);
   }
 
   // Performs the net request for an update of |registration_|'s incumbent
   // to the script |response|. Returns the new version.
   scoped_refptr<ServiceWorkerVersion> UpdateScript(
-      const std::string& response) {
+      const std::string& response,
+      bool status_success = true,
+      bool network_accessed = true) {

[nhiroki, 2015/09/09 08:27:48]

nit: Passing multiple booleans could be error-prone. How about passing a
callback to be set in |mock_protocol_handler_| instead?

  UpdateScript(base::Bind(&CreateCachedResponseJob, response));

Or, creating separate functions?

  scoped_refptr<SWV> UpdateScriptWithNetworkResponse(response) {
    mock_protocol_handler_->SetCreateJobCallback(
        base::Bind(&CreateNetworkResponseJob, response));
    UpdateScriptInternal();
  }

  scoped_refptr<SWV> UpdateScriptWithCachedResponse(response) {
    mock_protocol_handler_->SetCreateJobCallback(
        base::Bind(&CreateCachedResponseJob, response));
    UpdateScriptInternal();
  }

  scoped_refptr<SWV> UpdateScriptWithErrorResponse(response) {
    mock_protocol_handler_->SetCreateJobCallback(
        base::Bind(&CreateErrorResponseJob, response));
    UpdateScriptInternal();
  }

[jungkees, 2015/09/09 13:13:37]

Agreed. I did it with the passing a callback option. I think it improves the
readability too.

     int render_process_id = NextRenderProcessId();
     int provider_id = NextProviderId();
     scoped_refptr<ServiceWorkerVersion> new_version =
         new ServiceWorkerVersion(registration_.get(), script_url_,
                                  NextVersionId(), context()->AsWeakPtr());
     CreateHostForVersion(render_process_id, provider_id, new_version);
 
     SetUpScriptRequest(render_process_id, provider_id);
-    mock_protocol_handler_->SetCreateJobCallback(
-        base::Bind(&CreateResponseJob, response));
+    if (status_success) {
+      // Whether the script load request accessed the network or was served from
+      // HTTPCache.
+      if (network_accessed) {
+        mock_protocol_handler_->SetCreateJobCallback(
+            base::Bind(&CreateNetworkResponseJob, response));
+      } else {
+        mock_protocol_handler_->SetCreateJobCallback(
+            base::Bind(&CreateCachedResponseJob, response));
+      }
+    } else {
+      mock_protocol_handler_->SetCreateJobCallback(
+          base::Bind(&CreateErrorReponseJob));
+    }
     request_->Start();
     base::RunLoop().RunUntilIdle();
     return new_version;
   }
 
   void VerifyResource(int id, const std::string& expected) {
     ASSERT_NE(kInvalidServiceWorkerResourceId, id);
     bool is_equal = false;
     scoped_ptr<ServiceWorkerResponseReader> reader =
         context()->storage()->CreateResponseReader(id);
@@ skipping 182 matching lines @@
   // Update from non-empty to empty.
   version = UpdateScript(std::string());
   VerifyResource(GetResourceId(version.get()), std::string());
   registration_->SetWaitingVersion(version);
 
   // Update from empty to empty.
   version = UpdateScript(std::string());
   EXPECT_EQ(kInvalidServiceWorkerResponseId, GetResourceId(version.get()));
 }
 
+TEST_F(ServiceWorkerWriteToCacheJobTest, Update_BumpLastUpdateCheckTime) {
+  std::string response = GenerateLongResponse();
+  CreateIncumbent(response);
+
+  // Script was served from HTTPCache.
+  base::Time current = base::Time::Now();
+  registration_->set_last_update_check(current);
+  scoped_refptr<ServiceWorkerVersion> version = UpdateScript(
+      response, true /* status_success */, false /* network_accessed */);
+  EXPECT_EQ(current, registration_->last_update_check());
+
+  // Same script but the request accessed the network.
+  version = UpdateScript(response);
+  EXPECT_LT(current, registration_->last_update_check());
+
+  // Changed the first byte and the request accessed the network.
+  response[0] = 'x';
+  current = base::Time::Now();
+  registration_->set_last_update_check(current);
+  version = UpdateScript(response);
+  VerifyResource(GetResourceId(version.get()), response);
+  registration_->SetWaitingVersion(version);
+  EXPECT_LT(current, registration_->last_update_check());
+
+  // The request accessed the network but received a non-2xx error.
+  current = base::Time::Now();
+  registration_->set_last_update_check(current);
+  version = UpdateScript(response, false /* status_success */);
+  EXPECT_EQ(current, registration_->last_update_check());
+}
+
 }  // namespace content