[Webkit-unassigned] [Bug 76035] Add state attribute to history's dom interface.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=76035





--- Comment #30 from Kentaro Hara <haraken at chromium.org>  2012-01-17 02:32:16 PST ---
(From update of attachment 122586)
View in context: https://bugs.webkit.org/attachment.cgi?id=122586&action=review

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2039
> +                                        push(@implContent, "    ScriptExecutionContext* scriptContext = static_cast<JSDOMGlobalObject*>(exec->lexicalGlobalObject())->scriptExecutionContext();\n");

Don't we need to check if the returned scriptContext is valid or not?

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2042
> +                                }

Could you use GenerateAttributeCallWith?

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:908
> +        my $callWithArg;

This declaration can be inside the following if block.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:945
> +        if ($hasScriptState) {

Maybe we can clean up the code by writing '$callWith eq "ScriptState"' here, and remove $hasScriptState.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:1142
> +    my $hasScriptState = 0;

Remove this.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:1175
> +            my $callWithArg;

This declaration can be inside the following if block.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:1179
> +                unshift(@arguments, $callWithArg);

push here, instead of unshift.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:1181
> +

if ($callWith eq "ScriptState") { push(@implContentDecls, "    if (state.hadException())\n"); ...; } is necessary, isn't it?

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:1493
> +        push(@$outputArray, "    EmptyScriptState state;\n");

Nit: Maybe rename to ScriptStateHolder, or just ScriptState. EmptyScriptState might be confusing. (I know other places are using EmptyScriptState though) It's up to you.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:1500
> +            push(@$outputArray, "        return v8::Undefined();\n");

I am not sure but do you really want to skip this return for setters?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.