Bug 24616 – Add custom V8 bindings for DOMWindow

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Mike Belshe]

Custom bindings for the DOMWindow under v8.

[Mike Belshe]

Created attachment 28941 [details]
patch

[Eric Seidel (no email)]

Sam will likely be curious about this change too.

[Dimitri Glazkov (Google)]

Comment on attachment 28941 [details]
patch

These are all style-nits, obviously the code works and is good otherwise.

Bug URL in ChangeLog.

> +    WindowSetLocation(imp, ToWebCoreString(value));

toWebCoreString

> +        String eventType = ToWebCoreString(args[0]);

toWebCoreString


> +        String eventType = ToWebCoreString(args[0]);

toWebCoreString


> +    String message = ToWebCoreString(args[0]);

toWebCoreString

> +    WindowFeatures wargs;

wargs -> windowFeatures? No strong feelings here, just wargs sounds a bit overcondensed.

> +            (!parseURL(urlString).startsWith("javascript:", false) ||
> +             ScriptController::isSafeScript(frame))) {

|| goes on new line or you could just remove line break

> +        return v8::Handle<v8::Value>();

notHandledByInterceptor() here and elsewhere in this method.

> +    String propName = ToWebCoreString(name);

toWebCoreString

> +    static HashMap<String, String> kLazyInitMap;

kLazyInitMap -> lazyInitMap

> +        if (context.IsEmpty()) return v8::Handle<v8::Value>();

return on new line.

> +    if (!parseURL(v).startsWith("javascript:", false) ||

|| on new line or remove line break

> +    int handle = ToInt32(args[0]);

toInt32

[Dimitri Glazkov (Google)]

Comment on attachment 28941 [details]
patch

Whoops, I meant r-.

[Mike Belshe]

Created attachment 28972 [details]
patch w/ Dimitri's comments

[Dimitri Glazkov (Google)]

Comment on attachment 28972 [details]
patch w/ Dimitri's comments

Good enough. The following are nice, but not required.

> +    return v8::Handle<v8::Value>();

notHandledByInterceptor()

> +    if (!name->IsString())
> +        return v8::Handle<v8::Value>();

notHandledByInterceptor()

> +    if (holder.IsEmpty())
> +        return v8::Handle<v8::Value>();

notHandledByInterceptor()


> +    if (!window)
> +        return v8::Handle<v8::Value>();

notHandledByInterceptor()


> +    Frame* frame = window->frame();
> +    // window is detached from a frame.
> +    if (!frame)
> +        return v8::Handle<v8::Value>();

notHandledByInterceptor()


> +
> +    return v8::Handle<v8::Value>();

notHandledByInterceptor()

[Mike Belshe]

Created attachment 28983 [details]
use notImplementedByInterceptor()

[Dimitri Glazkov (Google)]

Landed as http://trac.webkit.org/changeset/42071.

[Dimitri Glazkov (Google)]

Whoops, I just realized this file is in the wrong place. It should be in bindings/v8/custom.

[Dimitri Glazkov (Google)]

Fix landed in http://trac.webkit.org/changeset/42072.