Using base::Time* types in blink (starting with Events)

[Majid Valipour]

Hi,

At the moment, we use double (and sometimes long long) for all kinds of timestamps in Blink. We also have DOMHighResTimeStamp and DOMTimeStamp which are just type alias for the above.

Every time we convert a timestamp value from Chromium to Blink we lose valuable information such as time type, and its scale. There is already a bug that proposed to do this for whole of blink. 

I like to start this process by changing the timestamp type for platform/PlatformEvent and core/Event. Do you have any objection or advice on this? I believe the preference is usually to have a think wrapper in wtf for these types.

Thanks

Majid

[Kentaro Hara]

I support this.

Yes, a thin wrapper in wtf/ sounds good.

 

Thanks

Majid

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAB8RdXvTmi6fCDyJn6mJRE6gEEJCS0XLSHsimcNcscyEC2_unQ%40mail.gmail.com.

--

Kentaro Hara, Tokyo, Japan

[Yuta Kitamura]

I'm generally supportive of this. However, be wary of performance regressions: event is one of performance-sensitive areas.

(Personally, I feel it somewhat hard to use time.h interfaces correctly, though.)

[Elliott Sprehn]

I think this is okay if we use a wrapper and limit the surface area of base::Time* we expose as we go. I don't really want to expose things like FromString, it uses nspr which requires null terminated strings. I don't think we want string copies and conversions, we should support parsing dates in WTF or make the base thing support our strings.

[Brett Wilson]

On Wed, Apr 27, 2016 at 11:28 AM, Elliott Sprehn <esp...@chromium.org> wrote:

I think this is okay if we use a wrapper and limit the surface area of base::Time* we expose as we go. I don't really want to expose things like FromString, it uses nspr which requires null terminated strings. I don't think we want string copies and conversions, we should support parsing dates in WTF or make the base thing support our strings.

I don't like NSPR. If we good and date-parsing code that we can adapt to use StringPiece & base types, move to base, and delete base/third_party/nspr, that sounds like a positive improvement to me (will need to be very careful about compat). It may also be the case that the NSPR code is easy to convert to StringPiece itself. A brief look at the code doesn't obviously disprove this.

Brett

[Majid Valipour]

Hi, 

Just wanted to close the loop on this one. I have landed an initial patch to expose wrapped base time types in WTF and also have used them successfully for Blink events \o/. (Thanks esprehn@ for reviewing and providing feedback)

I think the conversion process is best be done by each individual time at their own pace. To help facilitate this process I have written a small doc to guide Blink devs that may not be familiar with base/time types. 

Please take a look and provide feedback. Once you are happy I will propose this to blink-dev.

Thanks

Majid

[Kentaro Hara]

The migration plan looks great to me!

--

You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAB8RdXt8gC5prXanxBkAXs0ON741uWbbkQcDYY-OZYeukjy%3DYA%40mail.gmail.com.