Bug 76398 – Web Inspector: HAR pageref attributes are wrong and inconsistent with pages array

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Andrey Kosyakov]

Original report by Steve Souders:

====
A simple case is to just load two simple pages - Bing and Craigslist:
1. Clear your cache.
2. Select "Preserve Log upon Navigation" 
3. Go to Bing & Craigslist. In the waterfall chart UI verify that you see all the requests for both pages.
4. Right-click (2-finger click) and select "Save All as HAR" and save as chrome-bing-craigslist.har.
5. Open the .har file in a text editor - very that info for all requests is present - for example AutoSug.js (from Bing) and craigslist.css.
6. View the .har file in various tools included pcapperf, Har to Page Speed, and HAR Viewer. In the first two tools, only 3 requests for Craigslist are shown. In HAR Viewer only error messages are shown.

This might be the same bug, but when I try to get a HAR file for Gmail a similar thing happens. I can see all the requests in the Chrome dev tools (eg, logo_small.png) and also see them in the .har file, but when I try to view that file only 4 requests are shown (instead of ~30).
====

There are two related problems that cause this:
- The pages array only contains one page, the one after the last navigation; the correct behavior is to populate pages array with all pages that are present in the log;
- pageref attribute is incorrect for some HAR entries -- notably for those that followed the main resource redirect and those loaded from subframes.

[Andrey Kosyakov]

Created attachment 123275 [details]
Patch

[Andrey Kosyakov]

Created attachment 123306 [details]
Patch

[Pavel Feldman]

Comment on attachment 123306 [details]
Patch

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

A bunch of small nits.

> Source/WebCore/inspector/front-end/HAREntry.js:325
> +    _convertPage: function(page)

If you move this method to the Page class, you wouldn't need to expose all of the page's properties to public.

> Source/WebCore/inspector/front-end/NetworkLog.js:56
> +    pageForResource: function(resource)

pageLoadForResource ?

> Source/WebCore/inspector/front-end/NetworkLog.js:58
> +        return resource.__page;

Please use Map for this.

> Source/WebCore/inspector/front-end/NetworkLog.js:76
> +                resource.__page = this._currentPageLoad;

ditto

> Source/WebCore/inspector/front-end/NetworkLog.js:88
> +        var frame = WebInspector.resourceTreeModel.frameForId(resource.frameId);

this seems to be unused.

> Source/WebCore/inspector/front-end/NetworkLog.js:89
> +        resource.__page = this._currentPageLoad;

ditto

[Andrey Kosyakov]

(In reply to comment #3)
> (From update of attachment 123306 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=123306&action=review
> 
> A bunch of small nits.
> 
> > Source/WebCore/inspector/front-end/HAREntry.js:325
> > +    _convertPage: function(page)
> 
> If you move this method to the Page class, you wouldn't need to expose all of the page's properties to public.

As discussed offline, this is not quite practical, as convertPage() relies on certain methods internal to HAR. Besides, we pretend this is a more generic, model level than HAR support -- I think this may be eventually reused in NetworkPanel, perhaps in audits. As agreed, I'm just removing getters and use fields directly.

> 
> > Source/WebCore/inspector/front-end/NetworkLog.js:56
> > +    pageForResource: function(resource)
> 
> pageLoadForResource ?

done.

> 
> > Source/WebCore/inspector/front-end/NetworkLog.js:58
> > +        return resource.__page;
> 
> Please use Map for this.

That would be cumbersome due to need to eventually release map entries. For HAR at least, the resource entries are primary and we only need PageLoad entries as long as there are some (network) Resources that refer to them. The life-time of resource is a bit complicated, as some may be cleaned manually in the network panel, some may survive page reloads (again due to network panel), and life-time of resource references in NetworkLog is different from the Network panel.
 
WebInspector.resourceTreeModel.frameForId(resource.frameId);
> 
> this seems to be unused.

Thanks, fixed!

[Andrey Kosyakov]

Created attachment 123319 [details]
Patch

[Andrey Kosyakov]

Committed r105596: <http://trac.webkit.org/changeset/105596>