When upgrading or constructing an element synchronously inside the HTML parser and document.createElement, we must report the exception instead of rethrowing the exception.
Created attachment 290782 [details] Fixes the bug
<rdar://problem/28647657>
Comment on attachment 290782 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=290782&action=review > Source/WebCore/bindings/js/JSCustomElementInterface.cpp:72 > + return element.get(); The get() here means we get reference count churn. Can we compile a version that transfers ownership instead? Maybe WTFMove(element) works? If not, then we should fix the Ref class so it does. > Source/WebCore/bindings/js/JSCustomElementInterface.cpp:91 > + RefPtr<Element> element = constructCustomElementSynchronously(document, vm, state, m_constructor.get(), localName); Can we use auto here instead of RefPtr? > Source/WebCore/html/parser/HTMLDocumentParser.cpp:200 > + Ref<Element> newElement = elementInterface.constructElementWithFallback(*document(), constructionData->name); Can we use auto here?
Thanks for the review. (In reply to comment #3) > Comment on attachment 290782 [details] > Fixes the bug > > View in context: > https://bugs.webkit.org/attachment.cgi?id=290782&action=review > > > Source/WebCore/bindings/js/JSCustomElementInterface.cpp:72 > > + return element.get(); > > The get() here means we get reference count churn. Can we compile a version > that transfers ownership instead? Maybe WTFMove(element) works? If not, then > we should fix the Ref class so it does. WTFMove(element) fails because Ref<HTMLUnknownElement>&& can be adopted by Ref<Element>&& :( I agree we should fix that, not in this patch though. > > > Source/WebCore/bindings/js/JSCustomElementInterface.cpp:91 > > + RefPtr<Element> element = constructCustomElementSynchronously(document, vm, state, m_constructor.get(), localName); > > Can we use auto here instead of RefPtr? Sure. > > Source/WebCore/html/parser/HTMLDocumentParser.cpp:200 > > + Ref<Element> newElement = elementInterface.constructElementWithFallback(*document(), constructionData->name); > > Can we use auto here? Sure.
Created attachment 290872 [details] Patch for landing
Comment on attachment 290872 [details] Patch for landing Clearing flags on attachment: 290872 Committed r206890: <http://trac.webkit.org/changeset/206890>
All reviewed patches have been landed. Closing bug.