isValidCSSSelector is unsafe to be called from a non-main thread
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Aug 2018 00:30:50 +0000 (00:30 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Aug 2018 00:30:50 +0000 (00:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188581
<rdar://problem/40517358>

Reviewed by Sam Weinig.

Source/WebCore:

Parsing and determining whether the css selectors are valid is fast enough to do before
hopping to the background thread for the slow NFA/DFA operations and writing to disk.
Doing it on the main thread avoids the thread safety issues in the CSSParser's use of strings.

* contentextensions/ContentExtensionCompiler.cpp:
(WebCore::ContentExtensions::compileRuleList):
* contentextensions/ContentExtensionCompiler.h:
* contentextensions/ContentExtensionParser.cpp:
(WebCore::ContentExtensions::isValidCSSSelector):
(WebCore::ContentExtensions::loadEncodedRules):
(WebCore::ContentExtensions::parseRuleList):
* contentextensions/ContentExtensionParser.h:
* contentextensions/ContentExtensionRule.cpp:
(WebCore::ContentExtensions::Trigger::isolatedCopy const):
(WebCore::ContentExtensions::Action::isolatedCopy const):
* contentextensions/ContentExtensionRule.h:
(WebCore::ContentExtensions::Trigger::isEmpty const):
(WebCore::ContentExtensions::Trigger::operator== const):
(WebCore::ContentExtensions::Action::Action):
(WebCore::ContentExtensions::ContentExtensionRule::isolatedCopy const):
(WebCore::ContentExtensions::ContentExtensionRule::operator== const):
(WebCore::ContentExtensions::vectorIsolatedCopy):

Source/WebKit:

* UIProcess/API/APIContentRuleListStore.cpp:
(API::compiledToFile):
(API::ContentRuleListStore::lookupContentRuleList):
(API::ContentRuleListStore::getAvailableContentRuleListIdentifiers):
(API::ContentRuleListStore::compileContentRuleList):
(API::ContentRuleListStore::removeContentRuleList):
(API::ContentRuleListStore::getContentRuleListSource):
* UIProcess/API/APIContentRuleListStore.h:
* UIProcess/API/Cocoa/WKContentRuleListStore.mm:

Source/WTF:

* wtf/Vector.h:
(WTF::minCapacity>::isolatedCopy):

Tools:

* TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp:
(TestWebKitAPI::InMemoryCompiledContentExtension::create):
(TestWebKitAPI::checkCompilerError):

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

15 files changed:
Source/WTF/ChangeLog
Source/WTF/wtf/Vector.h
Source/WebCore/ChangeLog
Source/WebCore/contentextensions/ContentExtensionCompiler.cpp
Source/WebCore/contentextensions/ContentExtensionCompiler.h
Source/WebCore/contentextensions/ContentExtensionParser.cpp
Source/WebCore/contentextensions/ContentExtensionParser.h
Source/WebCore/contentextensions/ContentExtensionRule.cpp
Source/WebCore/contentextensions/ContentExtensionRule.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp
Source/WebKit/UIProcess/API/APIContentRuleListStore.h
Source/WebKit/UIProcess/API/Cocoa/WKContentRuleListStore.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp

index 546638f..f556927 100644 (file)
@@ -1,5 +1,16 @@
 2018-08-14  Alex Christensen  <achristensen@webkit.org>
 
+        isValidCSSSelector is unsafe to be called from a non-main thread
+        https://bugs.webkit.org/show_bug.cgi?id=188581
+        <rdar://problem/40517358>
+
+        Reviewed by Sam Weinig.
+
+        * wtf/Vector.h:
+        (WTF::minCapacity>::isolatedCopy):
+
+2018-08-14  Alex Christensen  <achristensen@webkit.org>
+
         Use a Variant instead of a union in CSSSelector
         https://bugs.webkit.org/show_bug.cgi?id=188559
 
index 306f462..85184b9 100644 (file)
@@ -766,6 +766,8 @@ public:
 
     void clear() { shrinkCapacity(0); }
 
+    template<typename U = T> Vector<U> isolatedCopy() const;
+
     ALWAYS_INLINE void append(ValueType&& value) { append<ValueType>(std::forward<ValueType>(value)); }
     template<typename U> void append(U&&);
     template<typename... Args> void constructAndAppend(Args&&...);
@@ -1604,6 +1606,17 @@ template<typename T> struct ValueCheck<Vector<T>> {
 };
 #endif
 
+template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity>
+template<typename U>
+inline Vector<U> Vector<T, inlineCapacity, OverflowHandler, minCapacity>::isolatedCopy() const
+{
+    Vector<U> copy;
+    copy.reserveInitialCapacity(size());
+    for (const auto& element : *this)
+        copy.uncheckedAppend(element.isolatedCopy());
+    return copy;
+}
+    
 template<typename VectorType, typename Func>
 size_t removeRepeatedElements(VectorType& vector, const Func& func)
 {
index e35e1d4..f79159f 100644 (file)
@@ -1,3 +1,34 @@
+2018-08-14  Alex Christensen  <achristensen@webkit.org>
+
+        isValidCSSSelector is unsafe to be called from a non-main thread
+        https://bugs.webkit.org/show_bug.cgi?id=188581
+        <rdar://problem/40517358>
+
+        Reviewed by Sam Weinig.
+
+        Parsing and determining whether the css selectors are valid is fast enough to do before
+        hopping to the background thread for the slow NFA/DFA operations and writing to disk.
+        Doing it on the main thread avoids the thread safety issues in the CSSParser's use of strings.
+
+        * contentextensions/ContentExtensionCompiler.cpp:
+        (WebCore::ContentExtensions::compileRuleList):
+        * contentextensions/ContentExtensionCompiler.h:
+        * contentextensions/ContentExtensionParser.cpp:
+        (WebCore::ContentExtensions::isValidCSSSelector):
+        (WebCore::ContentExtensions::loadEncodedRules):
+        (WebCore::ContentExtensions::parseRuleList):
+        * contentextensions/ContentExtensionParser.h:
+        * contentextensions/ContentExtensionRule.cpp:
+        (WebCore::ContentExtensions::Trigger::isolatedCopy const):
+        (WebCore::ContentExtensions::Action::isolatedCopy const):
+        * contentextensions/ContentExtensionRule.h:
+        (WebCore::ContentExtensions::Trigger::isEmpty const):
+        (WebCore::ContentExtensions::Trigger::operator== const):
+        (WebCore::ContentExtensions::Action::Action):
+        (WebCore::ContentExtensions::ContentExtensionRule::isolatedCopy const):
+        (WebCore::ContentExtensions::ContentExtensionRule::operator== const):
+        (WebCore::ContentExtensions::vectorIsolatedCopy):
+
 2018-08-14  Ansh Shukla  <ansh_shukla@apple.com>
 
         NSURLAuthenticationMethodOAuth challenges are surfaced to clients in -didReceiveAuthenticationChallenge as NSURLAuthenticationMethodDefault
index 5e035a6..35a72dc 100644 (file)
@@ -283,12 +283,13 @@ static void compileToBytecode(CombinedURLFilters&& filters, UniversalActionSet&&
     LOG_LARGE_STRUCTURES(universalActions, universalActions.capacity() * sizeof(unsigned));
 }
 
-std::error_code compileRuleList(ContentExtensionCompilationClient& client, String&& ruleJSON)
+std::error_code compileRuleList(ContentExtensionCompilationClient& client, String&& ruleJSON, Vector<ContentExtensionRule>&& parsedRuleList)
 {
-    auto ruleList = parseRuleList(WTFMove(ruleJSON));
-    if (!ruleList.has_value())
-        return ruleList.error();
-    Vector<ContentExtensionRule> parsedRuleList = WTFMove(ruleList.value());
+#if !ASSERT_DISABLED
+    callOnMainThread([ruleJSON = ruleJSON.isolatedCopy(), parsedRuleList = parsedRuleList.isolatedCopy()] {
+        ASSERT(parseRuleList(ruleJSON) == parsedRuleList);
+    });
+#endif
 
     bool domainConditionSeen = false;
     bool topURLConditionSeen = false;
@@ -313,7 +314,7 @@ std::error_code compileRuleList(ContentExtensionCompilationClient& client, Strin
     MonotonicTime patternPartitioningStart = MonotonicTime::now();
 #endif
     
-    client.writeSource(ruleJSON);
+    client.writeSource(std::exchange(ruleJSON, String()));
 
     Vector<SerializedActionByte> actions;
     Vector<unsigned> actionLocations = serializeActions(parsedRuleList, actions);
index 5b811c9..eb3296a 100644 (file)
@@ -40,7 +40,7 @@ public:
     virtual ~ContentExtensionCompilationClient() = default;
     
     // Functions should be called in this order. All except writeActions and finalize can be called multiple times, though.
-    virtual void writeSource(const String&) = 0;
+    virtual void writeSource(String&&) = 0;
     virtual void writeActions(Vector<SerializedActionByte>&&, bool conditionsApplyOnlyToDomain) = 0;
     virtual void writeFiltersWithoutConditionsBytecode(Vector<DFABytecode>&&) = 0;
     virtual void writeFiltersWithConditionsBytecode(Vector<DFABytecode>&&) = 0;
@@ -48,7 +48,7 @@ public:
     virtual void finalize() = 0;
 };
 
-WEBCORE_EXPORT std::error_code compileRuleList(ContentExtensionCompilationClient&, String&&);
+WEBCORE_EXPORT std::error_code compileRuleList(ContentExtensionCompilationClient&, String&& ruleJSON, Vector<ContentExtensionRule>&&);
 
 } // namespace ContentExtensions
 } // namespace WebCore
index 77e3b4f..928c37e 100644 (file)
@@ -230,6 +230,7 @@ static Expected<Trigger, std::error_code> loadTrigger(ExecState& exec, const JSO
 
 bool isValidCSSSelector(const String& selector)
 {
+    ASSERT(isMainThread());
     AtomicString::init();
     QualifiedName::init();
     CSSParserContext context(HTMLQuirksMode);
@@ -299,7 +300,7 @@ static Expected<std::optional<ContentExtensionRule>, std::error_code> loadRule(E
     return { std::nullopt };
 }
 
-static Expected<Vector<ContentExtensionRule>, std::error_code> loadEncodedRules(ExecState& exec, String&& ruleJSON)
+static Expected<Vector<ContentExtensionRule>, std::error_code> loadEncodedRules(ExecState& exec, const String& ruleJSON)
 {
     VM& vm = exec.vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
@@ -347,7 +348,7 @@ static Expected<Vector<ContentExtensionRule>, std::error_code> loadEncodedRules(
     return WTFMove(ruleList);
 }
 
-Expected<Vector<ContentExtensionRule>, std::error_code> parseRuleList(String&& ruleJSON)
+Expected<Vector<ContentExtensionRule>, std::error_code> parseRuleList(const String& ruleJSON)
 {
 #if CONTENT_EXTENSIONS_PERFORMANCE_REPORTING
     MonotonicTime loadExtensionStartTime = MonotonicTime::now();
@@ -358,7 +359,7 @@ Expected<Vector<ContentExtensionRule>, std::error_code> parseRuleList(String&& r
     JSGlobalObject* globalObject = JSGlobalObject::create(*vm, JSGlobalObject::createStructure(*vm, jsNull()));
 
     ExecState* exec = globalObject->globalExec();
-    auto ruleList = loadEncodedRules(*exec, WTFMove(ruleJSON));
+    auto ruleList = loadEncodedRules(*exec, ruleJSON);
 
     vm = nullptr;
 
index dc3809d..f1134b6 100644 (file)
@@ -38,7 +38,7 @@ namespace ContentExtensions {
 
 class ContentExtensionRule;
 
-Expected<Vector<ContentExtensionRule>, std::error_code> parseRuleList(String&&);
+WEBCORE_EXPORT Expected<Vector<ContentExtensionRule>, std::error_code> parseRuleList(const String&);
 WEBCORE_EXPORT bool isValidCSSSelector(const String&);
 
 } // namespace ContentExtensions
index 145b6f2..ddc2258 100644 (file)
@@ -113,6 +113,28 @@ uint32_t Action::serializedLength(const SerializedActionByte* actions, const uin
     RELEASE_ASSERT_NOT_REACHED();
 }
 
+Trigger Trigger::isolatedCopy() const
+{
+    return {
+        urlFilter.isolatedCopy(),
+        urlFilterIsCaseSensitive,
+        topURLConditionIsCaseSensitive,
+        flags,
+        conditions.isolatedCopy(),
+        conditionType
+    };
+}
+
+Action Action::isolatedCopy() const
+{
+    return {
+        m_extensionIdentifier.isolatedCopy(),
+        m_type,
+        m_actionID,
+        m_stringArgument.isolatedCopy()
+    };
+}
+
 } // namespace ContentExtensions
 
 } // namespace WebCore
index c686748..cebf13e 100644 (file)
@@ -55,6 +55,8 @@ struct Trigger {
     };
     ConditionType conditionType { ConditionType::None };
 
+    WEBCORE_EXPORT Trigger isolatedCopy() const;
+    
     ~Trigger()
     {
         ASSERT(conditions.isEmpty() == (conditionType == ConditionType::None));
@@ -66,6 +68,7 @@ struct Trigger {
     {
         return urlFilter.isEmpty()
             && !urlFilterIsCaseSensitive
+            && !topURLConditionIsCaseSensitive
             && !flags
             && conditions.isEmpty()
             && conditionType == ConditionType::None;
@@ -75,6 +78,7 @@ struct Trigger {
     {
         return urlFilter == other.urlFilter
             && urlFilterIsCaseSensitive == other.urlFilterIsCaseSensitive
+            && topURLConditionIsCaseSensitive == other.topURLConditionIsCaseSensitive
             && flags == other.flags
             && conditions == other.conditions
             && conditionType == other.conditionType;
@@ -163,7 +167,16 @@ struct Action {
     uint32_t actionID() const { return m_actionID; }
     const String& stringArgument() const { return m_stringArgument; }
 
+    WEBCORE_EXPORT Action isolatedCopy() const;
+    
 private:
+    Action(String&& extensionIdentifier, ActionType type, uint32_t actionID, String&& stringArgument)
+        : m_extensionIdentifier(WTFMove(extensionIdentifier))
+        , m_type(type)
+        , m_actionID(actionID)
+        , m_stringArgument(WTFMove(stringArgument))
+    { }
+
     String m_extensionIdentifier;
     ActionType m_type;
     uint32_t m_actionID;
@@ -172,11 +185,20 @@ private:
     
 class ContentExtensionRule {
 public:
-    ContentExtensionRule(Trigger&&, Action&&);
+    WEBCORE_EXPORT ContentExtensionRule(Trigger&&, Action&&);
 
     const Trigger& trigger() const { return m_trigger; }
     const Action& action() const { return m_action; }
 
+    ContentExtensionRule isolatedCopy() const
+    {
+        return { m_trigger.isolatedCopy(), m_action.isolatedCopy() };
+    }
+    bool operator==(const ContentExtensionRule& other) const
+    {
+        return m_trigger == other.m_trigger && m_action == other.m_action;
+    }
+
 private:
     Trigger m_trigger;
     Action m_action;
index a8ff3d6..1bf23d4 100644 (file)
@@ -1,3 +1,21 @@
+2018-08-14  Alex Christensen  <achristensen@webkit.org>
+
+        isValidCSSSelector is unsafe to be called from a non-main thread
+        https://bugs.webkit.org/show_bug.cgi?id=188581
+        <rdar://problem/40517358>
+
+        Reviewed by Sam Weinig.
+
+        * UIProcess/API/APIContentRuleListStore.cpp:
+        (API::compiledToFile):
+        (API::ContentRuleListStore::lookupContentRuleList):
+        (API::ContentRuleListStore::getAvailableContentRuleListIdentifiers):
+        (API::ContentRuleListStore::compileContentRuleList):
+        (API::ContentRuleListStore::removeContentRuleList):
+        (API::ContentRuleListStore::getContentRuleListSource):
+        * UIProcess/API/APIContentRuleListStore.h:
+        * UIProcess/API/Cocoa/WKContentRuleListStore.mm:
+
 2018-08-14  Ansh Shukla  <ansh_shukla@apple.com>
 
         NSURLAuthenticationMethodOAuth challenges are surfaced to clients in -didReceiveAuthenticationChallenge as NSURLAuthenticationMethodDefault
index 5d6ea1b..3dcfcb3 100644 (file)
 #include "WebCompiledContentRuleList.h"
 #include <WebCore/ContentExtensionCompiler.h>
 #include <WebCore/ContentExtensionError.h>
+#include <WebCore/ContentExtensionParser.h>
 #include <WebCore/QualifiedName.h>
 #include <string>
+#include <wtf/CompletionHandler.h>
 #include <wtf/NeverDestroyed.h>
 #include <wtf/RunLoop.h>
 #include <wtf/WorkQueue.h>
@@ -209,7 +211,7 @@ static bool writeDataToFile(const Data& fileData, PlatformFileHandle fd)
     return success;
 }
 
-static std::error_code compiledToFile(String&& json, const String& finalFilePath, ContentRuleListMetaData& metaData, Data& mappedData)
+static std::error_code compiledToFile(String&& json, Vector<WebCore::ContentExtensions::ContentExtensionRule>&& parsedRules, const String& finalFilePath, ContentRuleListMetaData& metaData, Data& mappedData)
 {
     using namespace WebCore::ContentExtensions;
 
@@ -227,7 +229,8 @@ static std::error_code compiledToFile(String&& json, const String& finalFilePath
             ASSERT(!metaData.conditionsApplyOnlyToDomain);
         }
         
-        void writeSource(const String& sourceJSON) final {
+        void writeSource(String&& sourceJSON) final
+        {
             ASSERT(!m_filtersWithoutConditionsBytecodeWritten);
             ASSERT(!m_filtersWithConditionBytecodeWritten);
             ASSERT(!m_conditionFiltersBytecodeWritten);
@@ -339,7 +342,7 @@ static std::error_code compiledToFile(String&& json, const String& finalFilePath
 
     CompilationClient compilationClient(temporaryFileHandle, metaData);
     
-    if (auto compilerError = compileRuleList(compilationClient, WTFMove(json))) {
+    if (auto compilerError = compileRuleList(compilationClient, WTFMove(json), WTFMove(parsedRules))) {
         WTFLogAlways("Content Rule List compiling failed: Compiling failed.");
         closeFile(temporaryFileHandle);
         return compilerError;
@@ -391,7 +394,7 @@ static Ref<API::ContentRuleList> createExtension(const String& identifier, const
     return API::ContentRuleList::create(identifier, WTFMove(compiledContentRuleList));
 }
 
-void ContentRuleListStore::lookupContentRuleList(const WTF::String& identifier, Function<void(RefPtr<API::ContentRuleList>, std::error_code)> completionHandler)
+void ContentRuleListStore::lookupContentRuleList(const WTF::String& identifier, CompletionHandler<void(RefPtr<API::ContentRuleList>, std::error_code)> completionHandler)
 {
     m_readQueue->dispatch([protectedThis = makeRef(*this), identifier = identifier.isolatedCopy(), storePath = m_storePath.isolatedCopy(), legacyFilename = m_legacyFilename, completionHandler = WTFMove(completionHandler)]() mutable {
         auto path = constructedPath(storePath, identifier, legacyFilename);
@@ -399,26 +402,26 @@ void ContentRuleListStore::lookupContentRuleList(const WTF::String& identifier,
         ContentRuleListMetaData metaData;
         Data fileData;
         if (!openAndMapContentRuleList(path, metaData, fileData)) {
-            RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler)] {
+            RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler)] () mutable {
                 completionHandler(nullptr, Error::LookupFailed);
             });
             return;
         }
         
         if (metaData.version != ContentRuleListStore::CurrentContentRuleListFileVersion) {
-            RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler)] {
+            RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler)] () mutable {
                 completionHandler(nullptr, Error::VersionMismatch);
             });
             return;
         }
         
-        RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), identifier = identifier.isolatedCopy(), fileData = WTFMove(fileData), metaData = WTFMove(metaData), completionHandler = WTFMove(completionHandler)] {
+        RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), identifier = identifier.isolatedCopy(), fileData = WTFMove(fileData), metaData = WTFMove(metaData), completionHandler = WTFMove(completionHandler)] () mutable {
             completionHandler(createExtension(identifier, metaData, fileData), { });
         });
     });
 }
 
-void ContentRuleListStore::getAvailableContentRuleListIdentifiers(Function<void(WTF::Vector<WTF::String>)> completionHandler)
+void ContentRuleListStore::getAvailableContentRuleListIdentifiers(CompletionHandler<void(WTF::Vector<WTF::String>)> completionHandler)
 {
     m_readQueue->dispatch([protectedThis = makeRef(*this), storePath = m_storePath.isolatedCopy(), legacyFilename = m_legacyFilename, completionHandler = WTFMove(completionHandler)]() mutable {
 
@@ -435,43 +438,48 @@ void ContentRuleListStore::getAvailableContentRuleListIdentifiers(Function<void(
     });
 }
 
-void ContentRuleListStore::compileContentRuleList(const WTF::String& identifier, WTF::String&& json, Function<void(RefPtr<API::ContentRuleList>, std::error_code)> completionHandler)
+void ContentRuleListStore::compileContentRuleList(const WTF::String& identifier, WTF::String&& json, CompletionHandler<void(RefPtr<API::ContentRuleList>, std::error_code)> completionHandler)
 {
     AtomicString::init();
     WebCore::QualifiedName::init();
-    m_compileQueue->dispatch([protectedThis = makeRef(*this), identifier = identifier.isolatedCopy(), legacyFilename = m_legacyFilename, json = json.isolatedCopy(), storePath = m_storePath.isolatedCopy(), completionHandler = WTFMove(completionHandler)] () mutable {
+    
+    auto parsedRules = WebCore::ContentExtensions::parseRuleList(json);
+    if (!parsedRules.has_value())
+        return completionHandler(nullptr, parsedRules.error());
+    
+    m_compileQueue->dispatch([protectedThis = makeRef(*this), identifier = identifier.isolatedCopy(), legacyFilename = m_legacyFilename, json = json.isolatedCopy(), parsedRules = parsedRules.value().isolatedCopy(), storePath = m_storePath.isolatedCopy(), completionHandler = WTFMove(completionHandler)] () mutable {
         auto path = constructedPath(storePath, identifier, legacyFilename);
 
         ContentRuleListMetaData metaData;
         Data fileData;
-        auto error = compiledToFile(WTFMove(json), path, metaData, fileData);
+        auto error = compiledToFile(WTFMove(json), WTFMove(parsedRules), path, metaData, fileData);
         if (error) {
-            RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), error = WTFMove(error), completionHandler = WTFMove(completionHandler)] {
+            RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), error = WTFMove(error), completionHandler = WTFMove(completionHandler)] () mutable {
                 completionHandler(nullptr, error);
             });
             return;
         }
 
-        RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), identifier = WTFMove(identifier), fileData = WTFMove(fileData), metaData = WTFMove(metaData), completionHandler = WTFMove(completionHandler)] {
+        RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), identifier = WTFMove(identifier), fileData = WTFMove(fileData), metaData = WTFMove(metaData), completionHandler = WTFMove(completionHandler)] () mutable {
             RefPtr<API::ContentRuleList> contentRuleList = createExtension(identifier, metaData, fileData);
             completionHandler(contentRuleList, { });
         });
     });
 }
 
-void ContentRuleListStore::removeContentRuleList(const WTF::String& identifier, Function<void(std::error_code)> completionHandler)
+void ContentRuleListStore::removeContentRuleList(const WTF::String& identifier, CompletionHandler<void(std::error_code)> completionHandler)
 {
     m_removeQueue->dispatch([protectedThis = makeRef(*this), identifier = identifier.isolatedCopy(), storePath = m_storePath.isolatedCopy(), legacyFilename = m_legacyFilename, completionHandler = WTFMove(completionHandler)]() mutable {
         auto path = constructedPath(storePath, identifier, legacyFilename);
 
         if (!deleteFile(path)) {
-            RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler)] {
+            RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler)] () mutable {
                 completionHandler(Error::RemoveFailed);
             });
             return;
         }
 
-        RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler)] {
+        RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler)] () mutable {
             completionHandler({ });
         });
     });
