Bug 53013 - Web Inspector: bind resources to URLs upon adding them into the tree.
Summary: Web Inspector: bind resources to URLs upon adding them into the tree.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-24 08:16 PST by Pavel Feldman
Modified: 2011-01-25 05:32 PST (History)
10 users (show)

See Also:

Attachments
[PATCH] Proposed change (15.40 KB, patch)
2011-01-25 04:06 PST, Pavel Feldman 		no flags Details | Formatted Diff | Diff
Patch (17.05 KB, patch)
2011-01-25 04:42 PST, Pavel Feldman 		yurys: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2011-01-24 08:16:44 PST 
I am slowly splitting the 'network request' vs 'resource item' aspects in the Resource.js. From now on, resource  is only bound with URL upon addition into the resource tree.
Comment 1 Pavel Feldman 2011-01-25 04:06:11 PST 
Created attachment 80045 [details]
[PATCH] Proposed change
Comment 2 Andrey Kosyakov 2011-01-25 04:33:37 PST 
Comment on attachment 80045 [details]
[PATCH] Proposed change

View in context: https://bugs.webkit.org/attachment.cgi?id=80045&action=review

> Source/WebCore/inspector/front-end/NetworkManager.js:-305
> -        this._resourceTreeModel.unbindResourceURL(originalResource);

Shouldn't we clean up _inflightResourceByURL entry for the original URL here? There doesn't seem to be another chance.

> Source/WebCore/inspector/front-end/NetworkManager.js:330
> +        delete this._inflightResourcesByURL[resource.identifier];

you need to use resource.url, not identifier here.

> Source/WebCore/inspector/front-end/ScriptsPanel.js:276
> +        var resource = WebInspector.networkManager.inflightResourceForURL(script.sourceURL) || WebInspector.resourceForURL(script.sourceURL);

So, is this guaranteed to be called before resource is finished? Otherwise, there wouldn't be in inflight resources.
Comment 3 Pavel Feldman 2011-01-25 04:40:37 PST 
(In reply to comment #2)
> (From update of attachment 80045 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80045&action=review
> 
> > Source/WebCore/inspector/front-end/NetworkManager.js:-305
> > -        this._resourceTreeModel.unbindResourceURL(originalResource);
> 
> Shouldn't we clean up _inflightResourceByURL entry for the original URL here? There doesn't seem to be another chance.
> 

Now that bind is encapsulated in ResourceTreeModel, we only perform it upon resource addition to the tree. Which happens upon didReceiveResponse, i.e. does not happen for intermediate redirects.

> > Source/WebCore/inspector/front-end/NetworkManager.js:330
> > +        delete this._inflightResourcesByURL[resource.identifier];
> 
> you need to use resource.url, not identifier here.
>

Thanks, fixed.
 
> > Source/WebCore/inspector/front-end/ScriptsPanel.js:276
> > +        var resource = WebInspector.networkManager.inflightResourceForURL(script.sourceURL) || WebInspector.resourceForURL(script.sourceURL);
> 
> So, is this guaranteed to be called before resource is finished? Otherwise, there wouldn't be in inflight resources.

Yes, it is.
Comment 4 Pavel Feldman 2011-01-25 04:42:52 PST 
Created attachment 80049 [details]
Patch
Comment 5 Pavel Feldman 2011-01-25 05:32:57 PST 
Committed r76587: <http://trac.webkit.org/changeset/76587>