Bug 202596 – Make sure ActiveDOMObject properly deals with detached documents

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

Make sure ActiveDOMObject properly deals with detached documents.

[Chris Dumez]

Created attachment 380242 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/56036491>

[Chris Dumez]

Comment on attachment 380242 [details]
Patch

ping review?

[Geoffrey Garen]

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? (

[Chris Dumez]

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.

[WebKit Commit Bot]

Comment on attachment 380242 [details]
Patch

Clearing flags on attachment: 380242

Committed r250843: <https://trac.webkit.org/changeset/250843>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.