Calendar has experienced regression bug 769938 due to the patch for bug 711404, which fixes how xpconnect handles PRUint64. While the patch for bug 711404 is technically correct, we need to find a solution for PRTime: The problem that Calendar is experiencing, is that dates before 1970, which should have a negative native time, now underflow and mess up the date. I propose to change PRTime to long long, since dates before 1970 are currently much more common than those after 2038.
PRTime in NSPR is defined as PRInt64. (https://mxr.mozilla.org/mozilla-central/source/nsprpub/pr/include/prtime.h#48). But it is defined as unsigned long long in nosroot.idl (https://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsrootidl.idl#33). It is confusion. On CVS log, no reason exists... http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/xpcom/base&command=DIFF_FRAMESET&file=nsrootidl.idl&rev2=1.6&rev1=1.5.
Comment on attachment 639582 [details] [diff] [review] fix By bug 711404, we can handle PRUint64 on xpconnect correctly. But Although PRTime in NSPR is PRInt64, PRTime in nsroot.idl is defined as unsigned long long. So, we should change PRTime as long long.
Comment on attachment 639582 [details] [diff] [review] fix Please add a/some comment(s) explaining why we're testing timeProperty as -1. From a first glance, it's confusing.
Comment on attachment 639582 [details] [diff] [review] fix This is correct because PRTime is actually this in prtime.h, right? typedef PRInt64 PRTime; And so we're just making the IDL typedef match the correct C++ typedef?
(In reply to Benjamin Smedberg [:bsmedberg] from comment #5) You are correct. PRTime should be PRInt64 but is defined as PRUint64 in xpconnect by mistake. This was not observed as a problem in mozilla11 and older platform releases because PRUint64 was processed as PRInt64 in xpconnect by mistake and therefore PRTime worked as expected. The later mistake was fixed with Bug 711404 but unfortunately this fix regressed and broke PRTime in mozilla12 and newer platform releases.
Comment on attachment 639582 [details] [diff] [review] fix I'd appreciate if this patch could also be pushed to aurora and beta. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 711404 User impact if declined: Consumers of PRTime with dates before 1970 could be mistakingly placed far in the future, which might cause wrong dates to be placed in databases, etc. Risk to taking this patch (and alternatives if risky): Low risk, as PRTime has been (mistakingly) correct before Mozilla 12 all along. String or UUID changes made by this patch: nsIXPCTestObjectReadOnly, nsIXPCTestObjectReadWrite
Comment on attachment 639582 [details] [diff] [review] fix [Triage Comment] Given the fact that bug 711404 has been in the product since 12 (without obvious impact), I don't think we need to rush this past Aurora.
I'd appreciate if someone other than me could take care of the transplant, as I don't have the mozilla-aurora repository set up.