ServiceWorker: Skip byte-for-byte comparison when a script type is

changed.

Some tests in update-registration-with-type.https.html fail because
a new worker is not created when a script type is changed and a script
content is identical. This CL makes |skip_script_comparison_| true
when a script type is same. Also, I replaced has_installed_version()
newest_installed_version().

Bug: 824647
Change-Id: I755e3d031b97d6659a7a35a1f93f9acfc654368c
Reviewed-on: https://chromium-review.googlesource.com/c/1312828
Commit-Queue: Asami Doi <asamidoi@google.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604849}

content/browser/service_worker/service_worker_register_job.cc

@@ -403,12 +403,6 @@ void ServiceWorkerRegisterJob::UpdateAndContinue() {
     return;
   }
 
-  // "Let worker be a new ServiceWorker object..." and start
-  // the worker.
-  set_new_version(new ServiceWorkerVersion(
-      registration(), script_url_, worker_script_type_, version_id, context_));
-  new_version()->set_force_bypass_cache_for_scripts(force_bypass_cache_);
-
   // Module service workers don't support pause after download so we can't
   // perform script comparison.
   // TODO(asamidoi): Support pause after download in module workers.
@@ -416,7 +410,19 @@ void ServiceWorkerRegisterJob::UpdateAndContinue() {
     skip_script_comparison_ = true;
   }
 
-  if (registration()->has_installed_version() && !skip_script_comparison_) {
+  // Skip the byte-for-byte comparison when the script type is updated.
+  if (registration()->newest_installed_version() &&
+      registration()->newest_installed_version()->script_type() !=
+          worker_script_type_) {
+    skip_script_comparison_ = true;
+  }
+
+  // "Let worker be a new ServiceWorker object..." and start the worker.
+  set_new_version(new ServiceWorkerVersion(
+      registration(), script_url_, worker_script_type_, version_id, context_));
+  new_version()->set_force_bypass_cache_for_scripts(force_bypass_cache_);
+
+  if (registration()->newest_installed_version() && !skip_script_comparison_) {
     new_version()->SetToPauseAfterDownload(
         base::BindOnce(&ServiceWorkerRegisterJob::OnPausedAfterDownload,
                        weak_factory_.GetWeakPtr()));
@@ -628,7 +634,7 @@ void ServiceWorkerRegisterJob::CompleteInternal(
   if (registration()) {
     context_->storage()->NotifyDoneInstallingRegistration(
         registration(), new_version(), status);
-    if (registration()->has_installed_version())
+    if (registration()->newest_installed_version())
       registration()->set_is_uninstalled(false);
   }
 }
@@ -705,7 +711,7 @@ void ServiceWorkerRegisterJob::BumpLastUpdateCheckTimeIfNeeded() {
       registration()->last_update_check().is_null()) {
     registration()->set_last_update_check(base::Time::Now());
 
-    if (registration()->has_installed_version())
+    if (registration()->newest_installed_version())
       context_->storage()->UpdateLastUpdateCheckTime(registration());
   }
 }

content/browser/service_worker/service_worker_registration.h

@@ -106,8 +106,8 @@ class CONTENT_EXPORT ServiceWorkerRegistration
     return installing_version_.get();
   }
 
-  bool has_installed_version() const {
-    return active_version() || waiting_version();
+  ServiceWorkerVersion* newest_installed_version() const {
+    return waiting_version() ? waiting_version() : active_version();
   }
 
   const blink::mojom::NavigationPreloadState navigation_preload_state() const {

third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/update-registration-with-type.https-expected.txt

@@ -2,9 +2,9 @@ This is a testharness.js-based test.
 PASS Update the registration with a different script type (classic => module).
 PASS Update the registration with a different script type (module => classic).
 PASS Update the registration with a different script type (classic => module) and with a same main script.
-FAIL Update the registration with a different script type (module => classic) and with a same main script. promise_test: Unhandled rejection with value: object "Error: wait_for_state must be passed a ServiceWorker"
+PASS Update the registration with a different script type (module => classic) and with a same main script.
 PASS Does not update the registration with the same script type and the same main script.
 FAIL Update the registration with a different script type (classic => module) and with a same main script. Expect evaluation failed. assert_unreached: Should have rejected: Registering with invalid evaluation should be failed. Reached unreachable code
-FAIL Update the registration with a different script type (module => classic) and with a same main script. Expect evaluation failed. assert_unreached: Should have rejected: Registering with invalid evaluation should be failed. Reached unreachable code
+PASS Update the registration with a different script type (module => classic) and with a same main script. Expect evaluation failed.
 Harness: the test ran to completion.
 

third_party/blink/renderer/core/workers/worker_module_tree_client.cc

@@ -41,6 +41,9 @@ void WorkerModuleTreeClient::NotifyModuleTreeLoadFinished(
   // asynchronous completion, with script being the asynchronous completion
   // value."
   worker_reporting_proxy.WillEvaluateModuleScript();
+  // This |error| is always null because the second argument is |kReport|.
+  // TODO(nhiroki): Catch an error when an evaluation error happens.
+  // (https://crbug.com/680046)
   ScriptValue error = modulator_->ExecuteModule(
       module_script, Modulator::CaptureEvalErrorFlag::kReport);
   worker_reporting_proxy.DidEvaluateModuleScript(error.IsEmpty());