Bug 218840 - REGRESSION(r269701): inspector/console/webcore-logging.html is crashing
Summary: REGRESSION(r269701): inspector/console/webcore-logging.html is crashing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: BJ Burg
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-11-11 19:56 PST by BJ Burg
Modified: 2020-11-13 16:21 PST (History)
6 users (show)

See Also:


Attachments
Stack trace of ASSERT (121.12 KB, text/plain)
2020-11-11 19:57 PST, BJ Burg
no flags Details
Speculative fix (3.12 KB, patch)
2020-11-11 20:31 PST, BJ Burg
no flags Details | Formatted Diff | Diff
Patch v1.1 (3.90 KB, patch)
2020-11-12 11:38 PST, BJ Burg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2020-11-11 19:56:40 PST
.
Comment 1 Radar WebKit Bug Importer 2020-11-11 19:56:53 PST
<rdar://problem/71310952>
Comment 2 BJ Burg 2020-11-11 19:57:11 PST
Created attachment 413903 [details]
Stack trace of ASSERT
Comment 3 BJ Burg 2020-11-11 20:19:29 PST
Patch forthcoming.
Comment 4 BJ Burg 2020-11-11 20:22:56 PST
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.
Comment 5 BJ Burg 2020-11-11 20:23:38 PST
(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.
Comment 6 BJ Burg 2020-11-11 20:31:37 PST
Created attachment 413906 [details]
Speculative fix
Comment 7 Devin Rousso 2020-11-11 22:34:25 PST
Comment on attachment 413906 [details]
Speculative fix

rs=me

Should we also guard the `evaluateQueuedExpressions()` call inside `InspectorFrontendAPIDispatcher::unsuspend` behind an `if (m_frontendLoaded)`?
Comment 8 BJ Burg 2020-11-12 11:38:08 PST
Created attachment 413957 [details]
Patch v1.1

Newest patch adds a guard in suspend() per Devin's review comment.
Comment 9 BJ Burg 2020-11-12 11:38:45 PST
(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()
Comment 10 EWS 2020-11-13 16:21:09 PST
Committed r269806: <https://trac.webkit.org/changeset/269806>

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