Bug 111411

Created attachment 191434 [details] Patch

Comment on attachment 191434 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191434&action=review > Source/WebCore/bindings/v8/V8HTMLCustomElement.cpp:78 > + v8::Handle<v8::Value> constructorValue = WebCore::toV8(constructor.get(), creationContext, isolate); Nit: WebCore:: is not needed. > Source/WebCore/bindings/v8/V8HTMLCustomElement.cpp:82 > + v8::Handle<v8::Object> prototype = v8::Handle<v8::Object>::Cast(constructorWapper->Get(v8String("prototype", isolate))); Nit: v8String => v8::String::NewSymbol > Source/WebCore/bindings/v8/V8PerContextData.cpp:130 > + if (prototypeObject->InternalFieldCount() == v8PrototypeInternalFieldcount) > + prototypeObject->SetAlignedPointerInInternalField(v8PrototypeTypeIndex, type); It looks a bit fragile. In future there might be other objects whose InternalFieldCount() are 1. Can't you use type->wrapperTypePrototype instead of relying on InternalFieldCount() ?

Created attachment 191450 [details] Patch

Comment on attachment 191434 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191434&action=review Thanks for the quick look, haraken! I revised the patch. >> Source/WebCore/bindings/v8/V8HTMLCustomElement.cpp:78 >> + v8::Handle<v8::Value> constructorValue = WebCore::toV8(constructor.get(), creationContext, isolate); > > Nit: WebCore:: is not needed. compiler wants it since there are conflicting V8HTMLCusstomElement::toV8() >> Source/WebCore/bindings/v8/V8PerContextData.cpp:130 >> + prototypeObject->SetAlignedPointerInInternalField(v8PrototypeTypeIndex, type); > > It looks a bit fragile. In future there might be other objects whose InternalFieldCount() are 1. Can't you use type->wrapperTypePrototype instead of relying on InternalFieldCount() ? Changed in that way.

Comment on attachment 191450 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191450&action=review LGTM. > Source/WebCore/bindings/v8/V8HTMLCustomElement.cpp:62 > + v8::Handle<v8::Value> wrapperValue = toV8(toHTMLUnknownElement(impl.get()), creationContext, isolate); Hmm? You said a compiler wants WebCore::, but did you remove WebCore:: ?

Comment on attachment 191450 [details] Patch Attachment 191450 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17053076 New failing tests: editing/inserting/insert-composition-whitespace.html editing/pasteboard/paste-and-sanitize.html dom/xhtml/level3/core/documentadoptnode16.xhtml dom/xhtml/level1/core/hc_documentcreateelementcasesensitive.xhtml accessibility/canvas-fallback-content.html http/tests/cache/subresource-failover-to-network.html editing/execCommand/remove-format-elements.html editing/execCommand/applyblockelement-visiblepositionforindex-crash.html dom/xhtml/level3/core/nodeisdefaultnamespace09.xhtml dom/html/level1/core/hc_elementwrongdocumenterr.html dom/html/level1/core/hc_attrinsertbefore6.html editing/selection/extend-by-sentence-001.html dom/html/level1/core/hc_attrappendchild5.html editing/deleting/delete-and-cleanup.html dom/html/level1/core/hc_nodeappendchildnewchilddiffdocument.html dom/html/level1/core/hc_namednodemapwrongdocumenterr.html dom/xhtml/level3/core/documentnormalizedocument10.xhtml editing/editability/ignored-content.html dom/html/level1/core/hc_attrappendchild2.html dom/html/level1/core/hc_nodeinsertbeforenewchilddiffdocument.html editing/undo/replace-text-in-node-preserving-markers-crash.html editing/pasteboard/paste-without-nesting.html http/tests/misc/script-defer-after-slow-stylesheet.html editing/style/remove-format-without-enclosing-block.html editing/execCommand/ident-crashes-topnode-is-text.html editing/selection/root-inlinebox-selected-children-crash.html fast/block/child-not-removed-from-parent-lineboxes-crash.html dom/xhtml/level1/core/hc_attrappendchild2.xhtml fast/block/merge-anonymous-block-remove-child-crash2.html dom/html/level1/core/hc_nodereplacechildnewchilddiffdocument.html

Comment on attachment 191450 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191450&action=review >> Source/WebCore/bindings/v8/V8HTMLCustomElement.cpp:62 >> + v8::Handle<v8::Value> wrapperValue = toV8(toHTMLUnknownElement(impl.get()), creationContext, isolate); > > Hmm? You said a compiler wants WebCore::, but did you remove WebCore:: ? Oh, this one is OK. Another one below isn't.

Created attachment 191622 [details] Patch for landing

(In reply to comment #7) > > > > Hmm? You said a compiler wants WebCore::, but did you remove WebCore:: ? > > Oh, this one is OK. Another one below isn't. And turned out we need it after all :-/

Comment on attachment 191622 [details] Patch for landing Clearing flags on attachment: 191622 Committed r144865: <http://trac.webkit.org/changeset/144865>

All reviewed patches have been landed. Closing bug.