Bug 95352

Summary: Web Inspector: Create JavaScriptSources based on network resources.
Product: WebKit Reporter: Vsevolod Vlasov <vsevik>
Component: Web Inspector (Deprecated)Assignee: Vsevolod Vlasov <vsevik>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, dglazkov, joepeck, keishi, loislo, ossy, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 95695, 95729    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch pfeldman: review+, webkit.review.bot: commit-queue-

Vsevolod Vlasov
Reported 2012-08-29 09:36:15 PDT
Introduced NetworkUISourceCodeProvider that is listening for ResourceTreeModel and creates UISourceCodes for them. RawSourceCode does not create uiSourceCodes based on resource anymore (this is done by NetworkUISourceCodeProvider instead). Moved script-uiSourceCode binding logic from RawSourceCode to ResourceScriptMapping.
Attachments
Patch (77.98 KB, patch)
2012-08-29 09:49 PDT, Vsevolod Vlasov 		no flags
DetailsFormatted DiffDiff
Patch (93.60 KB, patch)
2012-09-03 07:44 PDT, Vsevolod Vlasov 		pfeldman: review+
webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Vsevolod Vlasov
Comment 1 2012-08-29 09:49:39 PDT
Created attachment 161252 [details] Patch
Pavel Feldman
Comment 2 2012-08-30 00:47:01 PDT
Comment on attachment 161252 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161252&action=review > Source/WebCore/inspector/front-end/NetworkUISourceCodeProvider.js:78 > + resource.request.addEventListener(WebInspector.NetworkRequest.Events.FinishedLoading, resourceFinished, this); We should only add resources that are finished at the first place. > Source/WebCore/inspector/front-end/NetworkUISourceCodeProvider.js:96 > + if ((isDocument && this._uiSourceCodeForDocumentResource[resource.url]) || (!isDocument && this._uiSourceCodeForScriptResource[resource.url])) Why do you distinguish those? > Source/WebCore/inspector/front-end/RawSourceCode.js:38 > +WebInspector.RawSourceCode = function(script) I don't see why we need this class now. > Source/WebCore/inspector/front-end/UISourceCode.js:545 > + setSourceMapping: function(sourceMapping) You should remove sourceMapping from the constructor now.
Vsevolod Vlasov
Comment 3 2012-09-03 07:44:48 PDT
Created attachment 161927 [details] Patch
Vsevolod Vlasov
Comment 4 2012-09-03 07:46:28 PDT
(In reply to comment #2) > (From update of attachment 161252 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=161252&action=review > > > Source/WebCore/inspector/front-end/NetworkUISourceCodeProvider.js:78 > > + resource.request.addEventListener(WebInspector.NetworkRequest.Events.FinishedLoading, resourceFinished, this); > > We should only add resources that are finished at the first place. I am going to fix this in another patch. > > > Source/WebCore/inspector/front-end/NetworkUISourceCodeProvider.js:96 > > + if ((isDocument && this._uiSourceCodeForDocumentResource[resource.url]) || (!isDocument && this._uiSourceCodeForScriptResource[resource.url])) > > Why do you distinguish those? This is needed to correctly process "dynamic" scripts (dynamically inserted script elements). > > > Source/WebCore/inspector/front-end/RawSourceCode.js:38 > > +WebInspector.RawSourceCode = function(script) > > I don't see why we need this class now. Removed. > > > Source/WebCore/inspector/front-end/UISourceCode.js:545 > > + setSourceMapping: function(sourceMapping) > > You should remove sourceMapping from the constructor now. Done
WebKit Review Bot
Comment 5 2012-09-03 07:58:07 PDT
Comment on attachment 161927 [details] Patch Attachment 161927 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13745145
Pavel Feldman
Comment 6 2012-09-03 08:26:10 PDT
Comment on attachment 161927 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161927&action=review > Source/WebCore/inspector/front-end/NetworkUISourceCodeProvider.js:94 > + if (this._uiSourceCodeForResource[resource.url]) You should check workspace for that. > Source/WebCore/inspector/front-end/NetworkUISourceCodeProvider.js:132 > + setTimeout(this._populate.bind(this), 0); Please use ProjectDidReset > Source/WebCore/inspector/front-end/ResourceScriptMapping.js:84 > + script.setSourceMapping(this); remove this. > Source/WebCore/inspector/front-end/ResourceScriptMapping.js:109 > + this._scriptIdForUISourceCode.put(uiSourceCode, scripts[i].scriptId); Remove that?
Vsevolod Vlasov
Comment 7 2012-09-03 09:33:50 PDT
Committed r127427: <http://trac.webkit.org/changeset/127427>
WebKit Review Bot
Comment 8 2012-09-03 10:09:34 PDT
Re-opened since this is blocked by 95695
Vsevolod Vlasov
Comment 9 2012-09-04 03:48:09 PDT
Committed r127454: <http://trac.webkit.org/changeset/127454>
Csaba Osztrogonác
Comment 10 2012-09-04 04:10:34 PDT
(In reply to comment #9) > Committed r127454: <http://trac.webkit.org/changeset/127454> It broke a test. Could you check it? Here is the new bug report - https://bugs.webkit.org/show_bug.cgi?id=95729 (Because reopening bugs is prohibited for me :) )
Note You need to log in before you can comment on or make changes to this bug.