"Worker not found" uncaught protocol errors Steps to Reproduce: 1. Inspect maps.google.com 2. Reload a few times => Errors, Worker was created+terminated in the backend before frontend could send messages, okay to not find the worker. Steps to Reproduce: 1. Inspect maps.google.com 2. Cross origin navigate to a page without workers => Potential Errors, leftover worker targets in WI.targets
Created attachment 361248 [details] [PATCH] Proposed Fix
Comment on attachment 361248 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=361248&action=review r=me > Source/WebInspectorUI/ChangeLog:12 > + frame, but for now this is the only a page transition can kill Grammar: "the only a page" -> "the only way a page" > Source/WebInspectorUI/UserInterface/Base/Main.js:232 > + let workerTargets = WI.targets.filter((x) => x.type === WI.Target.Type.Worker); Nit: rename x to target.
(In reply to Matt Baker from comment #2) > Comment on attachment 361248 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=361248&action=review > > r=me > > > Source/WebInspectorUI/ChangeLog:12 > > + frame, but for now this is the only a page transition can kill > > Grammar: "the only a page" -> "the only way a page" Doh! Thanks > > Source/WebInspectorUI/UserInterface/Base/Main.js:232 > > + let workerTargets = WI.targets.filter((x) => x.type === WI.Target.Type.Worker); > > Nit: rename x to target. There is already a target I don't want to shadow, and I think it is clear from context.
Comment on attachment 361248 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=361248&action=review >> Source/WebInspectorUI/ChangeLog:12 >> + frame, but for now this is the only a page transition can kill > > Grammar: "the only a page" -> "the only way a page" err, I think he means "but for now only a page transition can kill all worker targets." >> Source/WebInspectorUI/UserInterface/Base/Main.js:232 >> + let workerTargets = WI.targets.filter((x) => x.type === WI.Target.Type.Worker); > > Nit: rename x to target. We don't want to use `target` since that already exists in this function. I think `item` is a better/clearer name. > Source/WebInspectorUI/UserInterface/Controllers/WorkerManager.js:58 > + WorkerAgent.initialized(workerId).catch(function(){}); Rather than just silently handling failure, maybe we can do an engineering-build-only log? Like a `console.assert(false, ...);`? > Source/WebInspectorUI/UserInterface/Protocol/Connection.js:311 > + // Ignore errors if a worker went away quickly. We should also have this comment in WorkerManager.js. > Source/WebInspectorUI/UserInterface/Protocol/Connection.js:312 > + WorkerAgent.sendMessageToWorker(this._workerId, message).catch(function(){}); Ditto (>WorkerManager.js:58).
> > Source/WebInspectorUI/UserInterface/Controllers/WorkerManager.js:58 > > + WorkerAgent.initialized(workerId).catch(function(){}); > > Rather than just silently handling failure, maybe we can do an > engineering-build-only log? Like a `console.assert(false, ...);`? I don't want to assert(false) for something we know can and will happen. Jus loading maps.google.com for example.
Created attachment 361308 [details] [PATCH] For Landing
Comment on attachment 361308 [details] [PATCH] For Landing Clearing flags on attachment: 361308 Committed r241039: <https://trac.webkit.org/changeset/241039>
<rdar://problem/47860997>