[webkit-reviews] review granted: [Bug 137128] Add support for is<SVGDocument>() / downcast<SVGDocument>() : [Attachment 238671] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 137128: Add support for is<SVGDocument>() / downcast<SVGDocument>()
https://bugs.webkit.org/show_bug.cgi?id=137128

Attachment 238671: Patch
https://bugs.webkit.org/attachment.cgi?id=238671&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=238671&action=review


> Source/WebCore/page/EventHandler.cpp:763
> +    if (is<SVGDocument>(m_frame.document())
> +	   && downcast<SVGDocument>(*m_frame.document()).zoomAndPanEnabled()) {


We can probably fit this in one line instead.

> Source/WebCore/page/Frame.cpp:971
> +    if (is<SVGDocument>(document)) {
> +	   if (!downcast<SVGDocument>(*document).zoomAndPanEnabled())

Why don't we check both of these conditions in one if statement instead?

> Source/WebCore/page/FrameView.cpp:1964
> +    if (is<SVGDocument>(frame().document())) {
> +	   if (SVGSVGElement* svg =
downcast<SVGDocument>(*frame().document()).rootElement()) {

Ditto.

> Source/WebCore/svg/graphics/SVGImage.cpp:71
> -    SVGSVGElement* rootElement =
toSVGDocument(m_page->mainFrame().document())->rootElement();
> +    SVGSVGElement* rootElement =
downcast<SVGDocument>(*m_page->mainFrame().document()).rootElement();

Should we add a helper function to get the root element or nullptr?
Repeating the code over and over isn't so great...