commit 9e59dae7fdb9527f688b03fa314e917712024d4b Author: Kathy Brade brade@pearlcrescent.com Date: Tue Jul 21 15:46:12 2015 -0400

fixup! Bug 12827: Create preference to disable SVG.

Add NS_SVGEnabled() checks to the nsIContent::IsSVG() methods to avoid crashing due to code that assumes elements with an SVG namespace are always represented by nsSVGElement objects (some existing Mozilla code uses static_cast to convert to an nsSVGElement pointer). Also, cache the SVG enabled/disabled status within each document so that it persists until the document is reloaded.

Also fix another case where the HTML parser failed to check for a failed QI.

Fixes ticket #16495. --- dom/base/Element.cpp | 20 ++++++++++++++++ dom/base/nsDocument.cpp | 5 +++- dom/base/nsIContent.h | 10 ++------ dom/base/nsIDocument.h | 25 ++++++++++++++++++++ layout/svg/nsSVGUtils.cpp | 39 +++++++++++++++++++++++++++----- layout/svg/nsSVGUtils.h | 1 + parser/html/nsHtml5DocumentBuilder.cpp | 24 ++++++++++---------- 7 files changed, 97 insertions(+), 27 deletions(-)

diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index 02035f4..bd795b2 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -143,6 +143,26 @@ using namespace mozilla; using namespace mozilla::dom;

+// These IsSVG() methods were moved here from nsIContent.h because including +// nsSVGUtils.h in nsIContent.h (needed to to pick up the NS_SVGEnabled() +// prototype) creates a circular dependency: nsSVGUtils.h includes other +// headers that define functions that require the complete definition of +// nsPresContext... but nsPresContext.h includes nsIPresShell.h, which in turn +// includes nsIContent.h. +bool +nsIContent::IsSVG() const +{ + return NS_SVGEnabled(mNodeInfo->GetDocument()) && + IsInNamespace(kNameSpaceID_SVG); +} + +bool +nsIContent::IsSVG(nsIAtom* aTag) const +{ + return NS_SVGEnabled(mNodeInfo->GetDocument()) && + mNodeInfo->Equals(aTag, kNameSpaceID_SVG); +} + nsIAtom* nsIContent::DoGetID() const { diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 47f611e..c30306f 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -1575,7 +1575,8 @@ nsIDocument::nsIDocument() mIsBeingUsedAsImage(false), mHasLinksToUpdate(false), mPartID(0), - mDidFireDOMContentLoaded(true) + mDidFireDOMContentLoaded(true), + mSVGStatus(mozilla::dom::SVGStatus_Unknown) { SetInDocument();

@@ -2420,6 +2421,8 @@ nsDocument::ResetToURI(nsIURI *aURI, nsILoadGroup *aLoadGroup,

mXMLDeclarationBits = 0;

+ mSVGStatus = SVGStatus_Unknown; + // Now get our new principal if (aPrincipal) { SetPrincipal(aPrincipal); diff --git a/dom/base/nsIContent.h b/dom/base/nsIContent.h index b0a251f3..135e79e 100644 --- a/dom/base/nsIContent.h +++ b/dom/base/nsIContent.h @@ -274,15 +274,9 @@ public: return mNodeInfo->Equals(aTag, kNameSpaceID_XHTML); }

- inline bool IsSVG() const - { - return IsInNamespace(kNameSpaceID_SVG); - } + bool IsSVG() const;

- inline bool IsSVG(nsIAtom* aTag) const - { - return mNodeInfo->Equals(aTag, kNameSpaceID_SVG); - } + bool IsSVG(nsIAtom* aTag) const;

inline bool IsXUL() const { diff --git a/dom/base/nsIDocument.h b/dom/base/nsIDocument.h index 8c8fd7b..201d59f 100644 --- a/dom/base/nsIDocument.h +++ b/dom/base/nsIDocument.h @@ -143,6 +143,12 @@ struct FullScreenOptions { nsRefPtrgfx::VRHMDInfo mVRHMDDevice; };

+typedef enum { + SVGStatus_Unknown = 0, + SVGStatus_Enabled, + SVGStatus_Disabled +} SVGStatus; + } // namespace dom } // namespace mozilla

@@ -617,6 +623,22 @@ public: }

/** + * Get the cached SVG status for this document. + */ + mozilla::dom::SVGStatus GetSVGStatus() const + { + return mSVGStatus; + } + + /** + * Set the cached SVG status for this document. + */ + void SetSVGStatus(mozilla::dom::SVGStatus svgStatus) + { + mSVGStatus = svgStatus; + } + + /** * Access HTTP header data (this may also get set from other * sources, like HTML META tags). */ @@ -2874,6 +2896,9 @@ protected:

// Our live MediaQueryLists PRCList mDOMMediaQueryLists; + + // Cached value that indicates whether SVG is enabled for this document. + mozilla::dom::SVGStatus mSVGStatus; };

NS_DEFINE_STATIC_IID_ACCESSOR(nsIDocument, NS_IDOCUMENT_IID) diff --git a/layout/svg/nsSVGUtils.cpp b/layout/svg/nsSVGUtils.cpp index 0b820ee..c2b6608 100644 --- a/layout/svg/nsSVGUtils.cpp +++ b/layout/svg/nsSVGUtils.cpp @@ -54,6 +54,7 @@ #include "nsContentUtils.h" #include "SVGContentUtils.h" #include "mozilla/unused.h" +#include "nsIDocument.h"

using namespace mozilla; using namespace mozilla::dom; @@ -67,11 +68,27 @@ static bool sSVGNewGetBBoxEnabled;

