Bug 164334 – Web Inspector: URL Breakpoints that resolve in multiple workers should only appear in the UI once

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

[Joseph Pecoraro]

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.

[Radar WebKit Bug Importer]

<rdar://problem/29073523>

[Joseph Pecoraro]

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.

[Joseph Pecoraro]

Created attachment 294860 [details]
[PATCH] Proposed Fix

[Matt Baker]

Seems related to the change:

Regressions: Unexpected timeouts (1)
  inspector/worker/debugger-pause.html [ Timeout ]

[Matt Baker]

(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.

[WebKit Commit Bot]

Comment on attachment 294860 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 294860

Committed r208746: <http://trac.webkit.org/changeset/208746>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

(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