Summary: | RenderSVGModelObject::checkIntersection triggers layout | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||||||||
Component: | SVG | Assignee: | Ryosuke Niwa <rniwa> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, koivisto, rniwa, sabouhallawa, simon.fraser, zalan, zimmermann | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Ryosuke Niwa
2017-10-23 22:49:20 PDT
Created attachment 324646 [details]
Fixes the bug
In particular, collectIntersectionOrEnclosureList uses ElementIterator so it's not safe to update the layout which could run arbitrary scripts at the moment... Comment on attachment 324646 [details] Fixes the bug Clearing flags on attachment: 324646 Committed r223882: <https://trac.webkit.org/changeset/223882> All reviewed patches have been landed. Closing bug. Comment on attachment 324646 [details]
Fixes the bug
This doesn't seem right. getElementCTM() can be called from RenderSVGModelObject::checkIntersection, which is called from SVGSVGElement::checkIntersection which is exposed to JS. So now nothing forces layout before checkIntersection calls getElementCTM.
(In reply to Simon Fraser (smfr) from comment #6) > Comment on attachment 324646 [details] > Fixes the bug > > This doesn't seem right. getElementCTM() can be called from > RenderSVGModelObject::checkIntersection, which is called from > SVGSVGElement::checkIntersection which is exposed to JS. So now nothing > forces layout before checkIntersection calls getElementCTM. SVGSVGElement::checkIntersection triggers layout! https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/svg/SVGSVGElement.cpp#L336 (In reply to Ryosuke Niwa from comment #7) > (In reply to Simon Fraser (smfr) from comment #6) > > Comment on attachment 324646 [details] > > Fixes the bug > > > > This doesn't seem right. getElementCTM() can be called from > > RenderSVGModelObject::checkIntersection, which is called from > > SVGSVGElement::checkIntersection which is exposed to JS. So now nothing > > forces layout before checkIntersection calls getElementCTM. > > SVGSVGElement::checkIntersection triggers layout! > https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/svg/ > SVGSVGElement.cpp#L336 That's getIntersectionList, not checkIntersection. (In reply to Simon Fraser (smfr) from comment #8) > (In reply to Ryosuke Niwa from comment #7) > > (In reply to Simon Fraser (smfr) from comment #6) > > > Comment on attachment 324646 [details] > > > Fixes the bug > > > > > > This doesn't seem right. getElementCTM() can be called from > > > RenderSVGModelObject::checkIntersection, which is called from > > > SVGSVGElement::checkIntersection which is exposed to JS. So now nothing > > > forces layout before checkIntersection calls getElementCTM. > > > > SVGSVGElement::checkIntersection triggers layout! > > https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/svg/ > > SVGSVGElement.cpp#L336 > > That's getIntersectionList, not checkIntersection. Oh, didn't notice those were exposed directly. Let's fix that... Re-opening to address Simon's comment. Created attachment 324720 [details]
Address Simon's comment with a test
Comment on attachment 324720 [details]
Address Simon's comment with a test
Posting a new patch to land.
Created attachment 324727 [details]
Patch for landing
Comment on attachment 324727 [details] Patch for landing Attachment 324727 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4975689 New failing tests: storage/indexeddb/detached-iframe.html Created attachment 324744 [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
Comment on attachment 324727 [details] Patch for landing Attachment 324727 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4975525 New failing tests: storage/indexeddb/detached-iframe.html Created attachment 324745 [details]
Archive of layout-test-results from ews116 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 324746 [details]
Patch for landing
Committed r223947: <https://trac.webkit.org/changeset/223947> |