[webkit-reviews] review granted: [Bug 178201] Replace some stack raw pointers with RefPtrs within WebCore/html : [Attachment 323736] Patch

[bugzilla-daemon at webkit.org]

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

Attachment 323736: Patch

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




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

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

> Source/WebCore/html/FormAssociatedElement.cpp:113
> +	       newForm = downcast<HTMLFormElement>(newFormCandidate.get());

Why don't we just return this?

> Source/WebCore/html/FormAssociatedElement.cpp:114
> +	   return newForm.get();

And return nullptr here instead. That way, we can just get rid of newForm.

> Source/WebCore/html/HTMLFormControlElement.cpp:451
> -	   if (HTMLFormElement* form = this->form())
> +	   if (RefPtr<HTMLFormElement> form = this->form())

We should make form() return a RefPtr<HTMLFormElement> and just use auto here.

> Source/WebCore/html/HTMLFormElement.cpp:143
> -    Node* targetNode = event.target()->toNode();
> +    RefPtr<Node> targetNode = event.target()->toNode();

Can we make toNode return RefPtr<Node> instead?

> Source/WebCore/html/HTMLFormElement.cpp:272
> -    HTMLFormControlElement* submitElement = submitElementFromEvent(event);
> +    RefPtr<HTMLFormControlElement> submitElement =
submitElementFromEvent(event);

Can we make submitElementFromEvent return RefPtr?

> Source/WebCore/html/HTMLFormElement.cpp:787
> -    HTMLElement* elementFromPast = elementFromPastNamesMap(name);
> +    RefPtr<HTMLElement> elementFromPast = elementFromPastNamesMap(name);

Make elementFromPastNamesMap return a RefPtr?

> Source/WebCore/html/HTMLFrameElement.cpp:67
> -    const HTMLFrameSetElement* containingFrameSet =
HTMLFrameSetElement::findContaining(this);
> +    const RefPtr<HTMLFrameSetElement> containingFrameSet =
HTMLFrameSetElement::findContaining(this);

Why don't we make findContaining return a RefPtr?

> Source/WebCore/html/HTMLFrameSetElement.cpp:179
> -    const HTMLFrameSetElement* containingFrameSet = findContaining(this);
> +    const RefPtr<HTMLFrameSetElement> containingFrameSet =
findContaining(this);

Can we make findContaining return a RefPtr?

> Source/WebCore/html/HTMLImageElement.cpp:622
> -    ShadowRoot* shadowRoot = userAgentShadowRoot();
> +    RefPtr<ShadowRoot> shadowRoot = userAgentShadowRoot();

Make userAgentShadowRoot return a RefPtr?

> Source/WebCore/html/HTMLSelectElement.cpp:771
> +    RefPtr<HTMLOptionElement> foundSelected = 0;
> +    RefPtr<HTMLOptionElement> firstOption = 0;

No need to initialize them to 0. (Use nullptr if any).

> Source/WebCore/html/HTMLSelectElement.cpp:1057
> +    RefPtr<HTMLOptionElement> firstOption = nullptr;
> +    RefPtr<HTMLOptionElement> selectedOption = nullptr;

Get rid of initialization?

> Source/WebCore/html/HTMLTextFormControlElement.cpp:445
> -    TextControlInnerTextElement* innerText = innerTextElement();
> +    RefPtr<TextControlInnerTextElement> innerText = innerTextElement();

Can we make innerTextElement return a RefPtr?

> Source/WebCore/html/HTMLTrackElement.cpp:212
> -    HTMLMediaElement* parent = mediaElement();
> +    RefPtr<HTMLMediaElement> parent = mediaElement();

Can we make mediaElement() return a RefPtr?

> Source/WebCore/html/MediaElementSession.cpp:464
> -    MediaPlayer* player = element.player();
> +    RefPtr<MediaPlayer> player = element.player();

Can we make player return a RefPtr?

> Source/WebCore/html/shadow/MediaControlElements.cpp:736
> -    HTMLMediaElement* mediaElement = parentMediaElement(this);
> +    RefPtr<HTMLMediaElement> mediaElement = parentMediaElement(this);

Make parentMediaElement return RefPtr?

> Source/WebCore/html/shadow/SliderThumbElement.cpp:229
> -    HTMLInputElement* input = hostInput();
> +    RefPtr<HTMLInputElement> input = hostInput();

Make hostInput return a RefPtr?