Attachment 347124 Details for Bug 188581 – Patch

[patch] Patch

bug-188581-20180814161357.patch (text/plain), 27.91 KB, created by Alex Christensen on 2018-08-14 16:13:58 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Christensen

Created: 2018-08-14 16:13:58 PDT

Size: 27.91 KB

patch

obsolete

>Index: Source/WTF/ChangeLog
>===================================================================
>--- Source/WTF/ChangeLog	(revision 234859)
>+++ Source/WTF/ChangeLog	(working copy)
>@@ -1,3 +1,14 @@
>+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
>Index: Source/WTF/wtf/Vector.h
>===================================================================
>--- Source/WTF/wtf/Vector.h	(revision 234853)
>+++ Source/WTF/wtf/Vector.h	(working copy)
>@@ -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<V
> };
> #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: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 234859)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -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  Alex Christensen  <achristensen@webkit.org>
> 
>         Use a Variant instead of a union in CSSSelector
>Index: Source/WebCore/contentextensions/ContentExtensionCompiler.cpp
>===================================================================
>--- Source/WebCore/contentextensions/ContentExtensionCompiler.cpp	(revision 234853)
>+++ Source/WebCore/contentextensions/ContentExtensionCompiler.cpp	(working copy)
>@@ -283,12 +283,13 @@ static void compileToBytecode(CombinedUR
>     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(ContentE
>     MonotonicTime patternPartitioningStart = MonotonicTime::now();
> #endif
>     
>-    client.writeSource(ruleJSON);
>+    client.writeSource(std::exchange(ruleJSON, String()));
> 
>     Vector<SerializedActionByte> actions;
>     Vector<unsigned> actionLocations = serializeActions(parsedRuleList, actions);
>Index: Source/WebCore/contentextensions/ContentExtensionCompiler.h
>===================================================================
>--- Source/WebCore/contentextensions/ContentExtensionCompiler.h	(revision 234853)
>+++ Source/WebCore/contentextensions/ContentExtensionCompiler.h	(working copy)
>@@ -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: Source/WebCore/contentextensions/ContentExtensionParser.cpp
>===================================================================
>--- Source/WebCore/contentextensions/ContentExtensionParser.cpp	(revision 234853)
>+++ Source/WebCore/contentextensions/ContentExtensionParser.cpp	(working copy)
>@@ -230,6 +230,7 @@ static Expected<Trigger, std::error_code
> 
> bool isValidCSSSelector(const String& selector)
> {
>+    ASSERT(isMainThread());
>     AtomicString::init();
>     QualifiedName::init();
>     CSSParserContext context(HTMLQuirksMode);
>@@ -299,7 +300,7 @@ static Expected<std::optional<ContentExt
>     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<ContentExtensionR
>     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>, s
>     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: Source/WebCore/contentextensions/ContentExtensionParser.h
>===================================================================
>--- Source/WebCore/contentextensions/ContentExtensionParser.h	(revision 234853)
>+++ Source/WebCore/contentextensions/ContentExtensionParser.h	(working copy)
>@@ -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: Source/WebCore/contentextensions/ContentExtensionRule.cpp
>===================================================================
>--- Source/WebCore/contentextensions/ContentExtensionRule.cpp	(revision 234853)
>+++ Source/WebCore/contentextensions/ContentExtensionRule.cpp	(working copy)
>@@ -113,6 +113,28 @@ uint32_t Action::serializedLength(const 
>     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: Source/WebCore/contentextensions/ContentExtensionRule.h
>===================================================================
>--- Source/WebCore/contentextensions/ContentExtensionRule.h	(revision 234853)
>+++ Source/WebCore/contentextensions/ContentExtensionRule.h	(working copy)
>@@ -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: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 234853)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -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  Antti Koivisto  <antti@apple.com>
> 
>         RemoteLayerTreeTransaction should use OptionSet for change flags
>Index: Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp
>===================================================================
>--- Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp	(revision 234853)
>+++ Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp	(working copy)
>@@ -35,8 +35,10 @@
> #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& 
>     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(St
>             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(St
> 
>     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> createE
>     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::lookupContent
>         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::getAvailableC
>     });
> }
> 
>-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::invalidateCon
>     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: Source/WebKit/UIProcess/API/APIContentRuleListStore.h
>===================================================================
>--- Source/WebKit/UIProcess/API/APIContentRuleListStore.h	(revision 234853)
>+++ Source/WebKit/UIProcess/API/APIContentRuleListStore.h	(working copy)
>@@ -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: Source/WebKit/UIProcess/API/Cocoa/WKContentRuleListStore.mm
>===================================================================
>--- Source/WebKit/UIProcess/API/Cocoa/WKContentRuleListStore.mm	(revision 234853)
>+++ Source/WebKit/UIProcess/API/Cocoa/WKContentRuleListStore.mm	(working copy)
>@@ -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: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 234853)
>+++ Tools/ChangeLog	(working copy)
>@@ -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  Aakash Jain  <aakash_jain@apple.com>
> 
>         [ews-build] Add support for max_builds parameter for workers
>Index: Tools/TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp
>===================================================================
>--- Tools/TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp	(revision 234853)
>+++ Tools/TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp	(working copy)
>@@ -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
> {
>     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());

Actions: View | Formatted Diff | Diff

Attachments on bug 188581: 347113 | 347116 | 347121 | 347124