Summary: | Make sure ActiveDOMObject properly deals with detached documents | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||
Component: | WebCore Misc. | Assignee: | Chris Dumez <cdumez> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | achristensen, commit-queue, dbates, esprehn+autocc, ews-watchlist, ggaren, jer.noble, kangil.han, webkit-bug-importer, youennf | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 202293 | ||||||
Attachments: |
|
Description
Chris Dumez
2019-10-04 12:59:03 PDT
Created attachment 380242 [details]
Patch
Comment on attachment 380242 [details]
Patch
ping review?
Comment on attachment 380242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380242&action=review r=me > Source/WebCore/dom/ActiveDOMObject.cpp:50 > +inline ActiveDOMObject::ActiveDOMObject(ScriptExecutionContext* context, CheckedScriptExecutionContextType) > + : ContextDestructionObserver(context) > +{ > + ASSERT(!is<Document>(context) || &downcast<Document>(context)->contextDocument() == downcast<Document>(context)); > + if (!context) > return; > > - ASSERT(m_scriptExecutionContext->isContextThread()); > - m_scriptExecutionContext->didCreateActiveDOMObject(*this); > + ASSERT(context->isContextThread()); > + context->didCreateActiveDOMObject(*this); > +} Isn't this constructor still unsafe in the way you described? ( Comment on attachment 380242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380242&action=review >> Source/WebCore/dom/ActiveDOMObject.cpp:50 >> +} > > Isn't this constructor still unsafe in the way you described? ( Thus the CheckedScriptExecutionContextType parameter. This is a private constructor which other protected constructors call *after* they've checked the script execution context. This is purely to avoid code duplication between constructors. Comment on attachment 380242 [details] Patch Clearing flags on attachment: 380242 Committed r250843: <https://trac.webkit.org/changeset/250843> All reviewed patches have been landed. Closing bug. |