ServiceWorker: Change base URL for parsing script URL and scope URL

According to the spec change, this changes to use the relevant settings object instead of the entry settings object to parse the script URL and the scope URL given to ServiceWorkerContainer.register() and ServiceWorkerContainer.getRegistration(). Before this CL, register() and getRegistration() used entered execution context to parse the URLs. After this CL, the methods use the execution context retrieved from ScriptState::getExecutionContext(). WPT: external/wpt/service-workers/service-worker/multi-globals/url-parsing.https.html Spec issue: w3c/ServiceWorker#922 Spec change: w3c/ServiceWorker@ec1aac2 BUG=691008 Review-Url: https://codereview.chromium.org/2691903005 Cr-Commit-Position: refs/heads/master@{#453033}
mirror · Feb 25, 2017 · 255f794 · 255f794
1 parent f40832e
commit 255f794
Show file tree

Hide file tree

Showing 13 changed files with 48 additions and 185 deletions.
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations
@@ -2139,7 +2139,6 @@ crbug.com/626703 external/wpt/streams/writable-streams/close.dedicatedworker.htm
 crbug.com/626703 external/wpt/streams/writable-streams/close.html [ Timeout ]
 crbug.com/626703 external/wpt/streams/writable-streams/close.serviceworker.https.html [ Timeout ]
 crbug.com/626703 external/wpt/service-workers/service-worker/registration-useCache.https.html [ Timeout ]
-crbug.com/626703 external/wpt/service-workers/service-worker/multi-globals/url-parsing.https.html [ Pass Failure ]
 crbug.com/626703 external/wpt/streams/writable-streams/close.sharedworker.html [ Timeout ]
 crbug.com/626703 external/wpt/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/allow-scripts-flag-changing-2.html [ Timeout ]
 crbug.com/626703 external/wpt/html/semantics/embedded-content/the-iframe-element/cross_origin_parentage.html [ Timeout ]

