Commit ad443bce authored by fs's avatar fs Committed by Commit bot

Stricter treatment of SVGSVGElement::m_useCurrentView

m_useCurrentView being true implies that m_viewSpec is non-null. Perhaps
defensively, most code that checks it is written in a way such that it
expects that m_viewSpec can be null if m_useCurrentView is true - which
is not the case, and adds unnecessary code.
Instead change the code to adhere to the rule above, and assert that in
the various places instead.
Also add some checks of the invariant in methods that set the flag and
m_viewSpec. Refactor SVGSVGElement::setupInitialView to avoid the need
to sprinkle invariant checks "all over" it.

BUG=110195

Review-Url: https://codereview.chromium.org/2290293003
Cr-Commit-Position: refs/heads/master@{#415346}
parent 73583c0b
...@@ -99,7 +99,7 @@ SVGViewSpec* SVGSVGElement::currentView() ...@@ -99,7 +99,7 @@ SVGViewSpec* SVGSVGElement::currentView()
{ {
if (!m_viewSpec) if (!m_viewSpec)
m_viewSpec = SVGViewSpec::create(this); m_viewSpec = SVGViewSpec::create(this);
return m_viewSpec.get(); return m_viewSpec;
} }
float SVGSVGElement::currentScale() const float SVGSVGElement::currentScale() const
...@@ -161,8 +161,9 @@ bool SVGSVGElement::zoomAndPanEnabled() const ...@@ -161,8 +161,9 @@ bool SVGSVGElement::zoomAndPanEnabled() const
{ {
const SVGZoomAndPan* currentViewSpec = this; const SVGZoomAndPan* currentViewSpec = this;
if (m_useCurrentView) if (m_useCurrentView)
currentViewSpec = m_viewSpec.get(); currentViewSpec = m_viewSpec;
return currentViewSpec && currentViewSpec->zoomAndPan() == SVGZoomAndPanMagnify; DCHECK(currentViewSpec);
return currentViewSpec->zoomAndPan() == SVGZoomAndPanMagnify;
} }
void SVGSVGElement::parseAttribute(const QualifiedName& name, const AtomicString& oldValue, const AtomicString& value) void SVGSVGElement::parseAttribute(const QualifiedName& name, const AtomicString& oldValue, const AtomicString& value)
...@@ -568,8 +569,10 @@ bool SVGSVGElement::selfHasRelativeLengths() const ...@@ -568,8 +569,10 @@ bool SVGSVGElement::selfHasRelativeLengths() const
FloatRect SVGSVGElement::currentViewBoxRect() const FloatRect SVGSVGElement::currentViewBoxRect() const
{ {
if (m_useCurrentView) if (m_useCurrentView) {
return m_viewSpec ? m_viewSpec->viewBox()->currentValue()->value() : FloatRect(); DCHECK(m_viewSpec);
return m_viewSpec->viewBox()->currentValue()->value();
}
FloatRect useViewBox = viewBox()->currentValue()->value(); FloatRect useViewBox = viewBox()->currentValue()->value();
if (!useViewBox.isEmpty()) if (!useViewBox.isEmpty())
...@@ -628,8 +631,9 @@ float SVGSVGElement::intrinsicHeight() const ...@@ -628,8 +631,9 @@ float SVGSVGElement::intrinsicHeight() const
AffineTransform SVGSVGElement::viewBoxToViewTransform(float viewWidth, float viewHeight) const AffineTransform SVGSVGElement::viewBoxToViewTransform(float viewWidth, float viewHeight) const
{ {
if (!m_useCurrentView || !m_viewSpec) if (!m_useCurrentView)
return SVGFitToViewBox::viewBoxToViewTransform(currentViewBoxRect(), preserveAspectRatio()->currentValue(), viewWidth, viewHeight); return SVGFitToViewBox::viewBoxToViewTransform(currentViewBoxRect(), preserveAspectRatio()->currentValue(), viewWidth, viewHeight);
DCHECK(m_viewSpec);
AffineTransform ctm = SVGFitToViewBox::viewBoxToViewTransform(currentViewBoxRect(), m_viewSpec->preserveAspectRatio()->currentValue(), viewWidth, viewHeight); AffineTransform ctm = SVGFitToViewBox::viewBoxToViewTransform(currentViewBoxRect(), m_viewSpec->preserveAspectRatio()->currentValue(), viewWidth, viewHeight);
SVGTransformList* transformList = m_viewSpec->transform(); SVGTransformList* transformList = m_viewSpec->transform();
...@@ -645,41 +649,39 @@ AffineTransform SVGSVGElement::viewBoxToViewTransform(float viewWidth, float vie ...@@ -645,41 +649,39 @@ AffineTransform SVGSVGElement::viewBoxToViewTransform(float viewWidth, float vie
void SVGSVGElement::setupInitialView(const String& fragmentIdentifier, Element* anchorNode) void SVGSVGElement::setupInitialView(const String& fragmentIdentifier, Element* anchorNode)
{ {
LayoutObject* layoutObject = this->layoutObject(); if (m_viewSpec)
SVGViewSpec* view = m_viewSpec.get(); m_viewSpec->reset();
if (view)
view->reset();
bool hadUseCurrentView = m_useCurrentView; // If we previously had a view, we need to layout again, regardless of the
// state after setting.
bool needsViewUpdate = m_useCurrentView;
m_useCurrentView = false; m_useCurrentView = false;
if (fragmentIdentifier.startsWith("svgView(")) { if (fragmentIdentifier.startsWith("svgView(")) {
if (!view) SVGViewSpec* view = currentView(); // Ensure the SVGViewSpec has been created.
view = currentView(); // Create the SVGViewSpec.
view->inheritViewAttributesFromElement(this); view->inheritViewAttributesFromElement(this);
if (view->parseViewSpec(fragmentIdentifier)) { if (view->parseViewSpec(fragmentIdentifier)) {
UseCounter::count(document(), UseCounter::SVGSVGElementFragmentSVGView); UseCounter::count(document(), UseCounter::SVGSVGElementFragmentSVGView);
m_useCurrentView = true; m_useCurrentView = true;
needsViewUpdate = true;
} else { } else {
view->reset(); view->reset();
} }
} else if (isSVGViewElement(anchorNode)) {
if (layoutObject && (hadUseCurrentView || m_useCurrentView)) // Spec: If the SVG fragment identifier addresses a 'view' element
markForLayoutAndParentResourceInvalidation(layoutObject); // within an SVG document (e.g., MyDrawing.svg#MyView or
return; // MyDrawing.svg#xpointer(id('MyView'))) then the closest ancestor
} // 'svg' element is displayed in the viewport. Any view specification
// attributes included on the given 'view' element override the
// Spec: If the SVG fragment identifier addresses a 'view' element within an SVG document (e.g., MyDrawing.svg#MyView // corresponding view specification attributes on the closest ancestor
// or MyDrawing.svg#xpointer(id('MyView'))) then the closest ancestor 'svg' element is displayed in the viewport. // 'svg' element.
// Any view specification attributes included on the given 'view' element override the corresponding view specification // TODO(ed): The spec text above is a bit unclear.
// attributes on the closest ancestor 'svg' element. // Should the transform from outermost svg to nested svg be applied to
// TODO(ed): The spec text above is a bit unclear. // "display" the inner svg in the viewport, then let the view element
// Should the transform from outermost svg to nested svg be applied to "display" // override the inner svg's view specification attributes. Should it
// the inner svg in the viewport, then let the view element override the inner // fill/override the outer viewport?
// svg's view specification attributes. Should it fill/override the outer viewport?
if (isSVGViewElement(anchorNode)) {
SVGViewElement& viewElement = toSVGViewElement(*anchorNode); SVGViewElement& viewElement = toSVGViewElement(*anchorNode);
if (SVGSVGElement* svg = viewElement.ownerSVGElement()) { if (SVGSVGElement* svg = viewElement.ownerSVGElement()) {
...@@ -692,11 +694,13 @@ void SVGSVGElement::setupInitialView(const String& fragmentIdentifier, Element* ...@@ -692,11 +694,13 @@ void SVGSVGElement::setupInitialView(const String& fragmentIdentifier, Element*
} }
} }
// If we previously had a view and didn't get a new one, we need to LayoutObject* layoutObject = this->layoutObject();
// layout again. if (layoutObject && needsViewUpdate)
if (layoutObject && hadUseCurrentView)
markForLayoutAndParentResourceInvalidation(layoutObject); markForLayoutAndParentResourceInvalidation(layoutObject);
// If m_useCurrentView is true we should have a view-spec.
DCHECK(!m_useCurrentView || m_viewSpec);
// FIXME: We need to decide which <svg> to focus on, and zoom to it. // FIXME: We need to decide which <svg> to focus on, and zoom to it.
// FIXME: We need to actually "highlight" the viewTarget(s). // FIXME: We need to actually "highlight" the viewTarget(s).
} }
...@@ -708,6 +712,7 @@ void SVGSVGElement::inheritViewAttributes(SVGViewElement* viewElement) ...@@ -708,6 +712,7 @@ void SVGSVGElement::inheritViewAttributes(SVGViewElement* viewElement)
UseCounter::count(document(), UseCounter::SVGSVGElementFragmentSVGViewElement); UseCounter::count(document(), UseCounter::SVGSVGElementFragmentSVGViewElement);
view->inheritViewAttributesFromElement(this); view->inheritViewAttributesFromElement(this);
view->inheritViewAttributesFromElement(viewElement); view->inheritViewAttributesFromElement(viewElement);
DCHECK(!m_useCurrentView || m_viewSpec);
} }
void SVGSVGElement::finishParsingChildren() void SVGSVGElement::finishParsingChildren()
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment