[Webkit-unassigned] [Bug 26332] Upstream V8Helpers

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jun 11 16:34:03 PDT 2009


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





------- Comment #4 from japhet at google.com  2009-06-11 16:34 PDT -------
(In reply to comment #2)
> (From update of attachment 31173 [review])
> Just a few issues to clear up so r- for now.
> 
> > Index: WebCore/bindings/v8/V8Helpers.cpp
> 
> > +#define max max
> > +#define min min
> What is this about? (Why are they needed?)
No idea, don't think it's necessary.  Removed.
> 
> 
> > +using WebCore::V8Custom;
> Do we have a V8Custom namespace?
V8Custom lives in V8CustomBinding.{h,cpp}.  However, this appears to be
unnecessary, so I removed it.
> 
> > +void WrapNPObject(v8::Handle<v8::Object> object, NPObject* npObject)
> WrapNPObject casing.  Should be wrapNPObject
> 
> 
> > +v8::Local<v8::Context> getV8Context(NPP npp, NPObject* npObject)
> toV8Context would be more in keeping with WebKit naming (dglazkov helped me out
> here).
> 
> 
> > +WebCore::V8Proxy* GetV8Proxy(NPObject* npObject)
> s/GetV8Proxy/toV8Proxy/
> 
> 
> > Index: WebCore/bindings/v8/V8Helpers.h
> 
> > +// Retrieves the V8 Context from the NP context pr obj (at most 1 may be NULL).
> I can't understand this comment at all.
Ditto.
> 
> > +
> > +// Get V8Proxy object from an NPObject.
> This comment doesn't seem to add anything at all (except a line of text that
> may get out of sync with the code).
> 
> 
> > +#endif  // V8Helpers_h
> One space before trailing comments.
> 
> 
> Why aren't the functions declared in the WebCore namespace like the rest of the
> functions in these headers?
> 
Everything else fixed.


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



More information about the webkit-unassigned mailing list