Summary: | Eliminate isMainThread() checks in most call sites of NoEventDispatchAssertion | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||
Component: | DOM | Assignee: | Ryosuke Niwa <rniwa> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bfulgham, buildbot, cdumez, ddkilzer, fpizlo, ggaren, koivisto, webkit-bug-importer, zalan | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Ryosuke Niwa
2017-11-02 00:02:10 PDT
Created attachment 325694 [details]
Refactoring
Attachment 325694 [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]
ERROR: Source/WebCore/dom/Element.cpp:2478: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5]
ERROR: Source/WebCore/dom/Element.cpp:2485: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5]
ERROR: Source/WebCore/dom/Node.cpp:2329: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5]
ERROR: Source/WebCore/dom/Node.cpp:2342: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5]
ERROR: Source/WebCore/dom/EventDispatcher.cpp:133: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5]
ERROR: Source/WebCore/dom/ContainerNode.cpp:127: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5]
ERROR: Source/WebCore/dom/ContainerNode.cpp:766: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5]
ERROR: Source/WebCore/dom/ContainerNode.cpp:783: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5]
ERROR: Source/WebCore/dom/Document.cpp:4265: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5]
ERROR: Source/WebCore/dom/Document.cpp:4273: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5]
ERROR: Source/WebCore/dom/Document.cpp:5103: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5]
Total errors found: 12 in 14 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Yay, this one passed EWS! Committed r224356: <https://trac.webkit.org/changeset/224356> (In reply to Build Bot from comment #3) > Attachment 325694 [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] > ERROR: Source/WebCore/dom/Element.cpp:2478: Please replace > ASSERT_WITH_SECURITY_IMPLICATION() with > RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] > ERROR: Source/WebCore/dom/Element.cpp:2485: Please replace > ASSERT_WITH_SECURITY_IMPLICATION() with > RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] > ERROR: Source/WebCore/dom/Node.cpp:2329: Please replace > ASSERT_WITH_SECURITY_IMPLICATION() with > RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] > ERROR: Source/WebCore/dom/Node.cpp:2342: Please replace > ASSERT_WITH_SECURITY_IMPLICATION() with > RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] > ERROR: Source/WebCore/dom/EventDispatcher.cpp:133: Please replace > ASSERT_WITH_SECURITY_IMPLICATION() with > RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] > ERROR: Source/WebCore/dom/ContainerNode.cpp:127: Please replace > ASSERT_WITH_SECURITY_IMPLICATION() with > RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] > ERROR: Source/WebCore/dom/ContainerNode.cpp:766: Please replace > ASSERT_WITH_SECURITY_IMPLICATION() with > RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] > ERROR: Source/WebCore/dom/ContainerNode.cpp:783: Please replace > ASSERT_WITH_SECURITY_IMPLICATION() with > RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] > ERROR: Source/WebCore/dom/Document.cpp:4265: Please replace > ASSERT_WITH_SECURITY_IMPLICATION() with > RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] > ERROR: Source/WebCore/dom/Document.cpp:4273: Please replace > ASSERT_WITH_SECURITY_IMPLICATION() with > RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] > ERROR: Source/WebCore/dom/Document.cpp:5103: Please replace > ASSERT_WITH_SECURITY_IMPLICATION() with > RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] > Total errors found: 12 in 14 files Why was it not possible to change these? Is there concern that this would cause a performance regression? (In reply to David Kilzer (:ddkilzer) from comment #6) > (In reply to Build Bot from comment #3) > > Attachment 325694 [details] did not pass style-queue: > > Why was it not possible to change these? Is there concern that this would > cause a performance regression? Because NoEventDispatchAssertion is a no-op in release builds right now. It's rather misleading to enable them for release because it always evaluates to true regardless. Furthermore, we can't enable NoEventDispatchAssertion::isEventDispatchAllowedInSubtree in any of interesting core DOM code since it's too expensive to do O(n) traversal. |