To reproduce: `run-webkit-tests --iterations=100 inspector/debugger/breakpoints/resolved-dump-all-pause-locations.html` This began being flakey after r291746, which added a test case. It appears the we finally hit the edge of our luck with racing scripts being sent to the frontend, and this became flakey about 5% of the time (varies by platform). https://results.webkit.org/?suite=layout-tests&test=inspector%2Fdebugger%2Fbreakpoints%2Fresolved-dump-all-pause-locations.html
<rdar://problem/91639437>
Created attachment 457349 [details] Patch v1.0
Comment on attachment 457349 [details] Patch v1.0 Oo, neat!
Comment on attachment 457349 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=457349&action=review r=me > Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js:247 > + async awaitPendingDispatches() Rather than have a new method, could we make `runAfterPendingDispatches` just return a `Promise` if a `callback` is not provided (i.e. just like what we do with protocol commands in the frontend)? > LayoutTests/inspector/debugger/breakpoints/resources/dump.js:10 > + window.addDumpAllPauseLocationsTestCase = async function(suite, {name, scriptRegex, resourceRegex}) { Why does this need to be `async`? > LayoutTests/inspector/debugger/breakpoints/resources/dump.js:27 > + await InspectorBackend.awaitPendingDispatches(); Another option to waiting for pending dispatches could be to do something like `WI.networkManager.mainFrame.resourceCollection.addEventListener(WI.Collection.Event.ItemAdded, (event) => { ... })`. I feel like that's probably safer than just waiting for pending dispatches as I'm not sure it's guaranteed that the script message is pending (i.e. it could be coming *right* after the current messages). > LayoutTests/inspector/debugger/breakpoints/resources/dump.js:51 > + await InspectorBackend.awaitPendingDispatches(); possibly a ditto of :27 by waiting for `Debugger.breakpointResolved` instead?
Comment on attachment 457349 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=457349&action=review >> LayoutTests/inspector/debugger/breakpoints/resources/dump.js:10 >> + window.addDumpAllPauseLocationsTestCase = async function(suite, {name, scriptRegex, resourceRegex}) { > > Why does this need to be `async`? Oops. Remnant of solutions past. >> LayoutTests/inspector/debugger/breakpoints/resources/dump.js:27 >> + await InspectorBackend.awaitPendingDispatches(); > > Another option to waiting for pending dispatches could be to do something like `WI.networkManager.mainFrame.resourceCollection.addEventListener(WI.Collection.Event.ItemAdded, (event) => { ... })`. I feel like that's probably safer than just waiting for pending dispatches as I'm not sure it's guaranteed that the script message is pending (i.e. it could be coming *right* after the current messages). Yeah, let me take another look at this... You are probably right.
Comment on attachment 457349 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=457349&action=review Not sure why, but the bug is now reproducing after I pulled this patch back to my machine, so I'm not convinced I've actually/fully fixed this. >>> LayoutTests/inspector/debugger/breakpoints/resources/dump.js:10 >>> + window.addDumpAllPauseLocationsTestCase = async function(suite, {name, scriptRegex, resourceRegex}) { >> >> Why does this need to be `async`? > > Oops. Remnant of solutions past. Oops. Remnant of solutions past. >>> LayoutTests/inspector/debugger/breakpoints/resources/dump.js:27 >>> + await InspectorBackend.awaitPendingDispatches(); >> >> Another option to waiting for pending dispatches could be to do something like `WI.networkManager.mainFrame.resourceCollection.addEventListener(WI.Collection.Event.ItemAdded, (event) => { ... })`. I feel like that's probably safer than just waiting for pending dispatches as I'm not sure it's guaranteed that the script message is pending (i.e. it could be coming *right* after the current messages). > > Yeah, let me take another look at this... You are probably right. Yeah, let me take another look at this... You are probably right.
Pull request: https://github.com/WebKit/WebKit/pull/1066
Committed r294907 (251030@main): <https://commits.webkit.org/251030@main> Reviewed commits have been landed. Closing PR #1066 and removing active labels.