Have WorkerScriptLoader::loadAsynchronously() take a FetchOptions
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 8 Apr 2018 06:43:24 +0000 (06:43 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 8 Apr 2018 06:43:24 +0000 (06:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184385

Reviewed by Youenn Fablet.

Currently we pass various FetchOptions to WorkerScriptLoader::loadAsynchronously()
so that it can build up a ThreadableLoaderOptions structure to pass to the loader.
Each time we want to set another FetchOptions option we need to add a new parameter.
Instead we should have WorkerScriptLoader::loadAsynchronously() take a FetchOptions.
This will make it straightforward for a caller to set new loader options as needed.
In particular, this will make it straightforward to support setting the request's
destination flag (i.e. FetchOptions::destination) to support blocking scripts with
a non-JavaScript MIME type in a subsequent commit.

No functionality changed. So, no new tests.

* loader/ResourceLoaderOptions.h:
(WebCore::ResourceLoaderOptions::ResourceLoaderOptions): Modified to take a FetchOptions
by value so as to support both move and copy semantics.
* loader/ThreadableLoader.cpp:
(WebCore::ThreadableLoaderOptions::ThreadableLoaderOptions):  Added helper constructor
that takes a FetchOptions.
* loader/ThreadableLoader.h:
* workers/Worker.cpp:
(WebCore::Worker::create): Instantiate and pass a FetchOptions to the loader for the mode,
cache policy, and redirect policy.
* workers/WorkerScriptLoader.cpp:
(WebCore::WorkerScriptLoader::loadAsynchronously): Modified to take a FetchOptions and
instantiate a ThreadableLoaderOptions from it.
* workers/WorkerScriptLoader.h:
* workers/WorkerScriptLoaderClient.h:
(WebCore::WorkerScriptLoaderClient::isServiceWorkerClient const): Deleted. This function
is no longer needed because the Service Worker client now passes the service worker mode
directly to the loader.
* workers/service/ServiceWorkerJob.cpp:
(WebCore::ServiceWorkerJob::fetchScriptWithContext): Instantiate and pass a FetchOptions
to the loader.
* workers/service/ServiceWorkerJob.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230374 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/loader/ResourceLoaderOptions.h
Source/WebCore/loader/ThreadableLoader.cpp
Source/WebCore/loader/ThreadableLoader.h
Source/WebCore/workers/Worker.cpp
Source/WebCore/workers/WorkerScriptLoader.cpp
Source/WebCore/workers/WorkerScriptLoader.h
Source/WebCore/workers/WorkerScriptLoaderClient.h
Source/WebCore/workers/service/ServiceWorkerJob.cpp
Source/WebCore/workers/service/ServiceWorkerJob.h

index a15f7a9..2be2136 100644 (file)
@@ -1,3 +1,44 @@
+2018-04-07  Daniel Bates  <dabates@apple.com>
+
+        Have WorkerScriptLoader::loadAsynchronously() take a FetchOptions
+        https://bugs.webkit.org/show_bug.cgi?id=184385
+
+        Reviewed by Youenn Fablet.
+
+        Currently we pass various FetchOptions to WorkerScriptLoader::loadAsynchronously()
+        so that it can build up a ThreadableLoaderOptions structure to pass to the loader.
+        Each time we want to set another FetchOptions option we need to add a new parameter.
+        Instead we should have WorkerScriptLoader::loadAsynchronously() take a FetchOptions.
+        This will make it straightforward for a caller to set new loader options as needed.
+        In particular, this will make it straightforward to support setting the request's
+        destination flag (i.e. FetchOptions::destination) to support blocking scripts with
+        a non-JavaScript MIME type in a subsequent commit.
+
+        No functionality changed. So, no new tests.
+
+        * loader/ResourceLoaderOptions.h:
+        (WebCore::ResourceLoaderOptions::ResourceLoaderOptions): Modified to take a FetchOptions
+        by value so as to support both move and copy semantics.
+        * loader/ThreadableLoader.cpp:
+        (WebCore::ThreadableLoaderOptions::ThreadableLoaderOptions):  Added helper constructor
+        that takes a FetchOptions.
+        * loader/ThreadableLoader.h:
+        * workers/Worker.cpp:
+        (WebCore::Worker::create): Instantiate and pass a FetchOptions to the loader for the mode,
+        cache policy, and redirect policy.
+        * workers/WorkerScriptLoader.cpp:
+        (WebCore::WorkerScriptLoader::loadAsynchronously): Modified to take a FetchOptions and
+        instantiate a ThreadableLoaderOptions from it.
+        * workers/WorkerScriptLoader.h:
+        * workers/WorkerScriptLoaderClient.h:
+        (WebCore::WorkerScriptLoaderClient::isServiceWorkerClient const): Deleted. This function
+        is no longer needed because the Service Worker client now passes the service worker mode
+        directly to the loader.
+        * workers/service/ServiceWorkerJob.cpp:
+        (WebCore::ServiceWorkerJob::fetchScriptWithContext): Instantiate and pass a FetchOptions
+        to the loader.
+        * workers/service/ServiceWorkerJob.h:
+
 2018-04-07  Timothy Hatcher  <timothy@apple.com>
 
         Use the system's link color when system appearance is desired for a WebView.
index 251ab92..3d9a1a3 100644 (file)
@@ -114,7 +114,7 @@ enum class ContentEncodingSniffingPolicy {
 struct ResourceLoaderOptions : public FetchOptions {
     ResourceLoaderOptions() { }
 
-    ResourceLoaderOptions(const FetchOptions& options) : FetchOptions(options) { }
+    ResourceLoaderOptions(FetchOptions options) : FetchOptions { WTFMove(options) } { }
 
     ResourceLoaderOptions(SendCallbackPolicy sendLoadCallbacks, ContentSniffingPolicy sniffContent, DataBufferingPolicy dataBufferingPolicy, StoredCredentialsPolicy storedCredentialsPolicy, ClientCredentialPolicy credentialPolicy, FetchOptions::Credentials credentials, SecurityCheckPolicy securityCheck, FetchOptions::Mode mode, CertificateInfoPolicy certificateInfoPolicy, ContentSecurityPolicyImposition contentSecurityPolicyImposition, DefersLoadingPolicy defersLoadingPolicy, CachingPolicy cachingPolicy)
         : sendLoadCallbacks(sendLoadCallbacks)
index aed7b14..ef90eab 100644 (file)
@@ -49,6 +49,11 @@ ThreadableLoaderOptions::ThreadableLoaderOptions()
 
 ThreadableLoaderOptions::~ThreadableLoaderOptions() = default;
 
+ThreadableLoaderOptions::ThreadableLoaderOptions(FetchOptions&& baseOptions)
+    : ResourceLoaderOptions { WTFMove(baseOptions) }
+{
+}
+
 ThreadableLoaderOptions::ThreadableLoaderOptions(const ResourceLoaderOptions& baseOptions, PreflightPolicy preflightPolicy, ContentSecurityPolicyEnforcement contentSecurityPolicyEnforcement, String&& initiator, ResponseFilteringPolicy filteringPolicy)
     : ResourceLoaderOptions(baseOptions)
     , preflightPolicy(preflightPolicy)
index 30b21cf..421e39c 100644 (file)
@@ -63,6 +63,7 @@ namespace WebCore {
 
     struct ThreadableLoaderOptions : ResourceLoaderOptions {
         ThreadableLoaderOptions();
+        explicit ThreadableLoaderOptions(FetchOptions&&);
         ThreadableLoaderOptions(const ResourceLoaderOptions&, PreflightPolicy, ContentSecurityPolicyEnforcement, String&& initiator, ResponseFilteringPolicy);
         ~ThreadableLoaderOptions();
 
index 49d8320..a92565c 100644 (file)
@@ -102,7 +102,12 @@ ExceptionOr<Ref<Worker>> Worker::create(ScriptExecutionContext& context, JSC::Ru
 
     ResourceRequest request { scriptURL.releaseReturnValue() };
     request.setInitiatorIdentifier(worker->m_identifier);
-    worker->m_scriptLoader->loadAsynchronously(context, WTFMove(request), FetchOptions::Mode::SameOrigin, FetchOptions::Cache::Default, FetchOptions::Redirect::Follow, contentSecurityPolicyEnforcement, worker);
+
+    FetchOptions options;
+    options.mode = FetchOptions::Mode::SameOrigin;
+    options.cache = FetchOptions::Cache::Default;
+    options.redirect = FetchOptions::Redirect::Follow;
+    worker->m_scriptLoader->loadAsynchronously(context, WTFMove(request), WTFMove(options), contentSecurityPolicyEnforcement, ServiceWorkersMode::All, worker);
     return WTFMove(worker);
 }
 
index 00e82c8..82f3051 100644 (file)
@@ -73,7 +73,7 @@ void WorkerScriptLoader::loadSynchronously(ScriptExecutionContext* scriptExecuti
     WorkerThreadableLoader::loadResourceSynchronously(workerGlobalScope, WTFMove(*request), *this, options);
 }
 
-void WorkerScriptLoader::loadAsynchronously(ScriptExecutionContext& scriptExecutionContext, ResourceRequest&& scriptRequest, FetchOptions::Mode mode, FetchOptions::Cache cachePolicy, FetchOptions::Redirect redirectPolicy, ContentSecurityPolicyEnforcement contentSecurityPolicyEnforcement, WorkerScriptLoaderClient& client)
+void WorkerScriptLoader::loadAsynchronously(ScriptExecutionContext& scriptExecutionContext, ResourceRequest&& scriptRequest, FetchOptions&& fetchOptions, ContentSecurityPolicyEnforcement contentSecurityPolicyEnforcement, ServiceWorkersMode serviceWorkerMode, WorkerScriptLoaderClient& client)
 {
     m_client = &client;
     m_url = scriptRequest.url();
@@ -86,20 +86,19 @@ void WorkerScriptLoader::loadAsynchronously(ScriptExecutionContext& scriptExecut
 
     // Only used for loading worker scripts in classic mode.
     // FIXME: We should add an option to set credential mode.
-    ASSERT(mode == FetchOptions::Mode::SameOrigin);
+    ASSERT(fetchOptions.mode == FetchOptions::Mode::SameOrigin);
 
-    ThreadableLoaderOptions options;
+    ThreadableLoaderOptions options { WTFMove(fetchOptions) };
     options.credentials = FetchOptions::Credentials::SameOrigin;
-    options.mode = mode;
-    options.cache = cachePolicy;
-    options.redirect = redirectPolicy;
     options.sendLoadCallbacks = SendCallbacks;
     options.contentSecurityPolicyEnforcement = contentSecurityPolicyEnforcement;
+    // A service worker job can be executed from a worker context or a document context.
+    options.serviceWorkersMode = serviceWorkerMode;
 #if ENABLE(SERVICE_WORKER)
-    options.serviceWorkersMode = m_client->isServiceWorkerClient() ? ServiceWorkersMode::None : ServiceWorkersMode::All;
     if (auto* activeServiceWorker = scriptExecutionContext.activeServiceWorker())
         options.serviceWorkerRegistrationIdentifier = activeServiceWorker->registrationIdentifier();
 #endif
+
     // During create, callbacks may happen which remove the last reference to this object.
     Ref<WorkerScriptLoader> protectedThis(*this);
     m_threadableLoader = ThreadableLoader::create(scriptExecutionContext, *this, WTFMove(*request), options);
index a00fc57..4a52bf7 100644 (file)
@@ -54,7 +54,7 @@ public:
     }
 
     void loadSynchronously(ScriptExecutionContext*, const URL&, FetchOptions::Mode, FetchOptions::Cache, ContentSecurityPolicyEnforcement, const String& initiatorIdentifier);
-    void loadAsynchronously(ScriptExecutionContext&, ResourceRequest&&, FetchOptions::Mode, FetchOptions::Cache, FetchOptions::Redirect, ContentSecurityPolicyEnforcement, WorkerScriptLoaderClient&);
+    void loadAsynchronously(ScriptExecutionContext&, ResourceRequest&&, FetchOptions&&, ContentSecurityPolicyEnforcement, ServiceWorkersMode, WorkerScriptLoaderClient&);
 
     void notifyError();
 
index 722909b..274ce48 100644 (file)
@@ -34,7 +34,6 @@ class WorkerScriptLoaderClient {
 public:
     virtual void didReceiveResponse(unsigned long identifier, const ResourceResponse&) = 0;
     virtual void notifyFinished() = 0;
-    virtual bool isServiceWorkerClient() const { return false; }
 
 protected:
     virtual ~WorkerScriptLoaderClient() = default;
index 7606985..1ad2d30 100644 (file)
@@ -98,7 +98,11 @@ void ServiceWorkerJob::fetchScriptWithContext(ScriptExecutionContext& context, F
     request.setInitiatorIdentifier("serviceWorkerScriptLoad:");
     request.addHTTPHeaderField(ASCIILiteral("Service-Worker"), ASCIILiteral("script"));
 
-    m_scriptLoader->loadAsynchronously(context, WTFMove(request), FetchOptions::Mode::SameOrigin, cachePolicy, FetchOptions::Redirect::Error, ContentSecurityPolicyEnforcement::DoNotEnforce, *this);
+    FetchOptions options;
+    options.mode = FetchOptions::Mode::SameOrigin;
+    options.cache = cachePolicy;
+    options.redirect = FetchOptions::Redirect::Error;
+    m_scriptLoader->loadAsynchronously(context, WTFMove(request), WTFMove(options), ContentSecurityPolicyEnforcement::DoNotEnforce, ServiceWorkersMode::None, *this);
 }
 
 void ServiceWorkerJob::didReceiveResponse(unsigned long, const ResourceResponse& response)
index ee5daaf..0abd219 100644 (file)
@@ -78,8 +78,6 @@ private:
     // WorkerScriptLoaderClient
     void didReceiveResponse(unsigned long identifier, const ResourceResponse&) final;
     void notifyFinished() final;
-    bool isServiceWorkerClient() const final { return true; }
-
 
     Ref<ServiceWorkerJobClient> m_client;
     ServiceWorkerJobData m_jobData;