[webkit-reviews] review denied: [Bug 51914] ElementRareData::m_shadowRoot should not be RefPtr. : [Attachment 79147] Patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied MORITA Hajime
<morrita at google.com>'s request for review:
Bug 51914: ElementRareData::m_shadowRoot should not be RefPtr.
https://bugs.webkit.org/show_bug.cgi?id=51914

Attachment 79147: Patch
https://bugs.webkit.org/attachment.cgi?id=79147&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=79147&action=review

> Source/WebCore/dom/Element.cpp:1090
>  void Element::setShadowRoot(PassRefPtr<Node> node)

We're not passing RefPtr anymore, since it'll be stored as a raw ptr. Should we
still have PassRefPtr as an argument?

> Source/WebCore/dom/Element.cpp:1110
> +    ElementRareData* data = rareData();
> +    if (data->m_shadowRoot) {

Could be written in one line as "if (ElementRareData* data = rareData()) {"

> Source/WebCore/dom/Element.cpp:1111
> +	   RefPtr<Node> oldRoot = adoptRef(data->m_shadowRoot);

adoptRef, are you sure? You actually  do want to increment the ref count here.