Patch to follow. Tests for this mapping will be added once snippet could be evaluated.
Created attachment 131421 [details] Patch
Comment on attachment 131421 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131421&action=review > Source/WebCore/inspector/front-end/SnippetsModel.js:227 > + WebInspector.snippetsModel.addEventListener(WebInspector.SnippetsModel.EventTypes.SnippetDeleted, this._snippetDeleted.bind(this)); SnippetRemoved would be more consistent with other code. > Source/WebCore/inspector/front-end/SnippetsModel.js:237 > + var snippet = this._snippetForScriptId[rawLocation.scriptId]; Could you only store the last script in _snippetForScriptId and delete stale scripts (_snippetForLastScriptId)? There should be no need to access snippet for stale scripts. > Source/WebCore/inspector/front-end/SnippetsModel.js:242 > + if (!uiSourceCode) It's guaranteed that addScript was already called for this script, so uiSourceCode should always exist here. > Source/WebCore/inspector/front-end/SnippetsModel.js:261 > + if (!lastScript) There's no script for this UISourceCode. How could there be a lastScript for the snippet? > Source/WebCore/inspector/front-end/SnippetsModel.js:262 > + return null; Please make sure all clients will check for null. > Source/WebCore/inspector/front-end/SnippetsModel.js:274 > + result.push(snippet.uiSourceCode); Wouldn't there be duplicates? > Source/WebCore/inspector/front-end/SnippetsModel.js:289 > + var uiSourceCode = new WebInspector.UISourceCode(script.sourceURL, script.sourceURL, new WebInspector.ScriptContentProvider(script)); You should probably bind this script to existing snippet.uiSourceCode instead of creating a new one. > Source/WebCore/inspector/front-end/SnippetsModel.js:294 > + if (!snippet) Is it possible? Maybe preventing the snippet from being deleted while it's evaluated would make the code easier to understand. > Source/WebCore/inspector/front-end/SnippetsModel.js:310 > + uiSourceCode.isSnippet = true; uiSourceCode.isEditable = true? > Source/WebCore/inspector/front-end/SnippetsModel.js:350 > + this._lastScriptForSnippet = new Map(); Clear other maps as well?
Comment on attachment 131421 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131421&action=review >> Source/WebCore/inspector/front-end/SnippetsModel.js:227 >> + WebInspector.snippetsModel.addEventListener(WebInspector.SnippetsModel.EventTypes.SnippetDeleted, this._snippetDeleted.bind(this)); > > SnippetRemoved would be more consistent with other code. Done. >> Source/WebCore/inspector/front-end/SnippetsModel.js:237 >> + var snippet = this._snippetForScriptId[rawLocation.scriptId]; > > Could you only store the last script in _snippetForScriptId and delete stale scripts (_snippetForLastScriptId)? There should be no need to access snippet for stale scripts. Done. >> Source/WebCore/inspector/front-end/SnippetsModel.js:242 >> + if (!uiSourceCode) > > It's guaranteed that addScript was already called for this script, so uiSourceCode should always exist here. Done. >> Source/WebCore/inspector/front-end/SnippetsModel.js:261 >> + if (!lastScript) > > There's no script for this UISourceCode. How could there be a lastScript for the snippet? I have removed _lastScriptForSnippet map at all and rewritten this piece of code as we discussed offline. >> Source/WebCore/inspector/front-end/SnippetsModel.js:289 >> + var uiSourceCode = new WebInspector.UISourceCode(script.sourceURL, script.sourceURL, new WebInspector.ScriptContentProvider(script)); > > You should probably bind this script to existing snippet.uiSourceCode instead of creating a new one. Done. >> Source/WebCore/inspector/front-end/SnippetsModel.js:310 >> + uiSourceCode.isSnippet = true; > > uiSourceCode.isEditable = true? Done. >> Source/WebCore/inspector/front-end/SnippetsModel.js:350 >> + this._lastScriptForSnippet = new Map(); > > Clear other maps as well? No need to clear snippet<->uiSourceCode maps, since snippet are not removed upon navigation.
Created attachment 131614 [details] Patch
Created attachment 131669 [details] Patch
Created attachment 131686 [details] Patch
Comment on attachment 131614 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131614&action=review looks good! > Source/WebCore/inspector/front-end/ScriptMapping.js:178 > + if (WebInspector.snippetsModel.snippetForSourceURL(script.sourceURL)) There could be no snippet at this moment. Please check that script.sourceURL starts with "snippet://" instead. > Source/WebCore/inspector/front-end/SnippetsModel.js:391 > + this._uiSourceCodeForScriptId = {}; Do you need to dispatch removed events here?
Comment on attachment 131686 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131686&action=review > Source/WebCore/inspector/front-end/SnippetsModel.js:307 > + _uiSourceCodesForScripts: function() Not uiSourceCodesForScripts actually, maybe releasedUISourceCodes?
Created attachment 131808 [details] Patch
Committed r110681: <http://trac.webkit.org/changeset/110681>