@@ -494,13 +502,13 @@ void ContentRuleListStore::invalidateContentRuleListVersion(const WTF::String& i
     closeFile(file);
 }
 
-void ContentRuleListStore::getContentRuleListSource(const WTF::String& identifier, Function<void(WTF::String)> completionHandler)
+void ContentRuleListStore::getContentRuleListSource(const WTF::String& identifier, CompletionHandler<void(WTF::String)> completionHandler)
 {
     m_readQueue->dispatch([protectedThis = makeRef(*this), identifier = identifier.isolatedCopy(), storePath = m_storePath.isolatedCopy(), legacyFilename = m_legacyFilename, completionHandler = WTFMove(completionHandler)]() mutable {
         auto path = constructedPath(storePath, identifier, legacyFilename);
         
         auto complete = [protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler)](String source) mutable {
-            RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler), source = source.isolatedCopy()] {
+            RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler), source = source.isolatedCopy()] () mutable {
                 completionHandler(source);
             });
         };
index 77f1d9a..56f0f1f 100644 (file)
@@ -62,15 +62,15 @@ public:
     explicit ContentRuleListStore(const WTF::String& storePath, bool legacyFilename);
     virtual ~ContentRuleListStore();
 
-    void compileContentRuleList(const WTF::String& identifier, WTF::String&& json, Function<void(RefPtr<API::ContentRuleList>, std::error_code)>);
-    void lookupContentRuleList(const WTF::String& identifier, Function<void(RefPtr<API::ContentRuleList>, std::error_code)>);
-    void removeContentRuleList(const WTF::String& identifier, Function<void(std::error_code)>);
-    void getAvailableContentRuleListIdentifiers(Function<void(WTF::Vector<WTF::String>)>);
+    void compileContentRuleList(const WTF::String& identifier, WTF::String&& json, CompletionHandler<void(RefPtr<API::ContentRuleList>, std::error_code)>);
+    void lookupContentRuleList(const WTF::String& identifier, CompletionHandler<void(RefPtr<API::ContentRuleList>, std::error_code)>);
+    void removeContentRuleList(const WTF::String& identifier, CompletionHandler<void(std::error_code)>);
+    void getAvailableContentRuleListIdentifiers(CompletionHandler<void(WTF::Vector<WTF::String>)>);
 
     // For testing only.
     void synchronousRemoveAllContentRuleLists();
     void invalidateContentRuleListVersion(const WTF::String& identifier);
