867312 - Expose some APIs for working with dates the way the JS engine does

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

For a start, js/public/Date.h which has a js::MakeDate or something which takes an integer year, a 0-11 month, and a 1-based day and produces a double millisecond timestamp.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Expose some APIs for creating millisecond timestamps corresponding to JS dates.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Expose some APIs for creating millisecond timestamps corresponding to JS dates.

Comment 3

[Boris Zbarsky [:bzbarsky]]

I guess it's not clear whether MakeDate should call TimeClip....

Comment 4

[Jeff Walden [:Waldo]]

Comment on attachment 743792 [details] [diff] [review]
Expose some APIs for creating millisecond timestamps corresponding to JS dates.

Review of attachment 743792 [details] [diff] [review]:
-----------------------------------------------------------------

Some sort of test that would have failed with the old <input> behavior would be nice, but I can't imagine it matters much, as no browser would ever be so stupid as to construct a Date object from the time value and call methods on it, assuming Date.prototype.* has its original values.  Right?  ;-)

::: js/public/Date.h
@@ +1,5 @@
> +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +

Add a #include "jstypes.h", please, so that JS_PUBLIC_API is actually defined and not bootlegged.  :-)

And, um, #include guards, please?  Copy the style from some other js/public header.

::: js/src/jsdate.cpp
@@ +48,5 @@
>  #include "jsobjinlines.h"
>  
>  using namespace js;
>  using namespace js::types;
> +using namespace JS;

Is this actually necessary?  I'm not sure it is.  And I think we're moving in the direction of having explicit usings for namespace-JS stuff inside the engine, mostly, so this should go away somehow.

@@ +153,1 @@
>  YearFromTime(double t)

Generally we don't wrap things in |namespace| blocks any more -- add a JS:: at the start of the declaration instead.

JS_PUBLIC_API adds overhead for intra-library calls, as I understand it.  I'm kind of leery of assuming that extra overhead doesn't matter, without good reason.  Could you leave these as statics and do the same thing you did for JS::MakeDate and JS::DayFromTime, just to not tempt fate?

Comment 5

[Jeff Walden [:Waldo]]

Hmm, TimeClip.

HTML doesn't appear to impose any requirements on the range of dates that can be accurately represented.  Given that it's exposing JS Date objects in various places, it probably should.  Probably there should be a spec change, to the effect that if the represented date/time in question exceeds the TimeClip range, it's the appropriate kind of error?  That would naturally fall out of this, I think, if all these methods TimeClip'd their return value.  So *probably* these methods should all TimeClip.  But really it somewhat depends on whether the spec moves in the direction I expect/hope it would.  So I'd say make all these methods TimeClip for now, and if something needs to change later, we can change it.

I assume any local date/time strings will first be converted to UTC, before the code being changed here will get called?  There could be fuzziness around the very-distant edges when a time value is a *local* time value.  :-\

Comment 6

[Boris Zbarsky [:bzbarsky]]

> Right? 

No comment.  ;)

> And, um, #include guards, please?

Yikes, good catch.  Will add.

> Is this actually necessary?  I'm not sure it is.

The other obvious option is to do s/YearFromTime/JS::YearFromTime/ on the whole file, and likewise for MonthFromTime.  I can do that if you'd prefer.  Let me know?

> add a JS:: at the start of the declaration instead.

OK, can do, assuming you mean the definition, not the declaration.

> Could you leave these as statics and do the same thing you did for JS::MakeDate and
> JS::DayFromTime

OK.  Need to be a bit careful with the namespacing bits, but should be doable.  Then I won't need the JS:: bits or the using bits either, I guess.

> So I'd say make all these methods TimeClip for now

It only makes sense to TimeClip the MakeDate method, no?  Certainly TimeClip on the month or day makes no sense; not sure about the year.

> I assume any local date/time strings will first be converted to UTC, before the code
> being changed here will get called?

I think that's the idea, yes.

Comment 7

[Jeff Walden [:Waldo]]

(In reply to Boris Zbarsky (:bz) from comment #6)
> It only makes sense to TimeClip the MakeDate method, no?  Certainly TimeClip
> on the month or day makes no sense; not sure about the year.

Er, yes.  Commenting too quickly, not being precise enough.

Comment 8

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3f705f8db7c1

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3f705f8db7c1