[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


Charles Reis <creis at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |creis at chromium.org




--- Comment #7 from Charles Reis <creis at chromium.org>  2009-12-09 13:43:28 PST ---
(In reply to comment #5)
> (From update of attachment 44544 [details])
> 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?
> 

(See below)

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

Well, reportUnsafeAccessTo (and the enum it relies on) looks like it's only
definied within V8Proxy.cpp, but I could move it to be a static member of
V8Proxy that we can call outside V8Proxy.cpp.

I could also move the immediatelyReportUnsafeAccessTo method to the
BindingsState class, since that already has bindings-specific code.  That'll
clean up BindingsSecurity so that it's all shared code.


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

 I think the template approach is probably the most flexible (while still
giving us compile-time dispatch).  I'll look into it.

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