msToGregorianDateTime has an outputIsUTC parameter - if false, the output should be in localtime. If the parameter is true, the result GregorianDateTime structure is filled with a time in UTC, but the utcOffset field is still set to the local timezone offset as if it were filled with a local time. I believe it should always be 0 if outputIsUTC is true. This happens to work in most cases because utcOffset is never used - it happens that everywhere msToGregorianDateTime is called with outputIsUTC true, it's in a code path which doesn't look at utcOffset. I don't think we should be depending on this, though.
Created attachment 32358 [details] patch to DateMath.cpp
So this code path is never used?
The most common idiom boils down to: msToGregorianDateTime(millisecs, outputIsUTC, t); if (outputIsUTC) // do something with t.month, t.year, etc. but not t.utcOffset else // do something with t.month, t.year, etc. adjusting for t.utcOffset So the code path in this patch is followed, but in the case where utcOffset is wrong it isn't used anyway. I believe all calls to msToGregorianDateTime work like this, but some of them are pretty complex so I can't be 100% sure. We have this patch in our WinCE branch because at one point Google Calendar didn't work for us without it. I thought that it worked now, but I was submitting it anyway because it seemed like the correct thing to do (and I figured if not we would find that out during the code review.) However, Yong thinks Google Calendar still doesn't work for us without this patch. I'm trying to find a test case now.
Joe, can you update this bug with latest info you have about the testcase?
I haven't been able to find any broken behaviour that this patch fixes. However, I also haven't been able to find anything that it breaks, so I think it should go in because it makes the code more resistant to future breakage.
Comment on attachment 32358 [details] patch to DateMath.cpp absence of comments otherwise and it looks both correct and reasonable to me, so r+
Landed with r45918.