Bug 178710 – RenderSVGModelObject::checkIntersection triggers layout

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Ryosuke Niwa]

Don't do that.

[Ryosuke Niwa]

Created attachment 324646 [details]
Fixes the bug

[Ryosuke Niwa]

<rdar://problem/35140862>

[Ryosuke Niwa]

In particular, collectIntersectionOrEnclosureList uses ElementIterator so it's not safe to update the layout which could run arbitrary scripts at the moment...

[WebKit Commit Bot]

Comment on attachment 324646 [details]
Fixes the bug

Clearing flags on attachment: 324646

Committed r223882: <https://trac.webkit.org/changeset/223882>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

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.

[Ryosuke Niwa]

(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

[Simon Fraser (smfr)]

(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.

[Ryosuke Niwa]

(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...

[Ryosuke Niwa]

Re-opening to address Simon's comment.

[Ryosuke Niwa]

Created attachment 324720 [details]
Address Simon's comment with a test

[Ryosuke Niwa]

Comment on attachment 324720 [details]
Address Simon's comment with a test

Posting a new patch to land.

[Ryosuke Niwa]

Created attachment 324727 [details]
Patch for landing

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Ryosuke Niwa]

Created attachment 324746 [details]
Patch for landing

[Ryosuke Niwa]

Committed r223947: <https://trac.webkit.org/changeset/223947>