-    void getContentRuleListSource(const WTF::String& identifier, Function<void(WTF::String)>);
+    void getContentRuleListSource(const WTF::String& identifier, CompletionHandler<void(WTF::String)>);
 
 private:
     WTF::String defaultStorePath(bool legacyFilename);
index bcbb4ec..56355af 100644 (file)
@@ -32,6 +32,7 @@
 #import "APIContentRuleListStore.h"
 #import "WKErrorInternal.h"
 #import <wtf/BlockPtr.h>
+#import <wtf/CompletionHandler.h>
 
 static WKErrorCode toWKErrorCode(const std::error_code& error)
 {
index 8e80d4a..80991f0 100644 (file)
@@ -1,3 +1,15 @@
+2018-08-14  Alex Christensen  <achristensen@webkit.org>
+
+        isValidCSSSelector is unsafe to be called from a non-main thread
+        https://bugs.webkit.org/show_bug.cgi?id=188581
+        <rdar://problem/40517358>
+
+        Reviewed by Sam Weinig.
+
+        * TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp:
+        (TestWebKitAPI::InMemoryCompiledContentExtension::create):
+        (TestWebKitAPI::checkCompilerError):
+
 2018-08-14  Ansh Shukla  <ansh_shukla@apple.com>
 
         NSURLAuthenticationMethodOAuth challenges are surfaced to clients in -didReceiveAuthenticationChallenge as NSURLAuthenticationMethodDefault
index 6021e52..1a1a0ef 100644 (file)
@@ -100,7 +100,7 @@ public:
     }
 
 private:
