[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





--- Comment #11 from Charles Reis <creis at chromium.org>  2009-12-11 13:27:47 PST ---
(In reply to comment #10)
> (From update of attachment 44696 [details])
> This looks like a great first step.  I've noted a few formatting issues below. 
> There are a couple of things that I'm not sure how they'll scale up when we
> convert more of the bindings, but they're fine for now.  We can revisit them if
> the code isn't growing the right direction.
> 
> + template <class Binding> class BindingSecurity {
> 
> I think we'd prefer
> 
> + template <class Binding>
> + class BindingSecurity {
> 
> (meaning a line break)
> 

Sure, I'll upload a patch in just a minute with linebreaks on all the
templates.

> + class BindingSecurityHelper
> 
> I'm not entirely sold on this class.  We might need to see more examples to see
> what the best general pattern is for this kind of thing.
> 

I wasn't thrilled about this, but it seemed the best approach.  That is, I
don't think files like DOMWindow.h are supposed to be included in header files
like BindingSecurity.h.  For example, it was causing stack corruption problems
for me in V8 code.

On the flip side, templates won't link unless you forward declare all your
instantiations in the cpp file (which is undesirable), or unless you include
the method implementations in the header file (which appears to be common in
WebKit).

This helper class at least solves both problems-- it lets us call methods on
the WebCore types while keeping them opaque in BindingSecurity.h.


> + typedef BindingSecurity<V8Binding> V8BindingSecurity;
> 
> I'm also not sure how this will scale.  It's fine here though.
> 

I was just using it to make the call sites a little shorter-- I'm not too
attached to it in the long run.

> + template <> class State<V8Binding> : public State<GenericBinding> {
> 
> We'd also like a line break here.

Sure.

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