[webkit-reviews] review granted: [Bug 178201] Replace some stack raw pointers with RefPtrs within WebCore/html : [Attachment 323736] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Oct 14 18:01:53 PDT 2017
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?
More information about the webkit-reviews
mailing list