[Webkit-unassigned] [Bug 26332] Upstream V8Helpers

[bugzilla-daemon at webkit.org]

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


levin at chromium.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #31173|review?                     |review-
               Flag|                            |




------- Comment #2 from levin at chromium.org  2009-06-11 15:26 PDT -------
(From update of attachment 31173)
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?)


> +using WebCore::V8Custom;
Do we have a V8Custom namespace?

> +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.

> +
> +// 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?


-- 
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.