Web Inspector: add compiler source mapping support to RawSourceCode.
Created attachment 108828 [details] Patch
Comment on attachment 108828 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108828&action=review > LayoutTests/inspector/debugger/raw-source-code.html:365 > + if (lineNumber < 10) 6 levels of nesting is too much even for the test. Please consider extracting these to the top level. > Source/WebCore/inspector/front-end/BreakpointManager.js:98 > + var rawLocation = breakpoint.uiSourceCode.rawSourceCode.sourceMapping.uiLocationToRawLocation(breakpoint.uiSourceCode, breakpoint.lineNumber); This breaks law of Demeter violently. You go 4 levels deep and pass reference to self to the call? This sounds like a candidate for UISourceCode.prototype.toRawLocation. It was already too much, but now it is just over the limit. > Source/WebCore/inspector/front-end/SourceFile.js:282 > +WebInspector.RawSourceCode.CompilerSourceMapping.prototype = { If sounds like you should have extracted RawSourceCode into its own class prior to introducing nested classes. > Source/WebCore/inspector/front-end/SourceFile.js:318 > +WebInspector.CompilerSourceMappingProvider = function() ditto > Source/WebCore/inspector/front-end/SourceFile.js:454 > +WebInspector.CompilerSourceMappingContentProvider = function(sourceURL, compilerSourceMappingProvider) ditto
(In reply to comment #2) > (From update of attachment 108828 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=108828&action=review > > > LayoutTests/inspector/debugger/raw-source-code.html:365 > > + if (lineNumber < 10) > > 6 levels of nesting is too much even for the test. Please consider extracting these to the top level. Done. > > > Source/WebCore/inspector/front-end/BreakpointManager.js:98 > > + var rawLocation = breakpoint.uiSourceCode.rawSourceCode.sourceMapping.uiLocationToRawLocation(breakpoint.uiSourceCode, breakpoint.lineNumber); > > This breaks law of Demeter violently. You go 4 levels deep and pass reference to self to the call? This sounds like a candidate for UISourceCode.prototype.toRawLocation. It was already too much, but now it is just over the limit. UISourceCode is an interface for ScriptsPanel, it should not mention rawLocations at all. UISourceCode::rawSourceCode is just a shortcut used by presentation model, it's not an actual link - UISourceCode and its clients should never use it. It could (and probably should) be replaced by UISourceCode->RawSourceCode mapping in the model to make things clear. > > > Source/WebCore/inspector/front-end/SourceFile.js:282 > > +WebInspector.RawSourceCode.CompilerSourceMapping.prototype = { > > If sounds like you should have extracted RawSourceCode into its own class prior to introducing nested classes. > > > Source/WebCore/inspector/front-end/SourceFile.js:318 > > +WebInspector.CompilerSourceMappingProvider = function() > > ditto > > > Source/WebCore/inspector/front-end/SourceFile.js:454 > > +WebInspector.CompilerSourceMappingContentProvider = function(sourceURL, compilerSourceMappingProvider) > > ditto Done.
Created attachment 109138 [details] Patch
Comment on attachment 109138 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109138&action=review > Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:285 > + var rawLocation = uiSourceCode.rawSourceCode.sourceMapping.uiLocationToRawLocation(uiSourceCode, lineNumber); Please put FIXME suggesting doing uiSourceCode.toRawLocation().
Committed r96585: <http://trac.webkit.org/changeset/96585>