Bug 81031

Summary: Web Inspector: MainScriptMapping should detect snippet scripts by means of sourceURL set before evaluation.
Product: WebKit Reporter: Vsevolod Vlasov <vsevik>
Component: Web Inspector (Deprecated)Assignee: Vsevolod Vlasov <vsevik>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, podivilov, rik, timothy, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 75094    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch pfeldman: review+

Vsevolod Vlasov
Reported 2012-03-13 13:37:04 PDT
MainScriptMapping should detect snippet scripts by means of sourceURL set before evaluation.
Attachments
Patch (19.62 KB, patch)
2012-03-14 12:58 PDT, Vsevolod Vlasov 		no flags
DetailsFormatted DiffDiff
Patch (19.87 KB, patch)
2012-03-14 13:01 PDT, Vsevolod Vlasov 		no flags
DetailsFormatted DiffDiff
Patch (20.11 KB, patch)
2012-03-15 09:15 PDT, Vsevolod Vlasov 		no flags
DetailsFormatted DiffDiff
Patch (20.07 KB, patch)
2012-03-15 10:17 PDT, Vsevolod Vlasov 		pfeldman: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Vsevolod Vlasov
Comment 1 2012-03-14 12:58:41 PDT
Created attachment 131901 [details] Patch
Vsevolod Vlasov
Comment 2 2012-03-14 13:01:17 PDT
Created attachment 131902 [details] Patch
Vsevolod Vlasov
Comment 3 2012-03-14 16:06:40 PDT
podivilov@, could you please take a look?
Pavel Podivilov
Comment 4 2012-03-15 02:27:06 PDT
Comment on attachment 131902 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131902&action=review > Source/WebCore/inspector/front-end/SnippetsModel.js:140 > + var sourceURL = this.sourceURLForSnippet(snippet, evaluationIndex); Please inline sourceURLForSnippet here and set snippet.sourceURL = sourceURL. > Source/WebCore/inspector/front-end/SnippetsModel.js:153 > + for (var i = 0; i < this._snippets.length; ++i) { Why not making this._snippets a map? > Source/WebCore/inspector/front-end/SnippetsModel.js:192 > + var snippetId = this.snippetIdForSourceURL(sourceURL); Please inline snippetIdForSourceURL here. > Source/WebCore/inspector/front-end/SnippetsModel.js:240 > + get lastEvaluationSourceURL() Please remove this public getter, all you need is a private field that is used solely by SnippetModel. > Source/WebCore/inspector/front-end/SnippetsModel.js:414 > + uiSourceCode.snippet = snippet; This looks wrong.
Vsevolod Vlasov
Comment 5 2012-03-15 09:15:10 PDT
Created attachment 132060 [details] Patch
Pavel Podivilov
Comment 6 2012-03-15 10:12:20 PDT
Comment on attachment 132060 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132060&action=review looks good! > Source/WebCore/inspector/front-end/SnippetsModel.js:153 > snippetIdForSourceURL: function(sourceURL) Please make it private.
Vsevolod Vlasov
Comment 7 2012-03-15 10:17:21 PDT
Created attachment 132068 [details] Patch
Vsevolod Vlasov
Comment 8 2012-03-15 10:46:00 PDT
> > Source/WebCore/inspector/front-end/SnippetsModel.js:153 > > snippetIdForSourceURL: function(sourceURL) > > Please make it private. It is used in MainScriptMapping to detect snippet scripts.
Vsevolod Vlasov
Comment 9 2012-03-15 11:27:02 PDT
Committed r110864: <http://trac.webkit.org/changeset/110864>
Note You need to log in before you can comment on or make changes to this bug.