Title: [264447] trunk
Revision
264447
Author
ysuz...@apple.com
Date
2020-07-15 23:32:56 -0700 (Wed, 15 Jul 2020)

Log Message

Should clear exception after microtask draining
https://bugs.webkit.org/show_bug.cgi?id=214391
<rdar://problem/65536227>

Reviewed by Mark Lam.

Source/WebCore:

Test: js/dom/microtask-drain-should-clear-exceptions.html

Since microtask draining involves JS calls, potentially it can cause exception.
We should clear exceptions after microtask draining since we do not handle it and suppress exception-check validation.
And we also should not create ThrowScope in canAccessDocument when parameter says it never throws an error.
Both cases are handled in the attached test.

* bindings/js/JSDOMBindingSecurity.cpp:
(WebCore::canAccessDocument):
* bindings/js/JSExecState.h:
(WebCore::JSExecState::~JSExecState):

LayoutTests:

* js/dom/microtask-drain-should-clear-exceptions-expected.txt: Added.
* js/dom/microtask-drain-should-clear-exceptions.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (264446 => 264447)


--- trunk/LayoutTests/ChangeLog	2020-07-16 04:05:29 UTC (rev 264446)
+++ trunk/LayoutTests/ChangeLog	2020-07-16 06:32:56 UTC (rev 264447)
@@ -1,3 +1,14 @@
+2020-07-15  Yusuke Suzuki  <ysuz...@apple.com>
+
+        Should clear exception after microtask draining
+        https://bugs.webkit.org/show_bug.cgi?id=214391
+        <rdar://problem/65536227>
+
+        Reviewed by Mark Lam.
+
+        * js/dom/microtask-drain-should-clear-exceptions-expected.txt: Added.
+        * js/dom/microtask-drain-should-clear-exceptions.html: Added.
+
 2020-07-15  Ryan Haddad  <ryanhad...@apple.com>
 
         Unreviewed test gardening, add iOS baseliens for tests re-synced in r264427 and r264432.

Added: trunk/LayoutTests/js/dom/microtask-drain-should-clear-exceptions-expected.txt (0 => 264447)


--- trunk/LayoutTests/js/dom/microtask-drain-should-clear-exceptions-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/js/dom/microtask-drain-should-clear-exceptions-expected.txt	2020-07-16 06:32:56 UTC (rev 264447)
@@ -0,0 +1,9 @@
+Ensure exception check does not fire after microtask draining
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/js/dom/microtask-drain-should-clear-exceptions.html (0 => 264447)


--- trunk/LayoutTests/js/dom/microtask-drain-should-clear-exceptions.html	                        (rev 0)
+++ trunk/LayoutTests/js/dom/microtask-drain-should-clear-exceptions.html	2020-07-16 06:32:56 UTC (rev 264447)
@@ -0,0 +1,13 @@
+<!DOCTYPE html><!-- webkit-test-runner [ jscOptions=--validateExceptionChecks=true ] -->
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script>
+description("Ensure exception check does not fire after microtask draining");
+Promise.resolve().then();
+</script>
+<script src=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (264446 => 264447)


--- trunk/Source/WebCore/ChangeLog	2020-07-16 04:05:29 UTC (rev 264446)
+++ trunk/Source/WebCore/ChangeLog	2020-07-16 06:32:56 UTC (rev 264447)
@@ -1,3 +1,23 @@
+2020-07-15  Yusuke Suzuki  <ysuz...@apple.com>
+
+        Should clear exception after microtask draining
+        https://bugs.webkit.org/show_bug.cgi?id=214391
+        <rdar://problem/65536227>
+
+        Reviewed by Mark Lam.
+
+        Test: js/dom/microtask-drain-should-clear-exceptions.html
+
+        Since microtask draining involves JS calls, potentially it can cause exception.
+        We should clear exceptions after microtask draining since we do not handle it and suppress exception-check validation.
+        And we also should not create ThrowScope in canAccessDocument when parameter says it never throws an error.
+        Both cases are handled in the attached test.
+
+        * bindings/js/JSDOMBindingSecurity.cpp:
+        (WebCore::canAccessDocument):
+        * bindings/js/JSExecState.h:
+        (WebCore::JSExecState::~JSExecState):
+
 2020-07-15  Devin Rousso  <drou...@apple.com>
 
         Unreviewed followup, use `objectReplacementCharacter` instead of `0xFFFC`

Modified: trunk/Source/WebCore/bindings/js/JSDOMBindingSecurity.cpp (264446 => 264447)


--- trunk/Source/WebCore/bindings/js/JSDOMBindingSecurity.cpp	2020-07-16 04:05:29 UTC (rev 264446)
+++ trunk/Source/WebCore/bindings/js/JSDOMBindingSecurity.cpp	2020-07-16 06:32:56 UTC (rev 264447)
@@ -44,9 +44,6 @@
 
 static inline bool canAccessDocument(JSC::JSGlobalObject* lexicalGlobalObject, Document* targetDocument, SecurityReportingOption reportingOption)
 {
-    VM& vm = lexicalGlobalObject->vm();
-    auto scope = DECLARE_THROW_SCOPE(vm);
-
     if (!targetDocument)
         return false;
 
@@ -59,9 +56,12 @@
         return true;
 
     switch (reportingOption) {
-    case ThrowSecurityError:
+    case ThrowSecurityError: {
+        VM& vm = lexicalGlobalObject->vm();
+        auto scope = DECLARE_THROW_SCOPE(vm);
         throwSecurityError(*lexicalGlobalObject, scope, targetDocument->domWindow()->crossDomainAccessErrorMessage(active, IncludeTargetOrigin::No));
         break;
+    }
     case LogSecurityError:
         printErrorMessageForFrame(targetDocument->frame(), targetDocument->domWindow()->crossDomainAccessErrorMessage(active, IncludeTargetOrigin::Yes));
         break;

Modified: trunk/Source/WebCore/bindings/js/JSExecState.h (264446 => 264447)


--- trunk/Source/WebCore/bindings/js/JSExecState.h	2020-07-16 04:05:29 UTC (rev 264446)
+++ trunk/Source/WebCore/bindings/js/JSExecState.h	2020-07-16 06:32:56 UTC (rev 264447)
@@ -139,8 +139,11 @@
 
         setCurrentState(m_previousState);
 
-        if (didExitJavaScript)
+        if (didExitJavaScript) {
             didLeaveScriptContext(lexicalGlobalObject);
+            // We need to clear any exceptions from microtask drain.
+            scope.clearException();
+        }
     }
 
     static void setCurrentState(JSC::JSGlobalObject* lexicalGlobalObject)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to