[webkit-reviews] review granted: [Bug 30648] [V8] Change event listeners to not hold context. : [Attachment 41620] Change event listeners to not hold context, v1.

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Dimitri Glazkov (Google)
<dglazkov at chromium.org>'s request for review:
Bug 30648: [V8] Change event listeners to not hold context.
https://bugs.webkit.org/show_bug.cgi?id=30648

Attachment 41620: Change event listeners to not hold context, v1.
https://bugs.webkit.org/attachment.cgi?id=41620&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
In v8::Local<v8::Context> V8Proxy::context() we seem to have lost this code:

if (frame != V8Proxy::retrieveFrame(context->get()))

Can you add it back before landing this?  I know there isn't a test that
explains this code.  The behavior is "wrong" but missing the code is worse.

Also, this patch as a subtle bug with isolated worlds and event handlers and I
must be tested properly.  I think because we're testing it with JS events, not
native events.	I think we should land this patch anyway and deal with that
issue separately (if it isn't a figment of my imagination).

Other than that *this patch is awesome*.  In my ideal world, the bindings would
have little or no knowledge of Frame.