[webkit-reviews] review granted: [Bug 179314] Replace some stack raw pointers with RefPtrs within WebCore/svg : [Attachment 326168] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Nov 7 22:09:00 PST 2017 

Ryosuke Niwa <rniwa at webkit.org> has granted Jiewen Tan <jiewen_tan at apple.com>'s
request for review:
Bug 179314: Replace some stack raw pointers with RefPtrs within WebCore/svg
https://bugs.webkit.org/show_bug.cgi?id=179314

Attachment 326168: Patch

https://bugs.webkit.org/attachment.cgi?id=326168&action=review




--- Comment #7 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 326168
  --> https://bugs.webkit.org/attachment.cgi?id=326168
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=326168&action=review

> Source/WebCore/rendering/svg/RenderSVGModelObject.cpp:119
> -    SVGElement* stopAtElement =
SVGLocatable::nearestViewportElement(element);
> +    auto* stopAtElement = SVGLocatable::nearestViewportElement(element);

Why are we not using RefPtr here?

> Source/WebCore/svg/SVGAElement.cpp:128
> -		   Element* targetElement =
treeScope().getElementById(url.substringSharingImpl(1));
> +		   RefPtr<Element> targetElement =
treeScope().getElementById(url.substringSharingImpl(1));

Use makeRefPtr?

> Source/WebCore/svg/SVGAElement.cpp:144
> -	       Frame* frame = document().frame();
> +	       RefPtr<Frame> frame = document().frame();

Use makeRefPtr?

> Source/WebCore/svg/SVGAnimateElementBase.cpp:87
> -    SVGElement* targetElement = this->targetElement();
> +    RefPtr<SVGElement> targetElement = this->targetElement();

Ditto.

> Source/WebCore/svg/SVGAnimateElementBase.cpp:189
> -    SVGElement* targetElement = this->targetElement();
> +    RefPtr<SVGElement> targetElement = this->targetElement();

Ditto.

> Source/WebCore/svg/SVGAnimateElementBase.cpp:339
> -    SVGElement* targetElement = this->targetElement();
> +    RefPtr<SVGElement> targetElement = this->targetElement();

Ditto.

> Source/WebCore/svg/SVGAnimateMotionElement.cpp:59
> -    SVGElement* targetElement = this->targetElement();
> +    RefPtr<SVGElement> targetElement = this->targetElement();

Ditto.

> Source/WebCore/svg/SVGAnimateMotionElement.cpp:168
> -    SVGElement* targetElement = this->targetElement();
> +    RefPtr<SVGElement> targetElement = this->targetElement();

Ditto.

> Source/WebCore/svg/SVGAnimateMotionElement.cpp:-234
> -    SVGElement* targetElement = this->targetElement();

Ditto.

> Source/WebCore/svg/SVGAnimateMotionElement.cpp:-274
> -    SVGElement* targetElement = this->targetElement();

Ditto.

> Source/WebCore/svg/SVGAnimationElement.cpp:646
> -    Element* parent = targetElement->parentElement();
> +    RefPtr<Element> parent = targetElement->parentElement();

Ditto.

> Source/WebCore/svg/SVGAnimationElement.cpp:665
> -    SVGElement* targetElement = this->targetElement();
> +    RefPtr<SVGElement> targetElement = this->targetElement();

Ditto.

> Source/WebCore/svg/SVGElement.cpp:291
> -	   if (SVGElement* correspondingElement =
m_svgRareData->correspondingElement())
> +	   if (RefPtr<SVGElement> correspondingElement =
m_svgRareData->correspondingElement())

Ditto.

> Source/WebCore/svg/SVGElement.cpp:449
> -	   if (SVGElement* oldCorrespondingElement =
m_svgRareData->correspondingElement())
> +	   if (RefPtr<SVGElement> oldCorrespondingElement =
m_svgRareData->correspondingElement())

Ditto.

