[webkit-reviews] review denied: [Bug 27785] [V8] Add a way to register V8 extensions for Isolated Worlds only : [Attachment 33678] first attempt

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Matt Perry
<mpcomplete at chromium.org>'s request for review:
Bug 27785: [V8] Add a way to register V8 extensions for Isolated Worlds only
https://bugs.webkit.org/show_bug.cgi?id=27785

Attachment 33678: first attempt
https://bugs.webkit.org/attachment.cgi?id=33678&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
+	 } else if (it->scheme.length() > 0 && (it->scheme !=
m_frame->loader()->activeDocumentLoader()->url().protocol() || it->scheme !=
m_frame->page()->mainFrame()->loader()->activeDocumentLoader()->url().protocol(
))) {

You don't need the "else" here because the "if" clause ends in continue.

+	 v8::Persistent<v8::Context> createNewContext(v8::Handle<v8::Object>
global, bool isolatedContext);

Adding bools to this function doesn't really scale.  Can we pass in the set of
extensions somehow?  You might want a helper function that grabs the proper set
of extensions for normal and for isolated worlds.

Also, there's is no such thing as an "isolated context".  There are isolated
worlds, which eventually will hold more than one context each.