diff --git a/...utTests/external/wpt/service-workers/service-worker/controller-on-load.https-expected.txt b/...utTests/external/wpt/service-workers/service-worker/controller-on-load.https-expected.txt
diff --git a/...Tests/external/wpt/service-workers/service-worker/controller-on-reload.https-expected.txt b/...Tests/external/wpt/service-workers/service-worker/controller-on-reload.https-expected.txt
diff --git a/...t/LayoutTests/external/wpt/service-workers/service-worker/controller-on-reload.https.html b/...t/LayoutTests/external/wpt/service-workers/service-worker/controller-on-reload.https.html
@@ -6,7 +6,8 @@
 <body>
 <script>
 promise_test(function(t) {
-    var scope = 'resources/blank.html';
+    const iframe_scope = 'blank.html';
+    const scope = 'resources/' + iframe_scope;
     var frame;
     var registration;
     var controller;
@@ -17,7 +18,7 @@
       .then(function(f) {
           frame = f;
           return frame.contentWindow.navigator.serviceWorker.register(
-              'resources/empty-worker.js', {scope: scope});
+              'empty-worker.js', {scope: iframe_scope});
         })
       .then(function(swr) {
           registration = swr;
@@ -42,7 +43,7 @@
           // objects from separate windows should not be equal
           assert_not_equals(controller, registration.active);
 
-          return w.navigator.serviceWorker.getRegistration();
+          return w.navigator.serviceWorker.getRegistration(iframe_scope);
         })
       .then(function(frameRegistration) {
           assert_equals(frameRegistration.active, controller);

diff --git a/.../external/wpt/service-workers/service-worker/multi-globals/url-parsing.https-expected.txt b/.../external/wpt/service-workers/service-worker/multi-globals/url-parsing.https-expected.txt
diff --git a/...bKit/LayoutTests/external/wpt/service-workers/service-worker/multiple-register.https.html b/...bKit/LayoutTests/external/wpt/service-workers/service-worker/multiple-register.https.html
@@ -45,7 +45,8 @@
     .then(function(f) {
         frame = f;
         return frame.contentWindow.navigator.serviceWorker.register(
-            worker_url, { scope: scope });
+            'empty-worker.js',
+            { scope: 'scope/subsequent-register-from-different-iframe' });
       })
     .then(function(new_registration) {
         assert_not_equals(

diff --git a/...it/LayoutTests/external/wpt/service-workers/service-worker/registration-iframe.https.html b/...it/LayoutTests/external/wpt/service-workers/service-worker/registration-iframe.https.html
@@ -5,15 +5,15 @@
 <script src="/resources/testharnessreport.js"></script>
 <script src="resources/test-helpers.sub.js"></script>
 <script>
-// Set script url and scope url relative to the calling frame's document's url.
-// Assert the implementation parses the urls against the calling frame's
-// document's url.
+
+// Set script url and scope url relative to the iframe's document's url. Assert
+// the implementation parses the urls against the iframe's document's url.
 async_test(function(t) {
   var url = 'resources/blank.html';
-  var scope = 'resources/registration-for-iframe-from-calling-frame';
-  var parsed_scope = normalizeURL(scope);
-  var script = 'resources/empty-worker.js';
-  var parsed_script = normalizeURL(script);
+  var scope = 'registration-for-iframe-from-parent-frame';
+  var expected_scope = normalizeURL('resources/' + scope);
+  var script = 'empty-worker.js';
+  var expected_script = normalizeURL('resources/' + script);
   var frame;
   var registration;
 
@@ -30,28 +30,25 @@
         return wait_for_state(t, r.installing, 'activated');
       })
     .then(function() {
-        assert_equals(
-          registration.scope, parsed_scope,
-          'registration\'s scope must be the scope parsed against calling ' +
-          'document\'s url');
-        assert_equals(
-          registration.active.scriptURL, parsed_script,
-          'worker\'s script must be the url parsed against calling ' +
-          'document\'s url');
+        assert_equals(registration.scope, expected_scope,
+                      'registration\'s scope must be parsed against the ' +
+                      '"relevant global object"');
+        assert_equals(registration.active.scriptURL, expected_script,
+                      'worker\'s scriptURL must be parsed against the ' +
+                      '"relevant global object"');
         frame.remove();
         return service_worker_unregister_and_done(t, scope);
       })
     .catch(unreached_rejection(t));
-  }, 'Subframe\'s container\'s register method should use calling frame\'s ' +
-     'document\'s url as a base url for parsing its script url and scope url ' +
-     '- normal case');
+  }, 'register method should use the "relevant global object" to parse its ' +
+     'scriptURL and scope - normal case');
 
-// Set script url and scope url relative to the iframe's document's url.
-// Assert the implementation throws a NetworkError exception.
+// Set script url and scope url relative to the parent frame's document's url.
+// Assert the implementation throws a TypeError exception.
 async_test(function(t) {
   var url = 'resources/blank.html';
-  var scope = 'registration-for-iframe-from-calling-frame';
-  var script = 'empty-worker.js';
+  var scope = 'resources/registration-for-iframe-from-parent-frame';
+  var script = 'resources/empty-worker.js';
   var frame;
   var registration;
 
@@ -68,21 +65,22 @@
         assert_unreached('register() should reject');
       },
       function(e) {
-        assert_equals(e.name, 'TypeError');
+        assert_equals(e.name, 'TypeError',
+                      'register method with scriptURL and scope parsed to ' +
+                      'nonexistent location should reject with TypeError');
         frame.remove();
         return service_worker_unregister_and_done(t, scope);
       })
     .catch(unreached_rejection(t));
-  }, 'Subframe\'s container\'s register method should use calling frame\'s ' +
-     'document\'s url as a base url for parsing its script url and scope url ' +
-     '- error case');
+  }, 'register method should use the "relevant global object" to parse its ' +
+     'scriptURL and scope - error case');
 
-// Set the scope url to a non-subdirectory of the script url.
-// Assert the implementation throws a SecurityError exception.
+// Set the scope url to a non-subdirectory of the script url. Assert the
+// implementation throws a SecurityError exception.
 async_test(function(t) {
   var url = 'resources/blank.html';
-  var scope = 'registration-for-iframe-from-calling-frame';
-  var script = 'resources/empty-worker.js';
+  var scope = '../registration-for-iframe-from-parent-frame';
+  var script = 'empty-worker.js';
   var frame;
   var registration;
 
@@ -99,10 +97,13 @@
         assert_unreached('register() should reject');
       },
       function(e) {
-        assert_equals(e.name, 'SecurityError');
+        assert_equals(e.name, 'SecurityError',
+                      'The scope set to a non-subdirectory of the scriptURL ' +
+                      'should reject with SecurityError');
         frame.remove();
         return service_worker_unregister_and_done(t, scope);
       })
     .catch(unreached_rejection(t));
   }, 'A scope url should start with the given script url');
+
 </script>
