Changeset 234539 in webkit


Ignore:
Timestamp:
Aug 3, 2018 1:16:48 AM (6 years ago)
Author:
rniwa@webkit.org
Message:

Release assert when throwing exceptions in custom element reactions
​https://bugs.webkit.org/show_bug.cgi?id=187805
<rdar://problem/42432714>

Reviewed by Saam Barati.

LayoutTests/imported/w3c:

Generated the expected result.

  • web-platform-tests/custom-elements/reactions/with-exceptions-expected.txt: Added.

Source/WebCore:

The release assertion was hit because we were not catching & re-throwing the exception thrown by DOM API
before trying to execute custom elements reactions in ~CustomElementReactionStack as specified here:
​https://html.spec.whatwg.org/multipage/custom-elements.html#cereactions
Fixed the bug by capturing the exception and re-throwing the exception as specified.

Tests: imported/w3c/web-platform-tests/custom-elements/reactions/with-exceptions.html

  • bindings/js/JSMainThreadExecState.h:

(WebCore::JSMainThreadNullState::JSMainThreadNullState): Use the previous JS state.

  • bindings/scripts/CodeGeneratorJS.pm:

(GeneratePut): Pass in the exec state to CustomElementReactionStack.
(GeneratePutByIndex): Ditto.
(GenerateDefineOwnProperty): Ditto.
(GenerateDeletePropertyCommon): Ditto.
(GenerateAttributeSetterBodyDefinition): Ditto.
(GenerateOperationBodyDefinition): Ditto.

  • bindings/scripts/test/JS/JSTestCEReactions.cpp:

(WebCore::setJSTestCEReactionsAttributeWithCEReactionsSetter):
(WebCore::setJSTestCEReactionsReflectAttributeWithCEReactionsSetter):
(WebCore::jsTestCEReactionsPrototypeFunctionMethodWithCEReactionsBody):

  • bindings/scripts/test/JS/JSTestCEReactionsStringifier.cpp:

(WebCore::setJSTestCEReactionsStringifierValueSetter):

  • dom/CustomElementReactionQueue.cpp:

(WebCore::CustomElementReactionQueue::ElementQueue::processQueue): Added. If there is a script running
in the stack (i.e. ExecState is not null), catch any exception before executing custom element reactions,
then re-throw the exception afterwards. ExecState is null when DOM API is invoked via Objective-C bindings
or when custom element reactions are executed in the backup queue (e.g. for editing operations).
(WebCore::CustomElementReactionStack::processQueue):
(WebCore::CustomElementReactionQueue::processBackupQueue):

  • dom/CustomElementReactionQueue.h:

(WebCore::CustomElementReactionStack::CustomElementReactionStack):
(WebCore::CustomElementReactionStack::~CustomElementReactionStack):

LayoutTests:

Unskipped the previously crashing test.

