[webkit-reviews] review denied: [Bug 76398] Web Inspector: HAR pageref attributes are wrong and inconsistent with pages array : [Attachment 123306] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Andrey Kosyakov
<caseq at chromium.org>'s request for review:
Bug 76398: Web Inspector: HAR pageref attributes are wrong and inconsistent
with pages array
https://bugs.webkit.org/show_bug.cgi?id=76398

Attachment 123306: Patch
https://bugs.webkit.org/attachment.cgi?id=123306&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
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