[Webkit-unassigned] [Bug 26366] Teach V8 bindings about isolated worlds

[bugzilla-daemon at webkit.org]

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





------- Comment #4 from abarth at webkit.org  2009-06-15 16:44 PDT -------
(In reply to comment #3)
> Holy moly! You rewrote the world!

Yeah, sorry about that.  It's possible that we could have hacked this in with a
smaller change, but it seemed better to separate the concerns more clearly
(i.e., a thread as a bundle of isolated wrapper maps).  I'm hoping to spend
some more time beautifying this code once we've got Aaron's use cases down.

> * v8_isolated_world should live upstream, in bindings/v8.

Yes.  I'll move this in a follow-up patch.

> * How does this affect bindings performance?

As we discussed on gchat, I'll run some benchmarks on Thursday.

> * Make sure there are no layout test regressions.

I ran the Chromium test_shell_tests in Debug mode and they passed.  If I've
screwed things up, the tree will go red and I'll revert.  :)

> No ChangeLog? :)

I'll add a ChangeLog before landing.  I don't have WebKitTools up and running
in my Chromium tree yet, but I'll have them when I land through my webkit.org
tree.

> > +    private:
> > +      DOMData* m_domData;
> 
> 4 spaces?

Fixed.

> > +    static void handleWeakObject(DOMDataStore::DOMWrapperMapType mapType,
> > +                                 v8::Handle<v8::Object> v8Object,
> > +                                 T* domObject);
> 
> No need to wrap here -- it's WebKit-land :)

Fixed.

> > +DOMDataStoreHandle::DOMDataStoreHandle()
> > +    : m_store(new ScopedDOMDataStore(DOMData::getCurrent()))  // TODO: Fix!
> 
> FIXME: and What?

Oops.  This was a not to myself to test this code.  It's working properly, so
I've removed the comment.

> > +void visitDOMNodesInCurrentThread(DOMWrapperMap<Node>::Visitor* visitor) {
> 
> Brace on new line.

Fixed x5.

Thanks!


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