135367 2014-07-28 16:27:56 -0700 Web Inspector: ProbeSetDetailsSection should not create live location links for unresolved breakpoints 2014-07-30 14:47:09 -0700 1 1 1 Unclassified WebKit Web Inspector 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 135396 1 joepeck burg burg graouts joepeck timothy webkit-bug-importer oldest_to_newest 1025300 0 joepeck 2014-07-28 16:27:56 -0700 * SUMMARY Opening the inspector in local builds I see a lot of asserts trying to make a display string for an non-resolved breakpoint. The Breakpoint's SourceCodeLocation will have a null sourceCode, and produces an assert. * STEPS TO REPRODUCE 1. Set breakpoints with probes on multiple domains (e.g. bogojoker.com/shell/, nytimes) 2. Load apple.com 3. Open inspector => asserts * DETAILS console.assert(sourceCode); [Error] Assertion failed: _locationString (SourceCodeLocation.js, line 294) originalLocationString (SourceCodeLocation.js, line 174) tooltipString (SourceCodeLocation.js, line 190) populateLiveDisplayLocationTooltip (SourceCodeLocation.js, line 243) createSourceCodeLocationLink (Main.js, line 1558) _probeSetPositionTextOrLink (ProbeSetDetailsSection.js, line 92) ProbeSetDetailsSection (ProbeSetDetailsSection.js, line 50) _probeSetAdded (ProbeDetailsSidebarPanel.js, line 93) dispatch (Object.js, line 141) dispatchEventToListeners (Object.js, line 148) _probeSetForBreakpoint (ProbeManager.js, line 178) (anonymous function) (ProbeManager.js, line 137) forEach _breakpointActionsChanged (ProbeManager.js, line 129) _breakpointAdded (ProbeManager.js, line 98) dispatch (Object.js, line 141) dispatchEventToListeners (Object.js, line 148) addBreakpoint (DebuggerManager.js, line 256) restoreBreakpointsSoon (DebuggerManager.js, line 71) restoreBreakpointsSoon ([native code], line 0) 1025302 1 webkit-bug-importer 2014-07-28 16:28:41 -0700 <rdar://problem/17836190> 1025591 2 235708 burg 2014-07-29 16:06:49 -0700 Created attachment 235708 Patch 1025606 3 235708 timothy 2014-07-29 16:34:07 -0700 Comment on attachment 235708 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? 1025657 4 235708 joepeck 2014-07-29 21:19:51 -0700 Comment on attachment 235708 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. 1025723 5 burg 2014-07-30 09:46:41 -0700 (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) 1025800 6 burg 2014-07-30 14:47:09 -0700 Committed r171819: <http://trac.webkit.org/changeset/171819> 235708 2014-07-29 16:06:49 -0700 2014-07-29 21:19:51 -0700 Patch bug-135367-20140729160610.patch text/plain 4145 burg U3VidmVyc2lvbiBSZXZpc2lvbjogMTcxNzExCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVj dG9yVUkvQ2hhbmdlTG9nIGIvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZwppbmRleCAx ZGJhMDc1NTU0NDg3YWQ5NDg4YjU1ZTY1YTEzZDgwNjZhMWM3NGQwLi5lNDgyYzU1N2Q5OTY2M2Fi ZmI3ODdmZWM4NjcwMzcxZTc5MDZhNmU4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5zcGVjdG9y VUkvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKQEAgLTEs NSArMSwyMCBAQAogMjAxNC0wNy0yOSAgQnJpYW4gSi4gQnVyZyAgPGJ1cmdAY3Mud2FzaGluZ3Rv bi5lZHU+CiAKKyAgICAgICAgV2ViIEluc3BlY3RvcjogUHJvYmVTZXREZXRhaWxzU2VjdGlvbiBz aG91bGQgbm90IGNyZWF0ZSBsaXZlIGxvY2F0aW9uIGxpbmtzIGZvciB1bnJlc29sdmVkIGJyZWFr cG9pbnRzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0x MzUzNjcKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBS ZWdlbmVyYXRlIHRoZSBzb3VyY2UgY29kZSBsaW5rIHdoZW5ldmVyIHRoZSBicmVha3BvaW50IHJl c29sdmVkIHN0YXR1cyBjaGFuZ2VzLiBJZiBpdCBpcworICAgICAgICByZXNvbHZlZCwgdGhlbiB3 ZSBjYW4gY3JlYXRlIGEgbGl2ZSBsb2NhdGlvbiBsaW5rIHRoYXQgcmVzcGVjdHMgc291cmNlIG1h cHMuCisKKyAgICAgICAgKiBVc2VySW50ZXJmYWNlL1ZpZXdzL1Byb2JlU2V0RGV0YWlsc1NlY3Rp b24uanM6CisgICAgICAgIChXZWJJbnNwZWN0b3IuUHJvYmVTZXREZXRhaWxzU2VjdGlvbik6Cisg ICAgICAgIChXZWJJbnNwZWN0b3IuUHJvYmVTZXREZXRhaWxzU2VjdGlvbi5wcm90b3R5cGUuX3Vw ZGF0ZUxpbmtFbGVtZW50KTogQWRkZWQuCisgICAgICAgIChXZWJJbnNwZWN0b3IuUHJvYmVTZXRE ZXRhaWxzU2VjdGlvbi5wcm90b3R5cGUuX3Byb2JlU2V0UG9zaXRpb25UZXh0T3JMaW5rKTogRGVs ZXRlZC4KKworMjAxNC0wNy0yOSAgQnJpYW4gSi4gQnVyZyAgPGJ1cmdAY3Mud2FzaGluZ3Rvbi5l ZHU+CisKICAgICAgICAgV2ViIEluc3BlY3RvcjogYnJlYWtwb2ludHMgYXJlIGFsd2F5cyBzcGVj dWxhdGl2ZWx5IHJlc29sdmVkIHdoZW4gcmVzdG9yZWQgZnJvbSBsb2NhbCBzdG9yYWdlCiAgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMzUzOTYKIApkaWZm IC0tZ2l0IGEvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvUHJvYmVT ZXREZXRhaWxzU2VjdGlvbi5qcyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNl L1ZpZXdzL1Byb2JlU2V0RGV0YWlsc1NlY3Rpb24uanMKaW5kZXggZGZmNzIxNjA5MTMzZDJiNzJk YjA5OTlkZGNiMDc4OWE4Y2YzZTkzNC4uNzY5YWQzODhkNDMwYWFjMzdhMTM2ZWIwNmM5MDVmZTg0 Y2EzMDc4YSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2Uv Vmlld3MvUHJvYmVTZXREZXRhaWxzU2VjdGlvbi5qcworKysgYi9Tb3VyY2UvV2ViSW5zcGVjdG9y VUkvVXNlckludGVyZmFjZS9WaWV3cy9Qcm9iZVNldERldGFpbHNTZWN0aW9uLmpzCkBAIC0zMCw3 ICszMCw3IEBAIFdlYkluc3BlY3Rvci5Qcm9iZVNldERldGFpbHNTZWN0aW9uID0gZnVuY3Rpb24o cHJvYmVTZXQpCiAgICAgdGhpcy5fbGlzdGVuZXJzID0gbmV3IFdlYkluc3BlY3Rvci5FdmVudExp c3RlbmVyU2V0KHRoaXMsICJQcm9iZVNldERldGFpbHNTZWN0aW9uIFVJIGxpc3RlbmVycyIpOwog ICAgIHRoaXMuX3Byb2JlU2V0ID0gcHJvYmVTZXQ7CiAKLSAgICB2YXIgb3B0aW9uc0VsZW1lbnQg PSBkb2N1bWVudC5jcmVhdGVFbGVtZW50KCJkaXYiKTsKKyAgICB2YXIgb3B0aW9uc0VsZW1lbnQg PSB0aGlzLl9vcHRpb25zRWxlbWVudCA9IGRvY3VtZW50LmNyZWF0ZUVsZW1lbnQoImRpdiIpOwog ICAgIG9wdGlvbnNFbGVtZW50LmNsYXNzTGlzdC5hZGQoV2ViSW5zcGVjdG9yLlByb2JlU2V0RGV0 YWlsc1NlY3Rpb24uU2VjdGlvbk9wdGlvbnNTdHlsZUNsYXNzTmFtZSk7CiAKICAgICB2YXIgcmVt b3ZlUHJvYmVCdXR0b24gPSBvcHRpb25zRWxlbWVudC5jcmVhdGVDaGlsZCgiaW1nIik7CkBAIC00 Nyw5ICs0NywxMCBAQCBXZWJJbnNwZWN0b3IuUHJvYmVTZXREZXRhaWxzU2VjdGlvbiA9IGZ1bmN0 aW9uKHByb2JlU2V0KQogICAgIGFkZFByb2JlQnV0dG9uLmNsYXNzTGlzdC5hZGQoV2ViSW5zcGVj dG9yLlByb2JlU2V0RGV0YWlsc1NlY3Rpb24uQWRkUHJvYmVWYWx1ZVN0eWxlQ2xhc3NOYW1lKTsK ICAgICB0aGlzLl9saXN0ZW5lcnMucmVnaXN0ZXIoYWRkUHJvYmVCdXR0b24sICJjbGljayIsIHRo aXMuX2FkZFByb2JlQnV0dG9uQ2xpY2tlZCk7CiAKLSAgICB2YXIgdGl0bGVFbGVtZW50ID0gdGhp cy5fcHJvYmVTZXRQb3NpdGlvblRleHRPckxpbmsoKTsKLSAgICB0aXRsZUVsZW1lbnQuY2xhc3NM aXN0LmFkZChXZWJJbnNwZWN0b3IuUHJvYmVTZXREZXRhaWxzU2VjdGlvbi5Eb250RmxvYXRMaW5r U3R5bGVDbGFzc05hbWUpOwotICAgIG9wdGlvbnNFbGVtZW50LmFwcGVuZENoaWxkKHRpdGxlRWxl bWVudCk7CisgICAgLy8gVXBkYXRlIHRoZSBzb3VyY2UgbGluayB3aGVuIHRoZSBicmVha3BvaW50 J3MgcmVzb2x2ZWQgc3RhdGUgY2hhbmdlcywKKyAgICAvLyBzbyB0aGF0IGl0IGNhbiBiZWNvbWUg YSBsaXZlIGxvY2F0aW9uIGxpbmsgd2hlbiBwb3NzaWJsZS4KKyAgICB0aGlzLl91cGRhdGVMaW5r RWxlbWVudCgpOworICAgIHRoaXMuX2xpc3RlbmVycy5yZWdpc3Rlcih0aGlzLl9wcm9iZVNldC5i cmVha3BvaW50LCBXZWJJbnNwZWN0b3IuQnJlYWtwb2ludC5FdmVudC5SZXNvbHZlZFN0YXRlRGlk Q2hhbmdlLCB0aGlzLl91cGRhdGVMaW5rRWxlbWVudCk7CiAKICAgICB0aGlzLl9kYXRhR3JpZCA9 IG5ldyBXZWJJbnNwZWN0b3IuUHJvYmVTZXREYXRhR3JpZChwcm9iZVNldCk7CiAgICAgdmFyIHNp bmdsZXRvblJvdyA9IG5ldyBXZWJJbnNwZWN0b3IuRGV0YWlsc1NlY3Rpb25Sb3c7CkBAIC04Niwx MCArODcsMjQgQEAgV2ViSW5zcGVjdG9yLlByb2JlU2V0RGV0YWlsc1NlY3Rpb24ucHJvdG90eXBl ID0gewogCiAgICAgLy8gUHJpdmF0ZQogCi0gICAgX3Byb2JlU2V0UG9zaXRpb25UZXh0T3JMaW5r OiBmdW5jdGlvbigpCisgICAgX3VwZGF0ZUxpbmtFbGVtZW50OiBmdW5jdGlvbigpCiAgICAgewog ICAgICAgICB2YXIgYnJlYWtwb2ludCA9IHRoaXMuX3Byb2JlU2V0LmJyZWFrcG9pbnQ7Ci0gICAg ICAgIHJldHVybiBXZWJJbnNwZWN0b3IuY3JlYXRlU291cmNlQ29kZUxvY2F0aW9uTGluayhicmVh a3BvaW50LnNvdXJjZUNvZGVMb2NhdGlvbik7CisgICAgICAgIHZhciB0aXRsZUVsZW1lbnQgPSBu dWxsOworICAgICAgICBpZiAoYnJlYWtwb2ludC5yZXNvbHZlZCkKKyAgICAgICAgICAgIHRpdGxl RWxlbWVudCA9IFdlYkluc3BlY3Rvci5jcmVhdGVTb3VyY2VDb2RlTG9jYXRpb25MaW5rKGJyZWFr cG9pbnQuc291cmNlQ29kZUxvY2F0aW9uKTsKKyAgICAgICAgZWxzZSB7CisgICAgICAgICAgICB2 YXIgbG9jYXRpb24gPSBicmVha3BvaW50LnNvdXJjZUNvZGVMb2NhdGlvbjsKKyAgICAgICAgICAg IHRpdGxlRWxlbWVudCA9IFdlYkluc3BlY3Rvci5saW5raWZ5TG9jYXRpb24oYnJlYWtwb2ludC51 cmwsIGxvY2F0aW9uLmRpc3BsYXlMaW5lTnVtYmVyLCBsb2NhdGlvbi5kaXNwbGF5Q29sdW1uTnVt YmVyKTsKKyAgICAgICAgfQorCisgICAgICAgIHRpdGxlRWxlbWVudC5jbGFzc0xpc3QuYWRkKFdl Ykluc3BlY3Rvci5Qcm9iZVNldERldGFpbHNTZWN0aW9uLkRvbnRGbG9hdExpbmtTdHlsZUNsYXNz TmFtZSk7CisKKyAgICAgICAgaWYgKHRoaXMuX2xpbmtFbGVtZW50KQorICAgICAgICAgICAgdGhp cy5fb3B0aW9uc0VsZW1lbnQucmVtb3ZlQ2hpbGQodGhpcy5fbGlua0VsZW1lbnQpOworCisgICAg ICAgIHRoaXMuX2xpbmtFbGVtZW50ID0gdGl0bGVFbGVtZW50OworICAgICAgICB0aGlzLl9vcHRp b25zRWxlbWVudC5hcHBlbmRDaGlsZCh0aGlzLl9saW5rRWxlbWVudCk7CiAgICAgfSwKIAogICAg IF9hZGRQcm9iZUJ1dHRvbkNsaWNrZWQ6IGZ1bmN0aW9uKGV2ZW50KQo=