Summary: URL Breakpoints that resolve in multiple workers should only appear in the UI once Steps to Reproduce: 1. Inspect <https://nerget.com/rayjs-mt/rayjs.html> 2. With 4 Renderers click "Go!" to spawn workers render 3. Set a breakpoint at render-task.js:14 4. Reload 5. Click "Go!" => Breakpoint resolves in each new worker and shows up 4 times in the UI Notes: - We should refactor and harden Breakpoints by URL more generally. - Clicking the gutter to set a breakpoint almost always (95%+) sets a breakpoint by URL. This is necessary to restore the breakpoint when reloading / navigating the page and hitting the breakpoint. So URL is almost always the right solution. - Any Breakpoint by URL may resolve to MULTIPLE locations - A page with multiple workers loading the same script (worker.js) will resolve the breakpoint to the same location but each tied to the different Worker target. - A page that loads the same URL (script.php) with different comments may resolve the breakpoint to different locations in each. This is rare and admittedly poor use of URLs and so rare that I don't think we should concern ourselves with this. I think the relationship between objects is: DebuggerManager - N Breakpoints Breakpoint (by URL) - 1 Set Location ("URL:line:column") - N Resolved Locations (sourceCodeLocation) - target1:script:line:column - target2:script:line:column - target3:script:line:column Breakpoint (by ScriptIdentifier) - 1 Set Location (target:scriptId:line:column) - At most 1 Resolved Location (sourceCodeLocation) This impacts the current "Breakpoint.resolved" state. It seems "resolved" should move to a computed property based on whether or not there are _any resolved locations_. Likewise the list of _unique locations_ would be better to show in the UI, instead of all the resolved locations on which there may be multiple script:line:column duplicates just in different targets. We could provide a way to enable/disable a breakpoint per-target, but that seems like such a rare case that we shouldn't provide that option.
<rdar://problem/29073523>
Hmm, so I have a simple fix to address this for now. I do want to cleanup the Model objects a bit though. I'm thinking something like: Breakpoint - identifier : string - userLocation : SourceCodeLocation - resolvedLocations : [SourceCodeLocation] One fortunate thing is that URL breakpoints get the same breakpoint identifier across all targets (it is url:line:column and not tied to a unique counter). So a breakpoint might resolve differently across multiple targets but have the same identifier.
Created attachment 294860 [details] [PATCH] Proposed Fix
Seems related to the change: Regressions: Unexpected timeouts (1) inspector/worker/debugger-pause.html [ Timeout ]
(In reply to comment #4) > Seems related to the change: > > Regressions: Unexpected timeouts (1) > inspector/worker/debugger-pause.html [ Timeout ] Actually it shouldn't, since this only changes what tree elements are added.
Comment on attachment 294860 [details] [PATCH] Proposed Fix Clearing flags on attachment: 294860 Committed r208746: <http://trac.webkit.org/changeset/208746>
All reviewed patches have been landed. Closing bug.
(In reply to comment #5) > (In reply to comment #4) > > Seems related to the change: > > > > Regressions: Unexpected timeouts (1) > > inspector/worker/debugger-pause.html [ Timeout ] > > Actually it shouldn't, since this only changes what tree elements are added. Addressing this in: bug 164787 <https://webkit.org/b/164787> Web Inspector: inspector/worker/debugger-pause.html fails on WebKit1