Summary: | Web Inspector: ProbeSetDetailsSection should not create live location links for unresolved breakpoints | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||
Component: | Web Inspector | Assignee: | Brian Burg <burg> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | burg, graouts, joepeck, timothy, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Bug Depends on: | 135396 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Joseph Pecoraro
2014-07-28 16:27:56 PDT
Created attachment 235708 [details]
Patch
Comment on attachment 235708 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235708&action=review > Source/WebInspectorUI/UserInterface/Views/ProbeSetDetailsSection.js:98 > + var location = breakpoint.sourceCodeLocation; > + titleElement = WebInspector.linkifyLocation(breakpoint.url, location.displayLineNumber, location.displayColumnNumber); This could use a comment. Maybe an assert that breakpoint.sourceCodeLocation.sourceCode is null? Comment on attachment 235708 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235708&action=review > Source/WebInspectorUI/ChangeLog:9 > + Regenerate the source code link whenever the breakpoint resolved status changes. If it is > + resolved, then we can create a live location link that respects source maps. Why do we need ProbeSet* objects for every breakpoints, resolved or not? Is it needed in order to support WebReplay recordings? If these objects only existed for resolved breakpoints, then almost all the changes in this patch could go away. That was my first take on how we would approach this. (In reply to comment #4) > (From update of attachment 235708 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=235708&action=review > > > Source/WebInspectorUI/ChangeLog:9 > > + Regenerate the source code link whenever the breakpoint resolved status changes. If it is > > + resolved, then we can create a live location link that respects source maps. > > Why do we need ProbeSet* objects for every breakpoints, resolved or not? > > Is it needed in order to support WebReplay recordings? If these objects only existed for resolved breakpoints, then almost all the changes in this patch could go away. That was my first take on how we would approach this. The view objects (probe section, etc) could be lazily created. The model objects should not be lazily loaded. It would be nice to not lose samples when the page is reloaded, just like the console. To do this, the probe set needs to stay alive across resolved-unresolved-resolved transitions when reloading. Navigating away could clear the probe samples, but the UI should be clearly delineating when the samples are from a previous load anyway. There are replay implications. In the replay setting, we want to retain probe sets across multiple executions of the same recording so that users can fill in missing table values in future playbacks. (RemoteObjects for old executions are clearly flushed) Committed r171819: <http://trac.webkit.org/changeset/171819> |