The gist of this change is to add ability to carry world's context (if necessary) in the event listener. IsolatedWorldContext is an abstraction that does it in an intelligent way. Everything else is just gravy to make it work. IsolatedWorldContext is like String in that it's an easy-copy, holds-on-to-a-ref-counted-load type of guy. The load in this case is the SharedPersistent<v8::Context>, which we grab upon instantiation -- if it's present. If it's not present, IsolatedWorldContext carries nothing. At the time of firing an event, IsolatedWorldContext provides the right context.
Created attachment 44629 [details] WIP: Fix isolated worlds event story
Did you mean to mark this for review? It has "WIP" in the name.
+ // FIXME: Move to its own file. You resolved this FIXME. This looks good. I imagine we'll need to change things slightly when we integrate with the JSC isolated worlds implementation, but we don't want to block this patch on that effort.
Comment on attachment 44629 [details] WIP: Fix isolated worlds event story Whoops sorry. I put this one up to generate test results on my Mac.
Created attachment 44634 [details] Fix isolated worlds event story
Attachment 44634 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/bindings/v8/WorldContextHandle.cpp:57: Could not find a newline character at the end of the file. [whitespace/ending_newline] [5] Total errors found: 1
Comment on attachment 44634 [details] Fix isolated worlds event story 47 WorldContextHandle(WorldToUse worldToUse); 48 v8::Local<v8::Context> adjustedContext(V8Proxy* proxy) const; Seems like we can omit the parameter names here. 44 if (V8IsolatedWorld* world = V8IsolatedWorld::getEntered()) 45 m_context = world->sharedContext(); Why do we capture the context this way? The more natural thing seems like proxy->context()... I guess the problem is we don't know what frame we're in? The problem case is where the world->sharedContext() is for a different frame then the one with the event listener. I'm not sure how to get that to happen at the moment. 52 if (!m_context || m_context->get().IsEmpty()) 53 return proxy->context(); When do we ever get into this case? It seems if we're in the UseCurrentWorld case, then m_context shouldn't be empty... I guess I don't understand why we keep WorldToUse m_worldToUse around at all. It seems like there are two case: 1) We want the current world, so we captured the context on construction. 2) We want the main world, so we figure that out when asked. What's the third case where we failed to capture but we still want the current context?
Comment on attachment 44634 [details] Fix isolated worlds event story Dimitri explained this to me over chat: me: there are really three contexts 1) the main world 2) the current world when we contruct 3) the current world when we retrieve Dimitri: yah! you said if betterer than I could. me: i see, we only care about (3) as an optimization to avoid holding handles === LGTM once you fix the stylebot nit and the param names. Thanks so much for working on this.
Landed as http://trac.webkit.org/changeset/51960.