[webkit-reviews] review denied: [Bug 8475] document.lastModified gives no output if the response doesn't have a Last-Modified header : [Attachment 27201] Fix for lastModified when there is no header from server

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Jan 31 00:21:34 PST 2009


Alexey Proskuryakov <ap at webkit.org> has denied Jon at Chromium
<jon at chromium.org>'s request for review:
Bug 8475: document.lastModified gives no output if the response doesn't have a
Last-Modified header
https://bugs.webkit.org/show_bug.cgi?id=8475

Attachment 27201: Fix for lastModified when there is no header from server
https://bugs.webkit.org/attachment.cgi?id=27201&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
> +    static String unixEpoc = "01/01/1970 00:00:00";

A static String will be destroyed at process exit time, affecting performance.
You can use a DEFINE_STATIC_LOCAL macro to create the variable in a way that
won't affect performance.

> +    if (!header || !header.length()) {
> +	   return unixEpoc;
> +    }
> +    return header;

We don't put braces around single line statements. Also, the check can be just
header.isEmpty().

This patch needs to include an automated test, and a ChangeLog modification.

r- due to the reasons given above, and because it's not clear if the specified
behavior is intentional.


More information about the webkit-reviews mailing list