diff --git a/third_party/WebKit/LayoutTests/http/tests/background_sync/oneshot.html b/third_party/WebKit/LayoutTests/http/tests/background_sync/oneshot.html
@@ -9,10 +9,10 @@
 <script>
 
 promise_test(function(t) {
-  var url = 'resources/empty_worker.js';
-  var scope = 'resources/scope/background_sync/oneshot.html';
+  const url = 'resources/empty_worker.js';
+  const iframe_scope = 'oneshot.html';
+  const scope = 'resources/scope/background_sync/' + iframe_scope;
   var sync_manager;
-  var sync_registration;
 
   // This test verifies that registration of one-shots fails from an iframe.
   return PermissionsHelper.setPermission('background-sync', 'granted')
@@ -27,7 +27,7 @@
       })
     .then(function(frame) {
         var w = frame.contentWindow;
-        return w.navigator.serviceWorker.getRegistration(scope);
+        return w.navigator.serviceWorker.getRegistration(iframe_scope);
       })
     .then(function(sw_registration_frame) {
         sync_manager = sw_registration_frame.sync;
@@ -45,10 +45,9 @@
      'from an iframe');
 
 promise_test(function(t) {
-  var url = 'resources/empty_worker.js';
-  var scope = 'resources/scope/background_sync/oneshot-uncontrolled.html';
+  const url = 'resources/empty_worker.js';
+  const scope = 'resources/scope/background_sync/oneshot-uncontrolled.html';
   var sync_manager;
-  var sync_registration;
 
   // This test verifies that one-shot syncs can be registered from uncontrolled
   // documents.

diff --git a/third_party/WebKit/LayoutTests/http/tests/serviceworker/controller-on-reload.html b/third_party/WebKit/LayoutTests/http/tests/serviceworker/controller-on-reload.html
@@ -15,7 +15,7 @@
       .then(function(f) {
           frame = f;
           return frame.contentWindow.navigator.serviceWorker.register(
-              'resources/empty-worker.js', {scope: scope});
+              'empty-worker.js', {scope: 'blank.html'});
         })
       .then(function(registration) {
           return wait_for_state(t, registration.installing, 'activated');

diff --git a/third_party/WebKit/LayoutTests/http/tests/serviceworker/registration-iframe.html b/third_party/WebKit/LayoutTests/http/tests/serviceworker/registration-iframe.html
diff --git a/third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp b/third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp
@@ -770,18 +770,6 @@ ExecutionContext* currentExecutionContext(v8::Isolate* isolate) {
   return toExecutionContext(isolate->GetCurrentContext());
 }
 
-ExecutionContext* enteredExecutionContext(v8::Isolate* isolate) {
-  ExecutionContext* context = toExecutionContext(isolate->GetEnteredContext());
-  if (!context) {
-    // We don't always have an entered execution context, for example during
-    // microtask callbacks from V8 (where the entered context may be the
-    // DOM-in-JS context). In that case, we fall back to the current context.
-    context = currentExecutionContext(isolate);
-    ASSERT(context);
-  }
-  return context;
-}
-
 Frame* toFrameIfNotDetached(v8::Local<v8::Context> context) {
   DOMWindow* window = toDOMWindow(context);
   if (window && window->isCurrentlyDisplayedInFrame())

diff --git a/third_party/WebKit/Source/bindings/core/v8/V8Binding.h b/third_party/WebKit/Source/bindings/core/v8/V8Binding.h
@@ -976,7 +976,6 @@ CORE_EXPORT ExecutionContext* toExecutionContext(v8::Local<v8::Context>);
 CORE_EXPORT void registerToExecutionContextForModules(
     ExecutionContext* (*toExecutionContextForModules)(v8::Local<v8::Context>));
 CORE_EXPORT ExecutionContext* currentExecutionContext(v8::Isolate*);
-CORE_EXPORT ExecutionContext* enteredExecutionContext(v8::Isolate*);
 
 // Returns a V8 context associated with a ExecutionContext and a
 // DOMWrapperWorld.  This method returns an empty context if there is no frame

diff --git a/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerContainer.cpp b/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerContainer.cpp
@@ -282,16 +282,14 @@ ScriptPromise ServiceWorkerContainer::registerServiceWorker(
   if (!executionContext)
     return ScriptPromise();
 
-  KURL scriptURL =
-      enteredExecutionContext(scriptState->isolate())->completeURL(url);
+  KURL scriptURL = executionContext->completeURL(url);
   scriptURL.removeFragmentIdentifier();
 
   KURL patternURL;
   if (options.scope().isNull())
     patternURL = KURL(scriptURL, "./");
   else
-    patternURL = enteredExecutionContext(scriptState->isolate())
-                     ->completeURL(options.scope());
+    patternURL = executionContext->completeURL(options.scope());
 
   registerServiceWorkerImpl(
       executionContext, scriptURL, patternURL,
@@ -339,8 +337,7 @@ ScriptPromise ServiceWorkerContainer::getRegistration(
     return promise;
   }
 
-  KURL completedURL =
-      enteredExecutionContext(scriptState->isolate())->completeURL(documentURL);
+  KURL completedURL = executionContext->completeURL(documentURL);
   completedURL.removeFragmentIdentifier();
   if (!documentOrigin->canRequest(completedURL)) {
     RefPtr<SecurityOrigin> documentURLOrigin =