Bug 179107 - Assert that NoEventDispatchAssertion is not in the stack when executing a script
Summary: Assert that NoEventDispatchAssertion is not in the stack when executing a script
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-11-01 00:36 PDT by Ryosuke Niwa
Modified: 2017-11-01 13:39 PDT (History)
7 users (show)

See Also:

Attachments
Adds assertion (4.59 KB, patch)
2017-11-01 00:42 PDT, Ryosuke Niwa 		simon.fraser: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2017-11-01 00:36:21 PDT 
We should assert that we're not trying to execute script when NoEventDispatchAssertion is in the stack.
Comment 1 Ryosuke Niwa 2017-11-01 00:42:12 PDT 
Created attachment 325554 [details]
Adds assertion
Comment 2 Ryosuke Niwa 2017-11-01 00:42:47 PDT 
Once this patch is landed, we can make NoEventDispatchAssertion cheap enough for the main thread to release-assert.
Comment 3 Radar WebKit Bug Importer 2017-11-01 00:43:25 PDT 
<rdar://problem/35288788>
Comment 4 Build Bot 2017-11-01 00:45:33 PDT 
Attachment 325554 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/ScriptController.cpp:681:  Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION().  [security/assertion] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Simon Fraser (smfr) 2017-11-01 10:52:40 PDT 
Comment on attachment 325554 [details]
Adds assertion

View in context: https://bugs.webkit.org/attachment.cgi?id=325554&action=review

> Source/WebCore/bindings/js/ScriptController.cpp:681
> +        ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventAllowedInMainThread());

Please add a blank line after this.
Comment 6 Ryosuke Niwa 2017-11-01 13:30:23 PDT 
(In reply to Simon Fraser (smfr) from comment #5)
> Comment on attachment 325554 [details]
> Adds assertion
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=325554&action=review
> 
> > Source/WebCore/bindings/js/ScriptController.cpp:681
> > +        ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventAllowedInMainThread());
> 
> Please add a blank line after this.

Done that.
Comment 7 Ryosuke Niwa 2017-11-01 13:36:50 PDT 
Talked with Keith (Miller) and Saam to confirm that creating event listener wouldn't run arbitrary scripts to be sure.
Comment 8 Ryosuke Niwa 2017-11-01 13:39:00 PDT 
Committed r224290: <https://trac.webkit.org/changeset/224290>