// Determine if SVG should be enabled for aDoc. The svg.in-content.enabled // preference is checked as well as whether aDoc is a content or chrome doc. -// If aChannel is NULL, the pref. value is returned. +// If aDoc is NULL, the pref. value is returned. +// Once we determine whether SVG is allowed for a given document, we record +// that fact inside the document. This is necessary to avoid crashes due +// to code that uses static_cast to cast an element object to an nsSVGElement +// object. When SVG is disabled, <svg> and related tags are not represented +// by nsSVGElement objects. bool NS_SVGEnabled(nsIDocument *aDoc) { - return NS_SVGEnabledForChannel(aDoc ? aDoc->GetChannel() : nullptr); + if (!aDoc) + return NS_SVGEnabledForChannel(nullptr); + + mozilla::dom::SVGStatus svgStatus = aDoc->GetSVGStatus(); + if (svgStatus == mozilla::dom::SVGStatus_Unknown) + { + svgStatus = NS_SVGEnabledForChannel(aDoc->GetChannel()) ? + mozilla::dom::SVGStatus_Enabled : mozilla::dom::SVGStatus_Disabled; + aDoc->SetSVGStatus(svgStatus); + } + + return (svgStatus == mozilla::dom::SVGStatus_Enabled); }

// Determine if SVG should be enabled for aChannel. The svg.in-content.enabled @@ -948,10 +965,11 @@ nsSVGUtils::GetBBox(nsIFrame *aFrame, uint32_t aFlags) // needs investigation to check that we won't break too much content. // NOTE: When changing this to apply to other frame types, make sure to // also update nsSVGUtils::FrameSpaceInCSSPxToUserSpaceOffset. - MOZ_ASSERT(content->IsSVG(), "bad cast"); - nsSVGElement *element = static_cast<nsSVGElement*>(content); - matrix = element->PrependLocalTransformsTo(matrix, + if (content->IsSVG()) { + nsSVGElement *element = static_cast<nsSVGElement*>(content); + matrix = element->PrependLocalTransformsTo(matrix, nsSVGElement::eChildToUserSpace); + } } bbox = svg->GetBBoxContribution(ToMatrix(matrix), aFlags).ToThebesRect(); // Account for 'clipped'. @@ -1151,7 +1169,9 @@ nsSVGUtils::GetNonScalingStrokeTransform(nsIFrame *aFrame, }

nsIContent *content = aFrame->GetContent(); - MOZ_ASSERT(content->IsSVG(), "bad cast"); + if (!content->IsSVG()) { + return false; + }

*aUserToOuterSVG = ThebesMatrix(SVGContentUtils::GetCTM( static_cast<nsSVGElement*>(content), true)); @@ -1442,6 +1462,10 @@ nsSVGUtils::GetStrokeWidth(nsIFrame* aFrame, gfxTextContextPaint *aContextPaint) content = content->GetParent(); }

+ if (!aFrame->GetContent()->IsSVG()) { + return 0.0; + } + nsSVGElement *ctx = static_cast<nsSVGElement*>(content);

return SVGContentUtils::CoordToFloat(ctx, style->mStrokeWidth); @@ -1455,6 +1479,9 @@ GetStrokeDashData(nsIFrame* aFrame, { const nsStyleSVG* style = aFrame->StyleSVG(); nsIContent *content = aFrame->GetContent(); + if (!content->IsSVG()) { + return false; + } nsSVGElement *ctx = static_cast<nsSVGElement*> (content->IsNodeOfType(nsINode::eTEXT) ? content->GetParent() : content); diff --git a/layout/svg/nsSVGUtils.h b/layout/svg/nsSVGUtils.h index 7b3edbb..16e73b6 100644 --- a/layout/svg/nsSVGUtils.h +++ b/layout/svg/nsSVGUtils.h @@ -30,6 +30,7 @@ class gfxContext; class gfxPattern; class nsFrameList; +class nsIChannel; class nsIContent; class nsIDocument; class nsIFrame; diff --git a/parser/html/nsHtml5DocumentBuilder.cpp b/parser/html/nsHtml5DocumentBuilder.cpp index 08e4d8a..4a705c5 100644 --- a/parser/html/nsHtml5DocumentBuilder.cpp +++ b/parser/html/nsHtml5DocumentBuilder.cpp @@ -67,18 +67,18 @@ nsHtml5DocumentBuilder::UpdateStyleSheet(nsIContent* aElement) }

nsCOMPtr<nsIStyleSheetLinkingElement> ssle(do_QueryInterface(aElement)); - NS_ASSERTION(ssle, "Node didn't QI to style."); - - ssle->SetEnableUpdates(true); - - bool willNotify; - bool isAlternate; - nsresult rv = ssle->UpdateStyleSheet(mRunsToCompletion ? nullptr : this, - &willNotify, - &isAlternate); - if (NS_SUCCEEDED(rv) && willNotify && !isAlternate && !mRunsToCompletion) { - ++mPendingSheetCount; - mScriptLoader->AddExecuteBlocker(); + if (ssle) { + ssle->SetEnableUpdates(true); + + bool willNotify; + bool isAlternate; + nsresult rv = ssle->UpdateStyleSheet(mRunsToCompletion ? nullptr : this, + &willNotify, + &isAlternate); + if (NS_SUCCEEDED(rv) && willNotify && !isAlternate && !mRunsToCompletion) { + ++mPendingSheetCount; + mScriptLoader->AddExecuteBlocker(); + } }

if (aElement->IsHTML(nsGkAtoms::link)) {