Changeset 182866 in webkit

Timestamp:

Apr 15, 2015, 3:56:56 PM (11 years ago)

Author:

ap@apple.com

Message:

No thread safety when passing ThreadableLoaderOptions from a worker thread
​https://bugs.webkit.org/show_bug.cgi?id=143790

Reviewed by Geoffrey Garen.

• loader/ThreadableLoader.h:
• loader/ThreadableLoader.cpp: (WebCore::ThreadableLoaderOptions::isolatedCopy): Added.

• loader/WorkerThreadableLoader.cpp:

(WebCore::WorkerThreadableLoader::MainThreadBridge::MainThreadBridge): Don't just send
a structure with strings to a different thread, that's bad.

• platform/CrossThreadCopier.h: I think that this is dead code, but for this bug,

just removing a clearly wrong specialization.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/ThreadableLoader.cpp (modified) (1 diff)
• loader/ThreadableLoader.h (modified) (1 diff)
• loader/WorkerThreadableLoader.cpp (modified) (2 diffs)
• platform/CrossThreadCopier.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-04-15  Alexey Proskuryakov  <ap@apple.com>
+
+        No thread safety when passing ThreadableLoaderOptions from a worker thread
+        https://bugs.webkit.org/show_bug.cgi?id=143790
+
+        Reviewed by Geoffrey Garen.
+
+        * loader/ThreadableLoader.h:
+        * loader/ThreadableLoader.cpp: (WebCore::ThreadableLoaderOptions::isolatedCopy): Added.
+
+        * loader/WorkerThreadableLoader.cpp:
+        (WebCore::WorkerThreadableLoader::MainThreadBridge::MainThreadBridge): Don't just send
+        a structure with strings to a different thread, that's bad.
+
+        * platform/CrossThreadCopier.h: I think that this is dead code, but for this bug,
+        just removing a clearly wrong specialization.
+
 2015-04-15  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WebCore/loader/ThreadableLoader.cpp

@@ -52 +52 @@
 }
 
+std::unique_ptr<ThreadableLoaderOptions> ThreadableLoaderOptions::isolatedCopy() const
+{
+    std::unique_ptr<ThreadableLoaderOptions> copy = std::make_unique<ThreadableLoaderOptions>();
+    copy->preflightPolicy = preflightPolicy;
+    copy->crossOriginRequestPolicy = crossOriginRequestPolicy;
+    if (securityOrigin)
+        copy->securityOrigin = securityOrigin->isolatedCopy();
+    copy->initiator = initiator.string().isolatedCopy();
+    return copy;
+}
+
 PassRefPtr<ThreadableLoader> ThreadableLoader::create(ScriptExecutionContext* context, ThreadableLoaderClient* client, const ResourceRequest& request, const ThreadableLoaderOptions& options)
 {

trunk/Source/WebCore/loader/ThreadableLoader.h

@@ -64 +64 @@
         ~ThreadableLoaderOptions();
 
+        std::unique_ptr<ThreadableLoaderOptions> isolatedCopy() const;
+
         PreflightPolicy preflightPolicy; // If AccessControl is used, how to determine if a preflight is needed.
         CrossOriginRequestPolicy crossOriginRequestPolicy;

trunk/Source/WebCore/loader/WorkerThreadableLoader.cpp

@@ -92 +92 @@
 
     auto* requestData = request.copyData().release();
+    auto* optionsCopy = options.isolatedCopy().release();
     StringCapture capturedOutgoingReferrer(outgoingReferrer);
-    m_loaderProxy.postTaskToLoader([this, requestData, options, capturedOutgoingReferrer](ScriptExecutionContext& context) {
+    m_loaderProxy.postTaskToLoader([this, requestData, optionsCopy, capturedOutgoingReferrer](ScriptExecutionContext& context) {
         ASSERT(isMainThread());
         Document& document = downcast<Document>(context);
@@ -99 +100 @@
         auto request = ResourceRequest::adopt(std::unique_ptr<CrossThreadResourceRequestData>(requestData));
         request->setHTTPReferrer(capturedOutgoingReferrer.string());
+
+        auto options = std::unique_ptr<ThreadableLoaderOptions>(optionsCopy);
 
         // FIXME: If the a site requests a local resource, then this will return a non-zero value but the sync path
         // will return a 0 value. Either this should return 0 or the other code path should do a callback with
         // a failure.
-        m_mainThreadLoader = DocumentThreadableLoader::create(document, *this, *request, options);
+        m_mainThreadLoader = DocumentThreadableLoader::create(document, *this, *request, *options);
         ASSERT(m_mainThreadLoader);
     });

trunk/Source/WebCore/platform/CrossThreadCopier.h

@@ -93 +93 @@
     // To allow a type to be passed across threads using its copy constructor, add a forward declaration of the type and
     // a CopyThreadCopierBase<false, false, TypeName> : public CrossThreadCopierPassThrough<TypeName> { }; to this file.
-    template<> struct CrossThreadCopierBase<false, false, ThreadableLoaderOptions> : public CrossThreadCopierPassThrough<ThreadableLoaderOptions> {
-    };
-
     template<> struct CrossThreadCopierBase<false, false, IntRect> : public CrossThreadCopierPassThrough<IntRect> {
     };