[webkit-reviews] review granted: [Bug 96198] [V8] setNamedHiddenWindowReference doesn't need to be a special case : [Attachment 162999] Patch

[bugzilla-daemon at webkit.org]

Nate Chapin <japhet at chromium.org> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 96198: [V8] setNamedHiddenWindowReference doesn't need to be a special case
https://bugs.webkit.org/show_bug.cgi?id=96198

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

------- Additional Comments from Nate Chapin <japhet at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=162999&action=review


If there was a reason the difference between DOMWindow handling and everything
else, I've long since forgotten and haven't succeeded in deriving it again. I'm
guessing I cargo-culted it.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:988
>      # the newly created wrapper into an internal field of the holder object.

> -    if (!IsNodeSubType($dataNode) && $attrName ne "self" &&
(IsWrapperType($returnType) && ($attribute->type =~ /^readonly/ ||
$attribute->signature->extendedAttributes->{"Replaceable"})
> +    if (!IsNodeSubType($dataNode) && $attrName ne "self" &&
(IsWrapperType($returnType) && ($attribute->type =~ /^readonly/ ||
$attribute->signature->extendedAttributes->{"Replaceable"} || $attrName eq
"location")
>	   && $returnType ne "EventTarget" && $returnType ne
"SerializedScriptValue" && $returnType ne "DOMWindow" 

This if statement is the most embarrassing code I've written in my career thus
far. I'm sad to add to it, but it's still better than the current state of the
code.