Use caller's document url to resolve scriptURL/patternURL in registerServiceWorker/getRegistration

Source/modules/serviceworkers/ServiceWorkerContainer.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 99 matching lines @@
 {
     ASSERT(RuntimeEnabledFeatures::serviceWorkerEnabled());
     RefPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = resolver->promise();
 
     if (!m_provider) {
         resolver->reject(DOMException::create(InvalidStateError, "The document is in an invalid state."));
         return promise;
     }
 
-    // FIXME: This should use the container's execution context, not
-    // the callers.
     ExecutionContext* executionContext = scriptState->executionContext();

[dominicc (has gone to gerrit), 2015/02/09 07:31:48]

I'm still not sure this is the right context. Which context is this? For example
if in frame A you do

frameB.contentWindow.ServiceWorkerContainer.prototype.register.call(frameC.navigator.serviceWorker,
...)

which one is used? (I think that the one we want is "C", BTW.)

[jungkees, 2015/02/13 08:12:37]

Having double-checked it, it's basically the container's context (not the
caller's context). I.e. for
frameB.contentWindow.navigator.serviceWorker.register(), frameB's context is
being used.

For the JS function's call method case as in your example, frameB's context is
also used, though. The ScriptState object (related to the context) is set to the
isolate's current context in the auto generated V8 binding code like the
following: 

ScriptState* scriptState = ScriptState::current(info.GetIsolate());
ScriptPromise result = impl->registerServiceWorker(scriptState, url, options);

If frameC's context is expected, do you think we better file a separate issue
for this?

     RefPtr<SecurityOrigin> documentOrigin = executionContext->securityOrigin();
     String errorMessage;
     if (!documentOrigin->canAccessFeatureRequiringSecureOrigin(errorMessage)) {
         resolver->reject(DOMException::create(NotSupportedError, errorMessage));
         return promise;
     }
 
     KURL pageURL = KURL(KURL(), documentOrigin->toString());
     if (!pageURL.protocolIsInHTTPFamily()) {
         resolver->reject(DOMException::create(SecurityError, "The URL protocol of the current origin is not supported: " + pageURL.protocol()));
         return promise;
     }
 
-    KURL patternURL = executionContext->completeURL(options.scope());
+    Document* callingDocument = callingDOMWindow(scriptState->isolate())->document();
+
+    KURL patternURL = callingDocument ? callingDocument->completeURL(options.scope()) : executionContext->completeURL(options.scope());

[dominicc (has gone to gerrit), 2015/02/09 07:31:48]

I don't think this in right; I think if from frame A we do

frameB.contentWindow.navigator.serviceWorker.register

then the URL should be resolved against frame B's location. For example look at
how LocalDOMWindow::open (which corresponds to Window.open) uses the entered
document to complete the URL, not the calling document. I'd be surprised if
register worked differently, but maybe you can point me to specs? Am I missing
something?

[jungkees, 2015/02/13 08:12:38]

This is not register specific, but HTML specifies this. I think this pointer
explains:
https://html.spec.whatwg.org/multipage/webappapis.html#entry-settings-object.
See the example snippet just below the definition of entry settings object and
incumbent settings objects.

[dominicc (has gone to gerrit), 2015/02/16 06:54:45]

On 2015/02/13 at 08:12:38, jungkees wrote: 
Thanks for the link, that does look like the definitive thing.

Where is the spec for using the current execution context when the calling
document is null? What's the precedent for that?

[jungkees, 2015/02/16 09:06:10]

My intention with checking whether the callingDocument be null was just to set a
guard to not dereference the value when null is returned. So did I assume the
callingDocument in this code refers to the iframe's document when it exists. If
a document doesn't have any iframe and the method is called withing the context,
shouldn't it use the current execution context by default? Please let me know if
I'm missing or overlooking any basic stuff here.

     patternURL.removeFragmentIdentifier();
     if (!documentOrigin->canRequest(patternURL)) {
         resolver->reject(DOMException::create(SecurityError, "The scope must match the current origin."));
         return promise;
     }
 
-    KURL scriptURL = executionContext->completeURL(url);
+    KURL scriptURL = callingDocument ? callingDocument->completeURL(url) : executionContext->completeURL(url);
     scriptURL.removeFragmentIdentifier();
     if (!documentOrigin->canRequest(scriptURL)) {
         resolver->reject(DOMException::create(SecurityError, "The origin of the script must match the current origin."));
         return promise;
     }
 
     if (!patternURL.string().startsWith(scriptURL.baseAsString())) {
         resolver->reject(DOMException::create(SecurityError, "The scope must be under the directory of the script URL."));
         return promise;
     }
@@ skipping 20 matching lines @@
 {
     ASSERT(RuntimeEnabledFeatures::serviceWorkerEnabled());
     RefPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = resolver->promise();
 
     if (!m_provider) {
         resolver->reject(DOMException::create(InvalidStateError, "The document is in an invalid state."));
         return promise;
     }
 
-    // FIXME: This should use the container's execution context, not
-    // the callers.
     ExecutionContext* executionContext = scriptState->executionContext();
     RefPtr<SecurityOrigin> documentOrigin = executionContext->securityOrigin();
     String errorMessage;
     if (!documentOrigin->canAccessFeatureRequiringSecureOrigin(errorMessage)) {
         resolver->reject(DOMException::create(NotSupportedError, errorMessage));
         return promise;
     }
 
     KURL pageURL = KURL(KURL(), documentOrigin->toString());
     if (!pageURL.protocolIsInHTTPFamily()) {
         resolver->reject(DOMException::create(SecurityError, "The URL protocol of the current origin is not supported: " + pageURL.protocol()));
         return promise;
     }
 
-    KURL completedURL = executionContext->completeURL(documentURL);
+    Document* callingDocument = callingDOMWindow(scriptState->isolate())->document();
+    KURL completedURL = callingDocument ? callingDocument->completeURL(documentURL) : executionContext->completeURL(documentURL);
     completedURL.removeFragmentIdentifier();
     if (!documentOrigin->canRequest(completedURL)) {
         resolver->reject(DOMException::create(SecurityError, "The documentURL must match the current origin."));
         return promise;
     }
     m_provider->getRegistration(completedURL, new GetRegistrationCallback(resolver));
 
     return promise;
 }
 
@@ skipping 101 matching lines @@
     m_ready = createReadyProperty();
 
     if (ServiceWorkerContainerClient* client = ServiceWorkerContainerClient::from(executionContext)) {
         m_provider = client->provider();
         if (m_provider)
             m_provider->setClient(this);
     }
 }
 
 } // namespace blink