remove nsInt64 usage from cookies

RESOLVED FIXED

Status

()

RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: dwitte, Assigned: dwitte)

Tracking

unspecified
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

12 years ago
nsInt64 is deprecated, so we can switch back to plain ol' PRInt64/PRUint64 here.
(Assignee)

Comment 1

12 years ago
Created attachment 268212 [details] [diff] [review]
patch v1

lots of simple one-line changes. removes usages of nsInt64, and also tweaks some API's to use PRUint64 instead of PRInt64 where appropriate.
Attachment #268212 - Flags: superreview?(cbiesinger)
Attachment #268212 - Flags: review?(cbiesinger)
(Assignee)

Comment 2

12 years ago
Created attachment 268213 [details] [diff] [review]
patch v1 supplement

whoops, missed an instance.
Attachment #268213 - Flags: superreview?(cbiesinger)
Attachment #268213 - Flags: review?(cbiesinger)
Comment on attachment 268212 [details] [diff] [review]
patch v1

shouldn't expiry allow negative values so that it can handle servers that send a pre-1970 date?
(Assignee)

Comment 4

12 years ago
you're right, that was an oversight. for some reason i thought PRTime was a PRUint64, but it's signed, so we could get negative values. new patch coming in a few hours.
(Assignee)

Updated

12 years ago
Attachment #268212 - Flags: superreview?(cbiesinger)
Attachment #268212 - Flags: review?(cbiesinger)
(Assignee)

Updated

12 years ago
Attachment #268213 - Flags: superreview?(cbiesinger)
Attachment #268213 - Flags: review?(cbiesinger)
(Assignee)

Comment 5

12 years ago
Created attachment 268705 [details] [diff] [review]
v2

back to PRInt64.
Attachment #268212 - Attachment is obsolete: true
Attachment #268213 - Attachment is obsolete: true
Attachment #268705 - Flags: superreview?(cbiesinger)
Attachment #268705 - Flags: review?(cbiesinger)
Comment on attachment 268705 [details] [diff] [review]
v2

netwerk/cookie/public/nsICookie.idl
+     * @status DEPRECATED - see nsICookie2.expiry.

Doxygen has an @deprecated command, which is probably more appropriate here
(http://www.stack.nl/~dimitri/doxygen/commands.html#cmddeprecated)

netwerk/cookie/src/nsCookieService.cpp
-  const PRBool newCookie = ParseAttributes(aCookieHeader, cookieAttributes);
+  PRBool newCookie = ParseAttributes(aCookieHeader, cookieAttributes);

why remove the const?

-  const nsInt64 currentTimeInUsec = NOW_IN_MICROSECONDS;
+  PRInt64 currentTimeInUsec = PR_Now();

here too
Attachment #268705 - Flags: superreview?(cbiesinger)
Attachment #268705 - Flags: superreview+
Attachment #268705 - Flags: review?(cbiesinger)
Attachment #268705 - Flags: review+
btw, why does some of this code use PRTime values (µsec), while other code uses seconds since 1970?
(Assignee)

Comment 8

12 years ago
neat; i'll use the @deprecated instead.

re the const, it looked a little weird to me, and any compiler worth its salt will realize it's not modified after assignment anyway.

re usec vs sec, expiry is stored as seconds (by convention). creationtime was introduced (actually it's still future tense, since i haven't checked it in yet) with the storage work, and is stored as microseconds. this is because it doubles as a unique id for each cookie in the database, so usec is more appropriate since it's more likely to be unique. (note that we do check for collisions in order to guarantee that.) so it's a little confusing that we use sec and usec, but i've tried to make it clearer in the code, by naming variables either |currentTime| or |currentTimeInUsec| :/

thanks biesi!
(Assignee)

Comment 9

12 years ago
fixed on trunk.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Updated

12 years ago
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.