[webkit-reviews] review denied: [Bug 30145] There should be a way to reuse isolated worlds : [Attachment 40754] patchity patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 6 18:49:03 PDT 2009


Adam Barth <abarth at webkit.org> has denied Aaron Boodman <aa at chromium.org>'s
request for review:
Bug 30145: There should be a way to reuse isolated worlds
https://bugs.webkit.org/show_bug.cgi?id=30145

Attachment 40754: patchity patch
https://bugs.webkit.org/attachment.cgi?id=40754&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
+ To destroy it, call deleteSoon().

This method doesn't seem to exist.  Also, delete() isn't the right name. 
detach() is probably better.

+ V8Proxy::~V8Proxy()

I wish we had a small scoped object to do this work instead of doing it
explicitly in the destructor.

You seem to only clear the worldID map during destruction of V8Proxy, but
doesn't V8Proxy have Frame-lifetime?  You need to clean up the map on every
navigation.  You probably want to do that work during |clearForNavigation|.

This design also doesn't account for the PageCache, but we don't support
PageCache anyway...

Tests???  Feel free to modify layoutTestController to make this stuff testable.
 You can see examples in LayoutTests/http/tests/security/isolatedWorld


More information about the webkit-reviews mailing list