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

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>