Bug 32810 - Date binding support for Objective-C
Summary: Date binding support for Objective-C
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 32698
Blocks:
  Show dependency treegraph
 
Reported: 2009-12-20 21:19 PST by Kent Tamura
Modified: 2010-01-26 20:17 PST (History)
5 users (show)

See Also:


Attachments
Proposed patch (4.65 KB, patch)
2009-12-21 21:43 PST, Kent Tamura
no flags Details | Formatted Diff | Diff
Proposed patch (rev.2) (4.62 KB, patch)
2009-12-22 00:31 PST, Kent Tamura
timothy: review-
Details | Formatted Diff | Diff
Proposed patch (rev.3) (8.53 KB, patch)
2009-12-23 23:46 PST, Kent Tamura
eric: review+
tkent: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2009-12-20 21:19:58 PST
Like Bug#32698 for JavaScriptCore, we need to support Date type binding for Objective-C DOM binding.
Comment 1 Kent Tamura 2009-12-21 21:43:01 PST
Created attachment 45365 [details]
Proposed patch

This patch is similar to patches in Bug#32698 and Bug#32699.
Comment 2 WebKit Review Bot 2009-12-21 21:43:53 PST
style-queue ran check-webkit-style on attachment 45365 [details] without any errors.
Comment 3 Kent Tamura 2009-12-21 22:04:21 PST
Generated code will be:

- (double)valueAsDate
{
    return IMPL->valueAsDate();
}

- (void)setValueAsDate:(double)newValueAsDate
{
    WebCore::ExceptionCode ec = 0;
    IMPL->setValueAsDate(newValueAsDate, ec);
    WebCore::raiseOnDOMError(ec);
}
Comment 4 Kent Tamura 2009-12-22 00:31:09 PST
Created attachment 45369 [details]
Proposed patch (rev.2)

- Remove #ifdef !LANGUAGE_COM.  COM binding support was removed.
Comment 5 WebKit Review Bot 2009-12-22 00:31:42 PST
style-queue ran check-webkit-style on attachment 45369 [details] without any errors.
Comment 6 Darin Adler 2009-12-22 10:33:25 PST
Comment on attachment 45369 [details]
Proposed patch (rev.2)

The type should be NSAbsoluteTime, not double.
Comment 7 Timothy Hatcher 2009-12-22 10:35:17 PST
Comment on attachment 45369 [details]
Proposed patch (rev.2)

WOuld making IsPrimitiveType return true for Date give you more of the code paths you need with out $idlType eq "Date"? Or is IsPrimitiveType used other places we don't want for Date? Could those be specialcased for Date?

Seems fine though…
Comment 8 Timothy Hatcher 2009-12-22 10:36:18 PST
Comment on attachment 45369 [details]
Proposed patch (rev.2)

Darin is right, this should be NSAbsoluteTime.
Comment 9 Darin Adler 2009-12-22 10:37:14 PST
I do think this can be done without adding "Date" in so many different places. Maybe we can claim it's a primitive type.
Comment 10 Kent Tamura 2009-12-22 11:27:18 PST
(In reply to comment #6)
> The type should be NSAbsoluteTime, not double.

Would you give me information about NSAbsoluteTime please?
I couldn't find it in header files and references.
Comment 11 Darin Adler 2009-12-22 11:40:43 PST
(In reply to comment #10)
> Would you give me information about NSAbsoluteTime please?
> I couldn't find it in header files and references.

Oops, sorry. It's NSTimeInterval (from NSDate.h). There should be an NSAbsoluteTime, but there is not!
Comment 12 Darin Adler 2009-12-22 11:43:01 PST
(In reply to comment #11)
> Oops, sorry. It's NSTimeInterval (from NSDate.h). There should be an
> NSAbsoluteTime, but there is not!

But also, dates inside WebKit are in seconds from a base of 1970, whereas in AppKit they have a base of 2001. So we will need to correct by kCFAbsoluteTimeIntervalSince1970 as we do in CurrentTime.cpp.
Comment 13 Timothy Hatcher 2009-12-22 13:00:14 PST
(In reply to comment #11)
> (In reply to comment #10)
> > Would you give me information about NSAbsoluteTime please?
> > I couldn't find it in header files and references.
> 
> Oops, sorry. It's NSTimeInterval (from NSDate.h). There should be an
> NSAbsoluteTime, but there is not!

There is CFAbsoluteTime, documented with an epoch of 2001. But I think NSTimeInterval is better to use since we are not a CF API.
Comment 14 Kent Tamura 2009-12-23 23:46:26 PST
Created attachment 45464 [details]
Proposed patch (rev.3)

- Map Date to NSTimeInterval
- Introduce functions to convert double from/to NSTimeInterval.
- Make Date a primitive type
Comment 15 WebKit Review Bot 2009-12-23 23:51:45 PST
style-queue ran check-webkit-style on attachment 45464 [details] without any errors.
Comment 16 Eric Seidel (no email) 2010-01-26 14:30:33 PST
Comment on attachment 45464 [details]
Proposed patch (rev.3)

Looks good to me.
Comment 17 Kent Tamura 2010-01-26 20:17:33 PST
Committed r53890: <http://trac.webkit.org/changeset/53890>