-    void writeSource(const String&) final { }
+    void writeSource(String&&) final { }
 
     void writeActions(Vector<ContentExtensions::SerializedActionByte>&& actions, bool conditionsApplyOnlyToDomain) final
     {
@@ -150,7 +150,8 @@ public:
     {
         CompiledContentExtensionData extensionData;
         InMemoryContentExtensionCompilationClient client(extensionData);
-        auto compilerError = ContentExtensions::compileRuleList(client, WTFMove(filter));
+        auto parsedRules = ContentExtensions::parseRuleList(filter);
+        auto compilerError = ContentExtensions::compileRuleList(client, WTFMove(filter), WTFMove(parsedRules.value()));
 
         // Compiling should always succeed here. We have other tests for compile failures.
         EXPECT_FALSE(compilerError);
@@ -1371,7 +1372,12 @@ void checkCompilerError(const char* json, std::error_code expectedError)
 {
     CompiledContentExtensionData extensionData;
     InMemoryContentExtensionCompilationClient client(extensionData);
-    std::error_code compilerError = ContentExtensions::compileRuleList(client, json);
+    auto parsedRules = ContentExtensions::parseRuleList(json);
+    std::error_code compilerError;
+    if (parsedRules.has_value())
+        compilerError = ContentExtensions::compileRuleList(client, json, WTFMove(parsedRules.value()));
+    else
+        compilerError = parsedRules.error();
     EXPECT_EQ(compilerError.value(), expectedError.value());
     if (compilerError.value())
         EXPECT_STREQ(compilerError.category().name(), expectedError.category().name());