V8Proxy::retrieve(*) leads to really obfuscated code and should be removed
Created attachment 156273 [details] Patch
Comment on attachment 156273 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156273&action=review > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3324 > + frame->script()->windowShell()->initContextIfNeeded(); I guess if we're running script in the first place, then script() is never null... > Source/WebCore/bindings/scripts/test/V8/V8TestNode.cpp:115 > + V8Proxy* proxy = impl->document()->frame() ? impl->document()->frame()->script()->proxy() : 0; This seems like a really long idiom... Maybe impl should have an accessor which can return script() for you?
(In reply to comment #2) > (From update of attachment 156273 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=156273&action=review > > > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3324 > > + frame->script()->windowShell()->initContextIfNeeded(); > > I guess if we're running script in the first place, then script() is never null... script() is no longer ever null. It's the object you ask if script is disabled. > > Source/WebCore/bindings/scripts/test/V8/V8TestNode.cpp:115 > > + V8Proxy* proxy = impl->document()->frame() ? impl->document()->frame()->script()->proxy() : 0; > > This seems like a really long idiom... Maybe impl should have an accessor which can return script() for you? Yeah, this is generated code, so I'm not that worried about it. I'd like to get rid of V8Proxy entirely, we will make it shorter.
Created attachment 156275 [details] Patch
Comment on attachment 156275 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156275&action=review OK. > Source/WebCore/bindings/v8/V8LazyEventListener.cpp:91 > + return v8::Local<v8::Value>(); Super ugly.
> > Source/WebCore/bindings/v8/V8LazyEventListener.cpp:91 > > + return v8::Local<v8::Value>(); > > Super ugly. We can make it v8::Undefined() if you like that better.
(In reply to comment #6) > > > Source/WebCore/bindings/v8/V8LazyEventListener.cpp:91 > > > + return v8::Local<v8::Value>(); > > > > Super ugly. > > We can make it v8::Undefined() if you like that better. Now the V8 team made v8::Handle<v8::Value>() as fast as v8::Undefined(Isolate). So we can use v8::Handle<v8::Value>() everywhere regardless of Isolate. I'm planning to implement v8::Handle<v8::Value> v8Undefined() { return v8::Handle<v8::Value>(); } in V8Binding.cpp and use v8Undefined() everywhere in V8 bindings.
Comment on attachment 156275 [details] Patch Ok. Sounds like we should land this with the ugly syntax and clean it up with the rest of them.
Comment on attachment 156275 [details] Patch Clearing flags on attachment: 156275 Committed r124583: <http://trac.webkit.org/changeset/124583>
All reviewed patches have been landed. Closing bug.