Don't leak Documents when using MutationObserver from extensions
Created attachment 191072 [details] Patch
Comment on attachment 191072 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191072&action=review > Source/WebCore/bindings/v8/V8Binding.cpp:273 > + if (!world) Seems like there ought to at least be a FIXME here, since it's not at all obvious why this is a signal for the main world...
Created attachment 191079 [details] Patch
(In reply to comment #2) > (From update of attachment 191072 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=191072&action=review > > > Source/WebCore/bindings/v8/V8Binding.cpp:273 > > + if (!world) > > Seems like there ought to at least be a FIXME here, since it's not at all obvious why this is a signal for the main world... Done.
Comment on attachment 191079 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191079&action=review We should eventually replace all uses of WorldContextHandle with this mechanism. > Source/WebCore/bindings/v8/V8Binding.cpp:278 > + return v8::Local<v8::Context>::New(frame->script()->windowShell(world)->context()); It it possible that windowShell(world) will return null? I guess not if the context is active in the frame. > Source/WebCore/bindings/v8/V8Binding.cpp:282 > + if (WorkerScriptController* script = static_cast<WorkerContext*>(context)->script()) Is it possible for script() to return null here? That seems like it would be an odd situation...
Comment on attachment 191079 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191079&action=review >> Source/WebCore/bindings/v8/V8Binding.cpp:278 >> + return v8::Local<v8::Context>::New(frame->script()->windowShell(world)->context()); > > It it possible that windowShell(world) will return null? I guess not if the context is active in the frame. It's not possible if you look at the code in ScriptController::windowShell which crashes if the shell would have been null since it does !shell->isContextInitialized() && shell->initializeIfNeeded() in an if statement below the find(). There should probably be an ASSERT in there to make this more obvious. >> Source/WebCore/bindings/v8/V8Binding.cpp:282 >> + if (WorkerScriptController* script = static_cast<WorkerContext*>(context)->script()) > > Is it possible for script() to return null here? That seems like it would be an odd situation... The other toV8Context and WorldContextHandle assume this is impossible.
Comment on attachment 191079 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191079&action=review >>> Source/WebCore/bindings/v8/V8Binding.cpp:282 >>> + if (WorkerScriptController* script = static_cast<WorkerContext*>(context)->script()) >> >> Is it possible for script() to return null here? That seems like it would be an odd situation... > > The other toV8Context and WorldContextHandle assume this is impossible. You probably mean "is possible". :)
Comment on attachment 191079 [details] Patch Clearing flags on attachment: 191079 Committed r144522: <http://trac.webkit.org/changeset/144522>
All reviewed patches have been landed. Closing bug.