Add appropriate null checking to ensure that we don't crash in certain edge cases if an SVG element's parent is set to document. Crash if the element has a transform: rdar://49890457 Crash if the element is an SVG <text> rdar://57075206
Created attachment 385640 [details] Patch
Comment on attachment 385640 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=385640&action=review > Source/WebCore/rendering/svg/RenderSVGText.cpp:375 > - } else if (m_needsTextMetricsUpdate || SVGRenderSupport::findTreeRootObject(*this)->isLayoutSizeChanged()) { > + } else if (m_needsTextMetricsUpdate || (SVGRenderSupport::findTreeRootObject(*this) && SVGRenderSupport::findTreeRootObject(*this)->isLayoutSizeChanged())) { The word "find" in the function name here is supposed to be a hint that the operation is nontrivial, so I suggest we restructure slightly so we can use a local variable and don’t need to call the function twice. Or rename the function if "find" is a guaranteed-inexpensive operation.
Created attachment 385646 [details] Patch
Comment on attachment 385646 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=385646&action=review > Source/WebCore/rendering/svg/RenderSVGText.cpp:377 > + } else if (m_needsTextMetricsUpdate) { > + RenderSVGRoot* rootObj = SVGRenderSupport::findTreeRootObject(*this); > + if (rootObj && rootObj->isLayoutSizeChanged()) { This implements &&, but in the old code it was ||.
Created attachment 385667 [details] Patch
Comment on attachment 385667 [details] Patch Looks sane to me.
Comment on attachment 385667 [details] Patch Clearing flags on attachment: 385667 Committed r253521: <https://trac.webkit.org/changeset/253521>
All reviewed patches have been landed. Closing bug.
Comment on attachment 385667 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=385667&action=review > Source/WebCore/ChangeLog:8 > + Test: svg/dom/replaceChild-document-crash.html Where is this test? I do not see it in this patch. It also did not land in r253521.
(In reply to Said Abou-Hallawa from comment #9) > Comment on attachment 385667 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=385667&action=review > > > Source/WebCore/ChangeLog:8 > > + Test: svg/dom/replaceChild-document-crash.html > > Where is this test? I do not see it in this patch. It also did not land in > r253521. Strange, my local git commit shows that file, but it's missing from the uploaded patch. Created new bug 205282 to track getting these tests in.
*** Bug 207030 has been marked as a duplicate of this bug. ***