Summary: | Add custom V8 bindings for DOMWindow | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mike Belshe <mbelshe> | ||||||||
Component: | WebCore JavaScript | Assignee: | Mike Belshe <mbelshe> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | eric, sam | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
Mike Belshe
2009-03-16 07:17:44 PDT
Created attachment 28941 [details]
patch
Sam will likely be curious about this change too. 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 Comment on attachment 28941 [details]
patch
Whoops, I meant r-.
Created attachment 28972 [details]
patch w/ Dimitri's comments
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() Created attachment 28983 [details]
use notImplementedByInterceptor()
Landed as http://trac.webkit.org/changeset/42071. Whoops, I just realized this file is in the wrong place. It should be in bindings/v8/custom. Fix landed in http://trac.webkit.org/changeset/42072. |