Diff
Modified: trunk/LayoutTests/ChangeLog (234538 => 234539)
--- trunk/LayoutTests/ChangeLog 2018-08-03 05:24:07 UTC (rev 234538)
+++ trunk/LayoutTests/ChangeLog 2018-08-03 08:16:48 UTC (rev 234539)
@@ -1,3 +1,15 @@
+2018-08-02 Ryosuke Niwa <[email protected]>
+
+ 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.
+
+ Unskipped the previously crashing test.
+
+ * TestExpectations:
+
2018-08-02 Basuke Suzuki <[email protected]>
[Curl] Test gardening
Modified: trunk/LayoutTests/TestExpectations (234538 => 234539)
--- trunk/LayoutTests/TestExpectations 2018-08-03 05:24:07 UTC (rev 234538)
+++ trunk/LayoutTests/TestExpectations 2018-08-03 08:16:48 UTC (rev 234539)
@@ -571,7 +571,6 @@
# WPT tests for custom elements
webkit.org/b/187800 imported/w3c/web-platform-tests/custom-elements/Document-createElement-svg.svg [ Skip ]
webkit.org/b/187802 imported/w3c/web-platform-tests/custom-elements/parser/parser-uses-create-an-element-for-a-token-svg.svg [ Skip ]
-webkit.org/b/187805 imported/w3c/web-platform-tests/custom-elements/reactions/with-exceptions.html [ Skip ]
# selectors
webkit.org/b/64861 imported/w3c/web-platform-tests/css/selectors/selectors-dir-selector-ltr-001.html [ ImageOnlyFailure ]
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (234538 => 234539)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2018-08-03 05:24:07 UTC (rev 234538)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2018-08-03 08:16:48 UTC (rev 234539)
@@ -1,3 +1,15 @@
+2018-08-02 Ryosuke Niwa <[email protected]>
+
+ 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.
+
+ Generated the expected result.
+
+ * web-platform-tests/custom-elements/reactions/with-exceptions-expected.txt: Added.
+
2018-08-01 Ryosuke Niwa <[email protected]>
Implement customElements.upgrade()
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/custom-elements/reactions/with-exceptions-expected.txt (0 => 234539)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/custom-elements/reactions/with-exceptions-expected.txt (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/custom-elements/reactions/with-exceptions-expected.txt 2018-08-03 08:16:48 UTC (rev 234539)
@@ -0,0 +1,3 @@
+
+PASS Reaction must run even after the exception is thrown
+
Modified: trunk/Source/WebCore/ChangeLog (234538 => 234539)
--- trunk/Source/WebCore/ChangeLog 2018-08-03 05:24:07 UTC (rev 234538)
+++ trunk/Source/WebCore/ChangeLog 2018-08-03 08:16:48 UTC (rev 234539)
@@ -1,3 +1,44 @@
+2018-08-02 Ryosuke Niwa <[email protected]>
+
+ 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.
+
+ 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):
+
2018-08-02 Zalan Bujtas <[email protected]>
[LFC][BFC] Apply the "10.6.6 Complicated cases" when computing height and margin for the document renderer
Modified: trunk/Source/WebCore/bindings/js/JSMainThreadExecState.h (234538 => 234539)
--- trunk/Source/WebCore/bindings/js/JSMainThreadExecState.h 2018-08-03 05:24:07 UTC (rev 234538)
+++ trunk/Source/WebCore/bindings/js/JSMainThreadExecState.h 2018-08-03 08:16:48 UTC (rev 234539)
@@ -161,6 +161,7 @@
public:
explicit JSMainThreadNullState()
: m_previousState(JSMainThreadExecState::s_mainThreadState)
+ , m_customElementReactionStack(m_previousState)
{
ASSERT(isMainThread());
JSMainThreadExecState::s_mainThreadState = nullptr;
Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (234538 => 234539)
--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2018-08-03 05:24:07 UTC (rev 234538)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2018-08-03 08:16:48 UTC (rev 234539)
@@ -932,7 +932,7 @@
push(@$outputArray, " ASSERT_GC_OBJECT_INHERITS(thisObject, info());\n\n");
if (($namedSetterOperation && $namedSetterOperation->extendedAttributes->{CEReactions}) || ($indexedSetterOperation && $indexedSetterOperation->extendedAttributes->{CEReactions})) {
- push(@$outputArray, " CustomElementReactionStack customElementReactionStack;\n\n");
+ push(@$outputArray, " CustomElementReactionStack customElementReactionStack(*state);\n\n");
AddToImplIncludes("CustomElementReactionQueue.h");
}
@@ -1004,7 +1004,7 @@
push(@$outputArray, " ASSERT_GC_OBJECT_INHERITS(thisObject, info());\n\n");
if (($namedSetterOperation && $namedSetterOperation->extendedAttributes->{CEReactions}) || ($indexedSetterOperation && $indexedSetterOperation->extendedAttributes->{CEReactions})) {
- push(@$outputArray, " CustomElementReactionStack customElementReactionStack;\n\n");
+ push(@$outputArray, " CustomElementReactionStack customElementReactionStack(*state);\n\n");
AddToImplIncludes("CustomElementReactionQueue.h");
}
@@ -1100,7 +1100,7 @@
push(@$outputArray, " ASSERT_GC_OBJECT_INHERITS(thisObject, info());\n\n");
if (($namedSetterOperation && $namedSetterOperation->extendedAttributes->{CEReactions}) || ($indexedSetterOperation && $indexedSetterOperation->extendedAttributes->{CEReactions})) {
- push(@$outputArray, " CustomElementReactionStack customElementReactionStack;\n\n");
+ push(@$outputArray, " CustomElementReactionStack customElementReactionStack(*state);\n\n");
AddToImplIncludes("CustomElementReactionQueue.h");
}
@@ -1224,7 +1224,7 @@
push(@$outputArray, " if (isVisibleNamedProperty<${overrideBuiltin}>(*state, thisObject, propertyName)) {\n");
if ($operation->extendedAttributes->{CEReactions}) {
- push(@$outputArray, " CustomElementReactionStack customElementReactionStack;\n");
+ push(@$outputArray, " CustomElementReactionStack customElementReactionStack(*state);\n");
AddToImplIncludes("CustomElementReactionQueue.h", $conditional);
}
@@ -4844,7 +4844,7 @@
push(@$outputArray, " UNUSED_PARAM(throwScope);\n");
if ($attribute->extendedAttributes->{CEReactions}) {
- push(@$outputArray, " CustomElementReactionStack customElementReactionStack;\n");
+ push(@$outputArray, " CustomElementReactionStack customElementReactionStack(state);\n");
AddToImplIncludes("CustomElementReactionQueue.h", $conditional);
}
@@ -5070,7 +5070,7 @@
if (!$generatingOverloadDispatcher) {
if ($operation->extendedAttributes->{CEReactions}) {
AddToImplIncludes("CustomElementReactionQueue.h", $conditional);
- push(@$outputArray, " CustomElementReactionStack customElementReactionStack;\n");
+ push(@$outputArray, " CustomElementReactionStack customElementReactionStack(*state);\n");
}
if ($interface->extendedAttributes->{CheckSecurity} and !$operation->extendedAttributes->{DoNotCheckSecurity}) {
Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestCEReactions.cpp (234538 => 234539)
--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestCEReactions.cpp 2018-08-03 05:24:07 UTC (rev 234538)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestCEReactions.cpp 2018-08-03 08:16:48 UTC (rev 234539)
@@ -203,7 +203,7 @@
static inline bool setJSTestCEReactionsAttributeWithCEReactionsSetter(ExecState& state, JSTestCEReactions& thisObject, JSValue value, ThrowScope& throwScope)
{
UNUSED_PARAM(throwScope);
- CustomElementReactionStack customElementReactionStack;
+ CustomElementReactionStack customElementReactionStack(state);
auto& impl = thisObject.wrapped();
auto nativeValue = convert<IDLDOMString>(state, value);
RETURN_IF_EXCEPTION(throwScope, false);
@@ -235,7 +235,7 @@
static inline bool setJSTestCEReactionsReflectAttributeWithCEReactionsSetter(ExecState& state, JSTestCEReactions& thisObject, JSValue value, ThrowScope& throwScope)
{
UNUSED_PARAM(throwScope);
- CustomElementReactionStack customElementReactionStack;
+ CustomElementReactionStack customElementReactionStack(state);
auto& impl = thisObject.wrapped();
auto nativeValue = convert<IDLDOMString>(state, value);
RETURN_IF_EXCEPTION(throwScope, false);
@@ -290,7 +290,7 @@
{
UNUSED_PARAM(state);
UNUSED_PARAM(throwScope);
- CustomElementReactionStack customElementReactionStack;
+ CustomElementReactionStack customElementReactionStack(*state);
auto& impl = castedThis->wrapped();
impl.methodWithCEReactions();
return JSValue::encode(jsUndefined());
Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestCEReactionsStringifier.cpp (234538 => 234539)
--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestCEReactionsStringifier.cpp 2018-08-03 05:24:07 UTC (rev 234538)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestCEReactionsStringifier.cpp 2018-08-03 08:16:48 UTC (rev 234539)
@@ -193,7 +193,7 @@
static inline bool setJSTestCEReactionsStringifierValueSetter(ExecState& state, JSTestCEReactionsStringifier& thisObject, JSValue value, ThrowScope& throwScope)
{
UNUSED_PARAM(throwScope);
- CustomElementReactionStack customElementReactionStack;
+ CustomElementReactionStack customElementReactionStack(state);
auto& impl = thisObject.wrapped();
auto nativeValue = convert<IDLDOMString>(state, value);
RETURN_IF_EXCEPTION(throwScope, false);
Modified: trunk/Source/WebCore/dom/CustomElementReactionQueue.cpp (234538 => 234539)
--- trunk/Source/WebCore/dom/CustomElementReactionQueue.cpp 2018-08-03 05:24:07 UTC (rev 234538)
+++ trunk/Source/WebCore/dom/CustomElementReactionQueue.cpp 2018-08-03 08:16:48 UTC (rev 234539)
@@ -34,6 +34,7 @@
#include "JSCustomElementInterface.h"
#include "JSDOMBinding.h"
#include "Microtasks.h"
+#include <_javascript_Core/CatchScope.h>
#include <_javascript_Core/Heap.h>
#include <wtf/NeverDestroyed.h>
#include <wtf/Optional.h>
@@ -231,6 +232,32 @@
RELEASE_ASSERT(m_elements.isEmpty());
}
+inline void CustomElementReactionQueue::ElementQueue::processQueue(JSC::ExecState* state)
+{
+ if (!state) {
+ invokeAll();
+ return;
+ }
+
+ auto& vm = state->vm();
+ JSC::JSLockHolder lock(vm);
+
+ JSC::Exception* previousException = nullptr;
+ {
+ auto catchScope = DECLARE_CATCH_SCOPE(vm);
+ previousException = catchScope.exception();
+ if (previousException)
+ catchScope.clearException();
+ }
+
+ invokeAll();
+
+ if (previousException) {
+ auto throwScope = DECLARE_THROW_SCOPE(vm);
+ throwException(state, throwScope, previousException);
+ }
+}
+
CustomElementReactionQueue& CustomElementReactionQueue::ensureCurrentQueue(Element& element)
{
ASSERT(element.reactionQueue());
@@ -249,10 +276,10 @@
CustomElementReactionStack* CustomElementReactionStack::s_currentProcessingStack = nullptr;
-void CustomElementReactionStack::processQueue()
+void CustomElementReactionStack::processQueue(JSC::ExecState* state)
{
ASSERT(m_queue);
- m_queue->invokeAll();
+ m_queue->processQueue(state);
delete m_queue;
m_queue = nullptr;
}
@@ -280,7 +307,7 @@
void CustomElementReactionQueue::processBackupQueue()
{
- backupElementQueue().invokeAll();
+ backupElementQueue().processQueue(nullptr);
s_processingBackupElementQueue = false;
}
Modified: trunk/Source/WebCore/dom/CustomElementReactionQueue.h (234538 => 234539)
--- trunk/Source/WebCore/dom/CustomElementReactionQueue.h 2018-08-03 05:24:07 UTC (rev 234538)
+++ trunk/Source/WebCore/dom/CustomElementReactionQueue.h 2018-08-03 08:16:48 UTC (rev 234539)
@@ -29,6 +29,12 @@
#include <wtf/Noncopyable.h>
#include <wtf/Vector.h>
+namespace JSC {
+
+class ExecState;
+
+}
+
namespace WebCore {
class CustomElementReactionQueueItem;
@@ -60,9 +66,11 @@
class ElementQueue {
public:
void add(Element&);
+ void processQueue(JSC::ExecState*);
+
+ private:
void invokeAll();
-
- private:
+
Vector<Ref<Element>> m_elements;
bool m_invoking { false };
};
@@ -78,24 +86,30 @@
class CustomElementReactionStack {
public:
- CustomElementReactionStack()
+ ALWAYS_INLINE CustomElementReactionStack(JSC::ExecState* state)
: m_previousProcessingStack(s_currentProcessingStack)
+ , m_state(state)
{
s_currentProcessingStack = this;
}
- ~CustomElementReactionStack()
+ ALWAYS_INLINE CustomElementReactionStack(JSC::ExecState& state)
+ : CustomElementReactionStack(&state)
+ { }
+
+ ALWAYS_INLINE ~CustomElementReactionStack()
{
if (UNLIKELY(m_queue))
- processQueue();
+ processQueue(m_state);
s_currentProcessingStack = m_previousProcessingStack;
}
private:
- WEBCORE_EXPORT void processQueue();
+ WEBCORE_EXPORT void processQueue(JSC::ExecState*);
CustomElementReactionQueue::ElementQueue* m_queue { nullptr }; // Use raw pointer to avoid generating delete in the destructor.
CustomElementReactionStack* m_previousProcessingStack;
+ JSC::ExecState* m_state;
WEBCORE_EXPORT static CustomElementReactionStack* s_currentProcessingStack;
