[webkit-reviews] review denied: [Bug 74188] Web Inspector: Generated HAR is missing pages.startedDateTime : [Attachment 119041] 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 74188: Web Inspector: Generated HAR is missing pages.startedDateTime
https://bugs.webkit.org/show_bug.cgi?id=74188

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

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=119041&action=review


> LayoutTests/http/tests/inspector/inspector-test.js:117
> +    return 0 <= delta && delta < 30 * 1000 ? "<plausible>" : value;

We should not rely upon current time in the tests.

> LayoutTests/http/tests/inspector/inspector-test.js:130
> +	   var formatter = nondeterministicProps &&
nondeterministicProps[prop];

It is hard for compiler to infer the type of this variable.

> LayoutTests/http/tests/inspector/inspector-test.js:133
> +	       InspectorTest.addResult(prefixWithName +
InspectorTest[formatter].call(InspectorTest, propValue));

Non-deterministic means that only the type should be printed.

> LayoutTests/http/tests/inspector/resources-test.js:12
> +    startedDateTime: "formatRecentTime",

This is a strange change to the non-deterministic properties. Should you test
for instanceof Date instead?