[Webkit-unassigned] [Bug 32326] Refactor some security code out of V8 bindings

[bugzilla-daemon at webkit.org]

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


Adam Barth <abarth at webkit.org> changed:

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




--- Comment #5 from Adam Barth <abarth at webkit.org>  2009-12-09 10:40:34 PST ---
(From update of attachment 44544)
Thanks for fixing the style nits.  At a high level, this looks great. I have a
few questions:

+ virtual DOMWindow* getActiveWindow() = 0;

Does this need to be virtual, or can we do link-time dispatch here?

+ // Implement BindingsSecurity methods that depend on V8-specific code.

Don't we want to implement this in a V8BindingsSecurity.cpp file instead of
V8Proxy?  V8Proxy is an all-sing, all-dance dumping ground.

+ class V8State : public BindingsState {

I'd call this V8BindingsState to mirror the DOMWindow / V8DOMWindow name
convention.

In general, it's slightly unclear to me whether we actually want a
V8BindingsState object that derives from BindingsState or whether we want to
have a single class where some of the methods are implemented by the different
bindings, like ScriptController.  The advantage of the latter is that we'll get
compile-time dispatch instead of runtime dispatch.

Another option is to have a shared BindingsStateBase and V8 implement a
subclass BindingsState that the shared code refers to.  That way, the bindings
can override whatever methods they need without incurring a virtual method
call.

A final option is to use template namespaces.  These shared classes would be

template<classname Bindings>
class ...

and then refer to Bindings::State.  Then we can have V8Binding::State derive
from GenericBinding::State.  That's more code, but it might make it easier to
support things like re-using this code for ObjCBindings::State, etc.

Thoughts?

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