Extract setShadowRoot function, harden it in preparation to be exposed to script.
Created attachment 99026 [details] Patch
Comment on attachment 99026 [details] Patch 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.
Comment on attachment 99026 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=99026&action=review >> Source/WebCore/dom/Element.cpp:1214 >> + 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. That is a very good point.
Created attachment 99038 [details] Patch for landing
Comment on attachment 99038 [details] Patch for landing whoops, boneheaded mistake.
Comment on attachment 99038 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=99038&action=review > Source/WebCore/dom/Element.cpp:1220 > + ASSERT(ec); ASSERT(!ec); Do I win the prize?
(In reply to comment #6) > (From update of attachment 99038 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=99038&action=review > > > Source/WebCore/dom/Element.cpp:1220 > > + ASSERT(ec); > > ASSERT(!ec); > > Do I win the prize? TOTALLY. I was half-asleep. Also, I think I'll bring back the hardening function and file a bug to write a test for it. I am trying to avoid the problem where enabling APIs is a huge atomic commit.
Created attachment 99109 [details] Patch for landing
Comment on attachment 99109 [details] Patch for landing Clearing flags on attachment: 99109 Committed r90026: <http://trac.webkit.org/changeset/90026>
All reviewed patches have been landed. Closing bug.