> Source/WebCore/svg/SVGElement.cpp:762
> -    if (Element* parent = parentOrShadowHostElement()) {
> +    if (RefPtr<Element> parent = parentOrShadowHostElement()) {

Ditto.

> Source/WebCore/svg/SVGElement.cpp:1089
> -    ContainerNode* node = parentNode();
> +    RefPtr<ContainerNode> node = parentNode();

Ditto.

> Source/WebCore/svg/SVGElement.cpp:1132
> +	   RefPtr<SVGElement> instance = *instances.begin();

Ditto.

> Source/WebCore/svg/SVGFEImageElement.cpp:105
> -    Element* target = SVGURIReference::targetElementFromIRIString(href(),
document(), &id);
> +    RefPtr<Element> target =
SVGURIReference::targetElementFromIRIString(href(), document(), &id);

Ditto.

> Source/WebCore/svg/SVGFEImageElement.cpp:176
> -    Element* parent = parentElement();
> +    RefPtr<Element> parent = parentElement();

Ditto.

> Source/WebCore/svg/SVGFELightElement.cpp:82
> -    SVGFELightElement* lightNode = findLightElement(svgElement);
> +    RefPtr<SVGFELightElement> lightNode = findLightElement(svgElement);

Ditto.

> Source/WebCore/svg/SVGFELightElement.cpp:178
> -    ContainerNode* parent = parentNode();
> +    RefPtr<ContainerNode> parent = parentNode();

Ditto.

> Source/WebCore/svg/SVGFilterPrimitiveStandardAttributes.cpp:157
> -    ContainerNode* parent = element->parentNode();
> +    RefPtr<ContainerNode> parent = element->parentNode();

Ditto.

> Source/WebCore/svg/SVGFontFaceElement.cpp:266
> -	       if (CSSFontFaceSrcValue* item =
downcast<CSSFontFaceSrcValue>(srcList->itemWithoutBoundsCheck(i)))
> +	       if (RefPtr<CSSFontFaceSrcValue> item =
downcast<CSSFontFaceSrcValue>(srcList->itemWithoutBoundsCheck(i)))

Ditto.

> Source/WebCore/svg/SVGFontFaceFormatElement.cpp:50
> -    ContainerNode* ancestor = parentNode()->parentNode();
> +    RefPtr<ContainerNode> ancestor = parentNode()->parentNode();

Ditto.

> Source/WebCore/svg/SVGFontFaceUriElement.cpp:79
> -    ContainerNode* grandparent = parentNode()->parentNode();
> +    RefPtr<ContainerNode> grandparent = parentNode()->parentNode();

Ditto.

> Source/WebCore/svg/SVGForeignObjectElement.cpp:145
> -    Element* ancestor = parentElement();
> +    RefPtr<Element> ancestor = parentElement();

Ditto.

> Source/WebCore/svg/SVGLengthContext.cpp:304
> -    SVGElement* viewportElement = m_context->viewportElement();
> +    RefPtr<SVGElement> viewportElement = m_context->viewportElement();

Ditto.

> Source/WebCore/svg/SVGLinearGradientElement.cpp:165
> -	   Node* refNode =
SVGURIReference::targetElementFromIRIString(current->href(), document());
> +	   RefPtr<Node> refNode =
SVGURIReference::targetElementFromIRIString(current->href(), document());

Ditto.

> Source/WebCore/svg/SVGMPathElement.cpp:65
> -    Element* target = SVGURIReference::targetElementFromIRIString(href(),
document(), &id);
> +    RefPtr<Element> target =
SVGURIReference::targetElementFromIRIString(href(), document(), &id);

Ditto.

> Source/WebCore/svg/SVGRadialGradientElement.cpp:181
> -	   Node* refNode =
SVGURIReference::targetElementFromIRIString(current->href(), document());
> +	   RefPtr<Node> refNode =
SVGURIReference::targetElementFromIRIString(current->href(), document());

Ditto.

> Source/WebCore/svg/SVGSVGElement.cpp:175
> -    Frame* frame = document().frame();
> +    RefPtr<Frame> frame = document().frame();

Ditto.

> Source/WebCore/svg/SVGSVGElement.cpp:376
> +    if (RefPtr<Frame> frame = document().frame())

Ditto.

> Source/WebCore/svg/SVGSVGElement.cpp:456
> -	       if (FrameView* view = document().view()) {
> +	       if (RefPtr<FrameView> view = document().view()) {

Ditto.

> Source/WebCore/svg/SVGSVGElement.cpp:641
> -    SVGViewSpec* view = m_viewSpec.get();
> +    RefPtr<SVGViewSpec> view(m_viewSpec);

Ditto.

> Source/WebCore/svg/SVGSVGElement.cpp:725
> -    Element* element = treeScope().getElementById(id);
> +    RefPtr<Element> element = treeScope().getElementById(id);

Ditto.

> Source/WebCore/svg/SVGStyleElement.cpp:58
> -    if (CSSStyleSheet* styleSheet = sheet())
> +    if (RefPtr<CSSStyleSheet> styleSheet = sheet())

Ditto.

> Source/WebCore/svg/SVGTRefElement.cpp:166
> -    Node* container = shadowRoot()->firstChild();
> +    RefPtr<Node> container = shadowRoot()->firstChild();

Ditto.

> Source/WebCore/svg/SVGTextPathElement.cpp:159
> -    Element* target = SVGURIReference::targetElementFromIRIString(href(),
document(), &id);
> +    RefPtr<Element> target =
SVGURIReference::targetElementFromIRIString(href(), document(), &id);

Ditto.

> Source/WebCore/svg/SVGUseElement.cpp:145
> -	   SVGElement* correspondingElement =
shadowElement.correspondingElement();
> +	   RefPtr<SVGElement> correspondingElement =
shadowElement.correspondingElement();

Ditto.

> Source/WebCore/svg/animation/SVGSMILElement.cpp:266
> -    SVGSVGElement* owner = ownerSVGElement();
> +    RefPtr<SVGSVGElement> owner = ownerSVGElement();

Ditto.

> Source/WebCore/svg/animation/SVGSMILElement.cpp:547
> -	       Element* eventBase = eventBaseFor(condition);
> +	       RefPtr<Element> eventBase = eventBaseFor(condition);

Ditto.

> Source/WebCore/svg/animation/SVGSMILElement.cpp:582
> -	       Element* eventBase = eventBaseFor(condition);
> +	       RefPtr<Element> eventBase = eventBaseFor(condition);

Ditto.

> Source/WebCore/svg/graphics/SVGImage.cpp:288
> -    FrameView* view = frameView();
> +    RefPtr<FrameView> view = frameView();

Ditto.

> Source/WebCore/svg/graphics/SVGImage.cpp:412
> -    Document* document = m_page->mainFrame().document();
> +    RefPtr<Document> document = m_page->mainFrame().document();

Ditto.

> Source/WebCore/svg/graphics/filters/SVGFEImage.cpp:121
> +	   RefPtr<SVGElement> contextNode =
downcast<SVGElement>(renderer->element());

Ditto.

> Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h:93
> -	   SVGPathElement* pathElement =
downcast<SVGPathElement>(contextElement());
> +	   RefPtr<SVGPathElement> pathElement =
downcast<SVGPathElement>(contextElement());

Ditto.

> Source/WebCore/svg/properties/SVGListPropertyTearOff.h:144
> -	       ListItemTearOff* item = m_wrappers->at(i);
> +	       RefPtr<ListItemTearOff> item = m_wrappers->at(i);

Ditto.

> Source/WebCore/svg/properties/SVGListPropertyTearOff.h:162
> -	   SVGAnimatedProperty* animatedPropertyOfItem =
newItem->animatedProperty();
> +	   RefPtr<SVGAnimatedProperty> animatedPropertyOfItem =
newItem->animatedProperty();

Ditto.

More information about the webkit-reviews mailing list