Summary: | Do not assert when CharacterData representing an Attr fires events | ||
---|---|---|---|
Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> |
Component: | WebCore Misc. | Assignee: | Brent Fulgham <bfulgham> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | bfulgham, buildbot, cdumez, commit-queue, dbates, esprehn+autocc, kangil.han, rniwa |
Priority: | P2 | Keywords: | InRadar |
Version: | WebKit Nightly Build | ||
Hardware: | All | ||
OS: | All | ||
Attachments: |
Description
Brent Fulgham
2017-04-04 09:52:05 PDT
Created attachment 306178 [details]
Patch
Comment on attachment 306178 [details]
Patch
You have to wrap the both statements in if !DISABLED_ASSERT because we don't want to be allocating the object in release builds.
(In reply to Ryosuke Niwa from comment #3) > Comment on attachment 306178 [details] > Patch > > You have to wrap the both statements in if !DISABLED_ASSERT because we don't > want to be allocating the object in release builds. Is that because I made it a uniqueptr? Or should it have been !ASSERT_DISABLED the entire time? Created attachment 306180 [details]
Patch
Created attachment 306185 [details]
Patch
Comment on attachment 306185 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=306185&action=review > LayoutTests/fast/dom/no-assert-for-malformed-js-url-attribute.html:14 > + testFrame.getAttributeNode("src").firstChild.appendData("javascript: missingFunction(this) orem ipsum dosolorem"); Does this really cover the assertion? I would be surprised since the resulting URL would be "foojavascript:..." which is not a javascript URL. Comment on attachment 306185 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=306185&action=review >> LayoutTests/fast/dom/no-assert-for-malformed-js-url-attribute.html:14 >> + testFrame.getAttributeNode("src").firstChild.appendData("javascript: missingFunction(this) orem ipsum dosolorem"); > > Does this really cover the assertion? I would be surprised since the resulting URL would be "foojavascript:..." which is not a javascript URL. We may want to keep both tests. This one crashes as well but the backtrace is slightly different given that it is not treated as a javascript URL: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x0000000117bc9ee4 WTFCrash + 36 1 com.apple.JavaScriptCore 0x0000000117bc9ef9 WTFCrashWithSecurityImplication + 9 2 com.apple.WebCore 0x000000010aec7ddf WebCore::EventTarget::fireEventListeners(WebCore::Event&) + 79 3 com.apple.WebCore 0x000000010adce903 WebCore::DOMWindow::dispatchEvent(WebCore::Event&, WebCore::EventTarget*) + 435 4 com.apple.WebCore 0x000000010b08acbb WebCore::FrameLoader::dispatchUnloadEvents(WebCore::UnloadEventPolicy) + 379 5 com.apple.WebCore 0x000000010b08aa65 WebCore::FrameLoader::stopLoading(WebCore::UnloadEventPolicy) + 133 6 com.apple.WebCore 0x000000010c588459 WebCore::NavigationScheduler::schedule(std::__1::unique_ptr<WebCore::ScheduledNavigation, std::__1::default_delete<WebCore::ScheduledNavigation> >) + 281 7 com.apple.WebCore 0x000000010c588ed5 WebCore::NavigationScheduler::scheduleLocationChange(WebCore::Document&, WebCore::SecurityOrigin&, WebCore::URL const&, WTF::String const&, WebCore::LockHistory, WebCore::LockBackForwardList) + 1765 8 com.apple.WebCore 0x000000010cea7117 WebCore::SubframeLoader::loadOrRedirectSubframe(WebCore::HTMLFrameOwnerElement&, WebCore::URL const&, WTF::AtomicString const&, WebCore::LockHistory, WebCore::LockBackForwardList) + 263 9 com.apple.WebCore 0x000000010cea6e9e WebCore::SubframeLoader::requestFrame(WebCore::HTMLFrameOwnerElement&, WTF::String const&, WTF::AtomicString const&, WebCore::LockHistory, WebCore::LockBackForwardList) + 350 10 com.apple.WebCore 0x000000010b2ad611 WebCore::HTMLFrameElementBase::openURL(WebCore::LockHistory, WebCore::LockBackForwardList) + 257 11 com.apple.WebCore 0x000000010b2ad9b9 WebCore::HTMLFrameElementBase::setLocation(WTF::String const&) + 185 12 com.apple.WebCore 0x000000010b2ad6e1 WebCore::HTMLFrameElementBase::parseAttribute(WebCore::QualifiedName const&, WTF::AtomicString const&) + 193 13 com.apple.WebCore 0x000000010b2b2c09 WebCore::HTMLIFrameElement::parseAttribute(WebCore::QualifiedName const&, WTF::AtomicString const&) + 393 14 com.apple.WebCore 0x000000010ae5e4d9 WebCore::Element::attributeChanged(WebCore::QualifiedName const&, WTF::AtomicString const&, WTF::AtomicString const&, WebCore::Element::AttributeModificationReason) + 793 15 com.apple.WebCore 0x000000010ce2aa2f WebCore::StyledElement::attributeChanged(WebCore::QualifiedName const&, WTF::AtomicString const&, WTF::AtomicString const&, WebCore::Element::AttributeModificationReason) + 223 Created attachment 306187 [details]
Patch
Comment on attachment 306187 [details] Patch Attachment 306187 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3473329 New failing tests: fast/dom/no-assert-for-malformed-js-url-attribute.html Created attachment 306197 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 306187 [details] Patch Attachment 306187 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3473341 New failing tests: fast/dom/no-assert-for-malformed-js-url-attribute.html Created attachment 306198 [details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Look like my revised test output is wrong. I'll rebase before landing. Comment on attachment 306187 [details] Patch Attachment 306187 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3473351 New failing tests: fast/dom/no-assert-for-malformed-js-url-attribute.html Created attachment 306200 [details]
Archive of layout-test-results from ews117 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 306187 [details] Patch Attachment 306187 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3473377 New failing tests: fast/dom/no-assert-for-malformed-js-url-attribute.html Created attachment 306208 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 306225 [details]
Patch for Landing
The commit-queue encountered the following flaky tests while processing attachment 306225 [details]: media/modern-media-controls/macos-fullscreen-media-controls/macos-fullscreen-media-controls-buttons-styles.html bug 168317 (author: graouts@apple.com) The commit-queue is continuing to process your patch. Comment on attachment 306225 [details] Patch for Landing Clearing flags on attachment: 306225 Committed r214915: <http://trac.webkit.org/changeset/214915> All reviewed patches have been landed. Closing bug. |