Location:
trunk
Files:
1 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r234538 r234539  
     12018-08-02  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Release assert when throwing exceptions in custom element reactions
     4        https://bugs.webkit.org/show_bug.cgi?id=187805
     5        <rdar://problem/42432714>
     6
     7        Reviewed by Saam Barati.
     8
     9        Unskipped the previously crashing test.
     10
     11        * TestExpectations:
     12
    1132018-08-02  Basuke Suzuki  <Basuke.Suzuki@sony.com>
    214

  • trunk/LayoutTests/TestExpectations

    r234337 r234539  
    572572webkit.org/b/187800 imported/w3c/web-platform-tests/custom-elements/Document-createElement-svg.svg [ Skip ]
    573573webkit.org/b/187802 imported/w3c/web-platform-tests/custom-elements/parser/parser-uses-create-an-element-for-a-token-svg.svg [ Skip ]
    574 webkit.org/b/187805 imported/w3c/web-platform-tests/custom-elements/reactions/with-exceptions.html [ Skip ]
    575574
    576575# selectors

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r234507 r234539  
     12018-08-02  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Release assert when throwing exceptions in custom element reactions
     4        https://bugs.webkit.org/show_bug.cgi?id=187805
     5        <rdar://problem/42432714>
     6
     7        Reviewed by Saam Barati.
     8
     9        Generated the expected result.
     10
     11        * web-platform-tests/custom-elements/reactions/with-exceptions-expected.txt: Added.
     12
    1132018-08-01  Ryosuke Niwa  <rniwa@webkit.org>
    214

  • trunk/Source/WebCore/ChangeLog

    r234537 r234539  
     12018-08-02  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Release assert when throwing exceptions in custom element reactions
     4        https://bugs.webkit.org/show_bug.cgi?id=187805
     5        <rdar://problem/42432714>
     6
     7        Reviewed by Saam Barati.
     8
     9        The release assertion was hit because we were not catching & re-throwing the exception thrown by DOM API
     10        before trying to execute custom elements reactions in ~CustomElementReactionStack as specified here:
     11        https://html.spec.whatwg.org/multipage/custom-elements.html#cereactions
     12        Fixed the bug by capturing the exception and re-throwing the exception as specified.
     13
     14        Tests: imported/w3c/web-platform-tests/custom-elements/reactions/with-exceptions.html
     15
     16        * bindings/js/JSMainThreadExecState.h:
     17        (WebCore::JSMainThreadNullState::JSMainThreadNullState): Use the previous JS state.
     18        * bindings/scripts/CodeGeneratorJS.pm:
     19        (GeneratePut): Pass in the exec state to CustomElementReactionStack.
     20        (GeneratePutByIndex): Ditto.
     21        (GenerateDefineOwnProperty): Ditto.
     22        (GenerateDeletePropertyCommon): Ditto.
     23        (GenerateAttributeSetterBodyDefinition): Ditto.
     24        (GenerateOperationBodyDefinition): Ditto.
     25        * bindings/scripts/test/JS/JSTestCEReactions.cpp:
     26        (WebCore::setJSTestCEReactionsAttributeWithCEReactionsSetter):
     27        (WebCore::setJSTestCEReactionsReflectAttributeWithCEReactionsSetter):
     28        (WebCore::jsTestCEReactionsPrototypeFunctionMethodWithCEReactionsBody):
     29        * bindings/scripts/test/JS/JSTestCEReactionsStringifier.cpp:
     30        (WebCore::setJSTestCEReactionsStringifierValueSetter):
     31        * dom/CustomElementReactionQueue.cpp:
     32        (WebCore::CustomElementReactionQueue::ElementQueue::processQueue): Added. If there is a script running
     33        in the stack (i.e. ExecState is not null), catch any exception before executing custom element reactions,
     34        then re-throw the exception afterwards. ExecState is null when DOM API is invoked via Objective-C bindings
     35        or when custom element reactions are executed in the backup queue (e.g. for editing operations).
     36        (WebCore::CustomElementReactionStack::processQueue):
     37        (WebCore::CustomElementReactionQueue::processBackupQueue):
     38        * dom/CustomElementReactionQueue.h:
     39        (WebCore::CustomElementReactionStack::CustomElementReactionStack):
     40        (WebCore::CustomElementReactionStack::~CustomElementReactionStack):
     41
    1422018-08-02  Zalan Bujtas  <zalan@apple.com>
    243

  • trunk/Source/WebCore/bindings/js/JSMainThreadExecState.h

    r228218 r234539  
    162162    explicit JSMainThreadNullState()
    163163        : m_previousState(JSMainThreadExecState::s_mainThreadState)
     164        , m_customElementReactionStack(m_previousState)
    164165    {
    165166        ASSERT(isMainThread());

  • trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

    r233765 r234539  
    933933   
    934934    if (($namedSetterOperation && $namedSetterOperation->extendedAttributes->{CEReactions}) || ($indexedSetterOperation && $indexedSetterOperation->extendedAttributes->{CEReactions})) {
    935         push(@$outputArray, "    CustomElementReactionStack customElementReactionStack;\n\n");
     935        push(@$outputArray, "    CustomElementReactionStack customElementReactionStack(*state);\n\n");
    936936        AddToImplIncludes("CustomElementReactionQueue.h");
    937937    }
    … …  
    10051005   
    10061006    if (($namedSetterOperation && $namedSetterOperation->extendedAttributes->{CEReactions}) || ($indexedSetterOperation && $indexedSetterOperation->extendedAttributes->{CEReactions})) {
    1007         push(@$outputArray, "    CustomElementReactionStack customElementReactionStack;\n\n");
     1007        push(@$outputArray, "    CustomElementReactionStack customElementReactionStack(*state);\n\n");
    10081008        AddToImplIncludes("CustomElementReactionQueue.h");
    10091009    }
    … …  
    11011101   
    11021102    if (($namedSetterOperation && $namedSetterOperation->extendedAttributes->{CEReactions}) || ($indexedSetterOperation && $indexedSetterOperation->extendedAttributes->{CEReactions})) {
    1103         push(@$outputArray, "    CustomElementReactionStack customElementReactionStack;\n\n");
     1103        push(@$outputArray, "    CustomElementReactionStack customElementReactionStack(*state);\n\n");
    11041104        AddToImplIncludes("CustomElementReactionQueue.h");
    11051105    }
    … …  
    12251225
    12261226    if ($operation->extendedAttributes->{CEReactions}) {
    1227         push(@$outputArray, "        CustomElementReactionStack customElementReactionStack;\n");
     1227        push(@$outputArray, "        CustomElementReactionStack customElementReactionStack(*state);\n");
    12281228        AddToImplIncludes("CustomElementReactionQueue.h", $conditional);
    12291229    }
    … …  
    48454845   
    48464846    if ($attribute->extendedAttributes->{CEReactions}) {
    4847         push(@$outputArray, "    CustomElementReactionStack customElementReactionStack;\n");
     4847        push(@$outputArray, "    CustomElementReactionStack customElementReactionStack(state);\n");
    48484848        AddToImplIncludes("CustomElementReactionQueue.h", $conditional);
    48494849    }
    … …  
    50715071        if ($operation->extendedAttributes->{CEReactions}) {
    50725072            AddToImplIncludes("CustomElementReactionQueue.h", $conditional);
    5073             push(@$outputArray, "    CustomElementReactionStack customElementReactionStack;\n");
     5073            push(@$outputArray, "    CustomElementReactionStack customElementReactionStack(*state);\n");
    50745074        }
    50755075

  • trunk/Source/WebCore/bindings/scripts/test/JS/JSTestCEReactions.cpp

    r233122 r234539  
    204204{
    205205    UNUSED_PARAM(throwScope);
    206     CustomElementReactionStack customElementReactionStack;
     206    CustomElementReactionStack customElementReactionStack(state);
    207207    auto& impl = thisObject.wrapped();
    208208    auto nativeValue = convert<IDLDOMString>(state, value);
    … …  
    236236{
    237237    UNUSED_PARAM(throwScope);
    238     CustomElementReactionStack customElementReactionStack;
     238    CustomElementReactionStack customElementReactionStack(state);
    239239    auto& impl = thisObject.wrapped();
    240240    auto nativeValue = convert<IDLDOMString>(state, value);
    … …  
    291291    UNUSED_PARAM(state);
    292292    UNUSED_PARAM(throwScope);
    293     CustomElementReactionStack customElementReactionStack;
     293    CustomElementReactionStack customElementReactionStack(*state);
    294294    auto& impl = castedThis->wrapped();
    295295    impl.methodWithCEReactions();

  • trunk/Source/WebCore/bindings/scripts/test/JS/JSTestCEReactionsStringifier.cpp

    r233122 r234539  
    194194{
    195195    UNUSED_PARAM(throwScope);
    196     CustomElementReactionStack customElementReactionStack;
     196    CustomElementReactionStack customElementReactionStack(state);
    197197    auto& impl = thisObject.wrapped();
    198198    auto nativeValue = convert<IDLDOMString>(state, value);

  • trunk/Source/WebCore/dom/CustomElementReactionQueue.cpp

    r234520 r234539  
    3535#include "JSDOMBinding.h"
    3636#include "Microtasks.h"
     37#include <JavaScriptCore/CatchScope.h>
    3738#include <JavaScriptCore/Heap.h>
    3839#include <wtf/NeverDestroyed.h>
    … …  
    232233}
    233234
     235inline void CustomElementReactionQueue::ElementQueue::processQueue(JSC::ExecState* state)
     236{
     237    if (!state) {
     238        invokeAll();
     239        return;
     240    }
     241
     242    auto& vm = state->vm();
     243    JSC::JSLockHolder lock(vm);
     244
     245    JSC::Exception* previousException = nullptr;
     246    {
     247        auto catchScope = DECLARE_CATCH_SCOPE(vm);
     248        previousException = catchScope.exception();
     249        if (previousException)
     250            catchScope.clearException();
     251    }
     252
     253    invokeAll();
     254
     255    if (previousException) {
     256        auto throwScope = DECLARE_THROW_SCOPE(vm);
     257        throwException(state, throwScope, previousException);
     258    }
     259}
     260
    234261CustomElementReactionQueue& CustomElementReactionQueue::ensureCurrentQueue(Element& element)
    235262{
    … …  
    250277CustomElementReactionStack* CustomElementReactionStack::s_currentProcessingStack = nullptr;
    251278
    252 void CustomElementReactionStack::processQueue()
     279void CustomElementReactionStack::processQueue(JSC::ExecState* state)
    253280{
    254281    ASSERT(m_queue);
    255     m_queue->invokeAll();
     282    m_queue->processQueue(state);
    256283    delete m_queue;
    257284    m_queue = nullptr;
    … …  
    281308void CustomElementReactionQueue::processBackupQueue()
    282309{
    283     backupElementQueue().invokeAll();
     310    backupElementQueue().processQueue(nullptr);
    284311    s_processingBackupElementQueue = false;
    285312}

  • trunk/Source/WebCore/dom/CustomElementReactionQueue.h

    r234520 r234539  
    2929#include <wtf/Noncopyable.h>
    3030#include <wtf/Vector.h>
     31
     32namespace JSC {
     33
     34class ExecState;
     35
     36}
    3137
    3238namespace WebCore {
    … …  
    6167    public:
    6268        void add(Element&);
     69        void processQueue(JSC::ExecState*);
     70
     71    private:
    6372        void invokeAll();
    64        
    65     private:
     73
    6674        Vector<Ref<Element>> m_elements;
    6775        bool m_invoking { false };
    … …  
    7987class CustomElementReactionStack {
    8088public:
    81     CustomElementReactionStack()
     89    ALWAYS_INLINE CustomElementReactionStack(JSC::ExecState* state)
    8290        : m_previousProcessingStack(s_currentProcessingStack)
     91        , m_state(state)
    8392    {
    8493        s_currentProcessingStack = this;
    8594    }
    8695
    87     ~CustomElementReactionStack()
     96    ALWAYS_INLINE CustomElementReactionStack(JSC::ExecState& state)
     97        : CustomElementReactionStack(&state)
     98    { }
     99
     100    ALWAYS_INLINE ~CustomElementReactionStack()
    88101    {
    89102        if (UNLIKELY(m_queue))
    90             processQueue();
     103            processQueue(m_state);
    91104        s_currentProcessingStack = m_previousProcessingStack;
    92105    }
    93106
    94107private:
    95     WEBCORE_EXPORT void processQueue();
     108    WEBCORE_EXPORT void processQueue(JSC::ExecState*);
    96109
    97110    CustomElementReactionQueue::ElementQueue* m_queue { nullptr }; // Use raw pointer to avoid generating delete in the destructor.
    98111    CustomElementReactionStack* m_previousProcessingStack;
     112    JSC::ExecState* m_state;
    99113
    100114    WEBCORE_EXPORT static CustomElementReactionStack* s_currentProcessingStack;
Note: See TracChangeset for help on using the changeset viewer.