ActiveDOMObject::hasPendingActivity() should never return true after stop() has been called. ActiveDOMObject::stop() gets called when the script execution context is about to get destroyed. At this point, we should no longer be running script so there is never any need to keep the JS wrapper alive. As a result, ActiveDOMObject::hasPendingActivity() should be returning false after stop() has been called. This will make it harder to cause JS wrapper leaks with ActiveDOMObjects.
Created attachment 395219 [details] Patch
Created attachment 395223 [details] Patch
Is it possible for a script to run in another frame, and to reference something in the stopped one?
Created attachment 395268 [details] Patch
(In reply to Alexey Proskuryakov from comment #3) > Is it possible for a script to run in another frame, and to reference > something in the stopped one? AFAIK, an ActiveDOMObject should really not run script after stop() has been called. I think that if events were still fired after stop() has been called, it would be a bug.
Created attachment 395280 [details] Patch
Created attachment 395283 [details] Patch
Comment on attachment 395283 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=395283&action=review > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4777 > - push(@implContent, " if (js${interfaceName}->wrapped().hasPendingActivity()) {\n"); > + push(@implContent, " if (!js${interfaceName}->wrapped().isContextStopped() && js${interfaceName}->wrapped().hasPendingActivity()) {\n"); Can we add a helper function like hasPendingActivityAndNotStopped so that we don't do de-referencing twice here?
(In reply to Ryosuke Niwa from comment #8) > Comment on attachment 395283 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=395283&action=review > > > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4777 > > - push(@implContent, " if (js${interfaceName}->wrapped().hasPendingActivity()) {\n"); > > + push(@implContent, " if (!js${interfaceName}->wrapped().isContextStopped() && js${interfaceName}->wrapped().hasPendingActivity()) {\n"); > > Can we add a helper function like hasPendingActivityAndNotStopped so that we > don't do de-referencing twice here? I can also add a stack variable in before the if check for js${interfaceName}->wrapped(). I figured it did not matter much because it is generated code.
Created attachment 395299 [details] Patch
Comment on attachment 395299 [details] Patch Clearing flags on attachment: 395299 Committed r259419: <https://trac.webkit.org/changeset/259419>
All reviewed patches have been landed. Closing bug.
(In reply to Chris Dumez from comment #9) > (In reply to Ryosuke Niwa from comment #8) > > Comment on attachment 395283 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=395283&action=review > > > > > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4777 > > > - push(@implContent, " if (js${interfaceName}->wrapped().hasPendingActivity()) {\n"); > > > + push(@implContent, " if (!js${interfaceName}->wrapped().isContextStopped() && js${interfaceName}->wrapped().hasPendingActivity()) {\n"); > > > > Can we add a helper function like hasPendingActivityAndNotStopped so that we > > don't do de-referencing twice here? > > I can also add a stack variable in before the if check for > js${interfaceName}->wrapped(). I figured it did not matter much because it > is generated code. Generated code or not, it would incur runtime cost.
<rdar://problem/61227728>