[webkit-reviews] review granted: [Bug 63596] Extract setShadowRoot function, harden it in preparation to be exposed to script. : [Attachment 99026] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Dimitri Glazkov (Google)
<dglazkov at chromium.org>'s request for review:
Bug 63596: Extract setShadowRoot function, harden it in preparation to be
exposed to script.
https://bugs.webkit.org/show_bug.cgi?id=63596

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=99026&action=review

> Source/WebCore/dom/Element.cpp:1214
> +    if (!shadowRoot)
> +	   return true;
> +
> +    if (shadowRoot->shadowHost()) {
> +	   ec = HIERARCHY_REQUEST_ERR;
> +	   return false;
> +    }
> +
> +    if (shadowRoot->document() != document) {
> +	   ec = WRONG_DOCUMENT_ERR;
> +	   return false;
> +    }
> +
> +    return true;

I would add this function in a separate patch once this function is exposed to
JS so you can write tests for these branches.

> Source/WebCore/dom/Element.cpp:1239
> +    ExceptionCode ec = 0;
> +    setShadowRoot(ShadowRoot::create(document()), ec);

We just drop the ec on the floor.  Typically we ASSERT that it is zero.