Bug 218840 – REGRESSION(r269701): inspector/console/webcore-logging.html is crashing

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Blaze Burg]

.

[Radar WebKit Bug Importer]

<rdar://problem/71310952>

[Blaze Burg]

Created attachment 413903 [details]
Stack trace of ASSERT

[Blaze Burg]

Patch forthcoming.

[Blaze Burg]

Analysis:

Test.html finishes loading. Our load event listener calls InspectorFrontendHost::loaded() which makes the  inspector focus, which makes the webpage window unfocus and resign first responder. This causes style recalc, and we have sync instrumentation underneath it (this should really be more async btw). The instrumentation generates an event to be dispatched to the frontend. The dispatcher hasn't been told that the window is loaded yet under InspectorFrontendHost::frontendLoaded. It's not safe to eval scripts due to the sync style recalc upthread, so the dispatcher suspends and tries again on another run loop turn.

[Blaze Burg]

(In reply to Brian Burg from comment #4)
> Analysis:
> 
> Test.html finishes loading. Our load event listener calls
> InspectorFrontendHost::loaded() which makes the  inspector focus, which
> makes the webpage window unfocus and resign first responder. This causes
> style recalc, and we have sync instrumentation underneath it (this should
> really be more async btw). The instrumentation generates an event to be
> dispatched to the frontend. The dispatcher hasn't been told that the window
> is loaded yet under InspectorFrontendHost::frontendLoaded. It's not safe to
> eval scripts due to the sync style recalc upthread, so the dispatcher
> suspends and tries again on another run loop turn.

And the crash is due to an ASSERT(m_frontendLoaded) failing.

[Blaze Burg]

Created attachment 413906 [details]
Speculative fix

[Devin Rousso]

Comment on attachment 413906 [details]
Speculative fix

rs=me

Should we also guard the `evaluateQueuedExpressions()` call inside `InspectorFrontendAPIDispatcher::unsuspend` behind an `if (m_frontendLoaded)`?

[Blaze Burg]

Created attachment 413957 [details]
Patch v1.1

Newest patch adds a guard in suspend() per Devin's review comment.

[Blaze Burg]

(In reply to Brian Burg from comment #8)
> Created attachment 413957 [details]
> Patch v1.1
> 
> Newest patch adds a guard in suspend() per Devin's review comment.

Typo: unsuspend(), not suspend()

[EWS]

Committed r269806: <https://trac.webkit.org/changeset/269806>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 413957 [details].