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
Vsevolod Vlasov
2012-08-29 09:36:15 PDT
Created attachment 161252 [details]
Patch
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. Created attachment 161927 [details]
Patch
(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 Comment on attachment 161927 [details] Patch Attachment 161927 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13745145 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? Committed r127427: <http://trac.webkit.org/changeset/127427> Re-opened since this is blocked by 95695 Committed r127454: <http://trac.webkit.org/changeset/127454> (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 :) ) |