Shouldn't we just have TimeTicks::ToMicroseconds** and TimeTicks::FromMicroseconds then that does what you are suggestion?

codesearch shows 700+ call sites for these methods. So it is common enough pattern to be worth capturing in methods. 


** Or perhaps InMicroseconds to be consistent with TimeDelta naming. conventions?

No, we shouldn't add a To/FromMicroseconds() for TimeTicks (or base::Time) because these values represent a specific point-in-time. In other words, TimeTicks::ToMicroseconds() has no semantic meaning. It only means something if client code is forced to make an assumption about an "origin point" on the timeline. We've gone to great lengths to prevent client code from having to make such assumptions (granted, with more work to do here). By alleviating the client code from making that assumption, we avoid bugs, enable compile-time checking of time math, and are free to change the internal representation of time values as needs arise. For example, what if one day we decide we want nanosecond resolution for TimeTicks?

Furthermore, in the case of TimeTicks, note that there is no static origin point: Internally, the origin can change for each run of the application, or can be "zero corresponds kernel boot time." Thus, it is totally meaningless to serialize a TimeTicks for the purposes of persisting time values that outlive the application or are sent outside of the application. Having To/FromInternalValue() allows developers to too-easily do things that violate the design, with usually unexpected consequences.

Hopefully, this all better explains the dangers of having these To/FromInternalValue() methods and why I'm trying to eliminate them.

I agree with the written sentiment and goal here but I don't see how the suggested route is going to get there. 

The original proposal was to do the following transformations:  

|FromInternalValue(value)| with |base::TimeTicks() + 
base::TimeDelta::FromMicroseconds(value);|


> It only means something if client code is forced to make an assumption about an "origin point" on the timeline. We've gone to great lengths to prevent client code from having to make such assumptions (granted, with more work to do here). By alleviating the client code from making that assumption, we avoid bugs, enable compile-time checking of time math, and are free to change the internal representation of time values as needs arise.

The proposed substitution is still making an assumption on a point of origin and a resolution for the value. It seems to me that actually defeats the purpose. 

Proposed InSeconds/ToSeconds methods in #2 do have the same issue but it is at least captured in a single place. But I agree that this can encourage clients to abuse these methods the same way they do To/FromInternalValue. So perhaps best to avoid as well.

I think what we want is an "opaque" time value that can *only* be serialized/deserialized but cannot be used to do any math/conversion. I think this was the intention of To/FromInternalValue() but returning the internal a numeral us_ allows the clients to abuse it.

Here are few other options:
 - Just define ParamTraits<base::Tim*>. This addresses the explicit serialization example you used.
 - ToInternalValue should scramble us_ so that client code cannot use it safely for math. You can only do this in debug builds if performance is a concern. ;)

For the rest of cases where the client is consciously making an explicit assumption on origin resolution then I agree it is a good to make the proposed substitution.
 

In another word:

- For actual (de)serialization logic, the original substitution is worse because it adds assumption of "origin point" and "resolution". I suggest other ways to provide opaque (de)serialization to time types.

- For cases when the client is making explicit assumptions about origin and resolution then just force them to use TimeDelta and To/In methods to make this clear instead of abusing To/FromInternalValue methods.

Sorry for late response. I've been off on other things, and only recently a CL was presented that led me to start this discussion again. ;-)

majidvp: Good points in c#3 and c#4. We already have ParamTraits for the time types, with both IPC and Mojo covered. I'm trying to think of any other reason people would want to do serialization on these time values. Meaning, if we were to eliminate all improper uses of To/FromInternalValue(), what else is left that's a legitimate serialization use case?

If it turns out there is still legitimate need for general-use serialization methods, I like your idea about scrambling the bit order to discourage abuse. It shouldn't be a performance concern since we could do something really cheap like XOR with a magic value or swap endianness (little-->big, or big-->little).

majidvp: It's fix-it week for my team, and I think now's a great time for me to work on this issue. We seem to be mostly in-agreement with approach here, with some lingering concerns.

So, to start, I'm going to see how many uses of To/FromInternalValue() I can replace with more-appropriate alternatives (i.e., where the client code doesn't have to make assumptions about the origin point). Then, we can take a look at any left-over call-points--hopefully very few in number--and be in a better position to consider how to handle those. SGTY?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8ca6e85e110110372381fcb9840d93721ec650eb

commit 8ca6e85e110110372381fcb9840d93721ec650eb
Author: Yuri Wiitala <miu@chromium.org>
Date: Wed Jul 19 02:31:56 2017

Remove uses of To/FromInternalValue() in ash/...

Replaces non-serialization uses of To/FromInternalValue() with
reasonable alternatives.

Bug: 634507
Change-Id: I2b4378643b00d9e287cd1e3d4bd0f403ffb1df40
Reviewed-on: https://chromium-review.googlesource.com/575259
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487724}
[modify] https://crrev.com/8ca6e85e110110372381fcb9840d93721ec650eb/ash/system/power/battery_notification.cc
[modify] https://crrev.com/8ca6e85e110110372381fcb9840d93721ec650eb/ash/system/power/power_status_view.cc
[modify] https://crrev.com/8ca6e85e110110372381fcb9840d93721ec650eb/ash/touch/touch_uma.cc
[modify] https://crrev.com/8ca6e85e110110372381fcb9840d93721ec650eb/ash/wm/lock_state_controller_unittest.cc

So, I've converted src/ash/*, and am about to put up a code review for src/cc/*. So far, the only "non-blatant abusive" uses of To/FromInternal() can be categorized in one of three buckets:

1. Used as an "identifier" or a "microsecond offset" in TRACE_EVENT(...args...). In the case, of "identifier" values, the semantics of the time values are being thrown away, since the TimeTicks is just being used as some unique number value. In the case of "microsecond offset," the origin point does not matter, just the relative difference between each of the trace events. So, IMHO, it would be okay to convert these use cases to (value - TimeTicks()).InMicroseconds().

2. Fake time values for unit tests: Some unit tests run their own mock clock, and hard-code expected time values in the code. Again, this feels like it would be okay to convert using TimeTicks() + TimeDelta::FromMicroseconds(N). In a couple of the src/cc unit tests, the conversion happened a lot, making use of the expression rather verbose. However, I was able to simply write a helper function to make things clean and simple:

  base::TimeTicks FakeTimeTicks(int64_t micros) {
    return base::TimeTicks() + base::TimeDelta::FromMicroseconds(micros);
  }

  ...

  EXPECT_EQ(FakeTimeTicks(19997), result_value);

3. Usage with base::Value: Apparently, there are a few code points that convert the time values into int64's and then into string form and back (because there are no int64's in base::Value). There's already helper code defined in base/value_conversions.cc, so again, IMHO it's fine for value_conversions.cc to use something like base::Int64ToString((value - TimeTicks()).InMicroseconds()), hiding the details of "serialization" from the calling code.

miu@ your plan in #7 sgtm.

Regarding #9 case 3, it seems to me that the case is where we can/should employ the idea to scrambling the bits to create an opaque serialized form and discourage abuse. wdyt?

The example you mentioned in base/value_conversions.cc is related to TimeDelta which can be safely turned into microseconds.

Ah, yes, it is just for TimeDelta. :) However, I did come across some code that did something similar for all time classes (in media/base/video_frame_metadata.cc). I'm ashamed to say I wrote that code myself a few years back. Heh. So, I think it would be good to consolidate those kinds of conversions in base/value_conversions.cc for everyone's convenience.

I like and will use your "scrambling the bits to create an opaque serialized form" idea for value_conversions.cc, IPC ParamTraits, and Mojo. I'm going to save those for the final CLs, just to make sure there aren't any other use cases we need to consider before implementing this solution.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2f8de5cd8a28b23388b3d340f225e2c7a8180d2f

commit 2f8de5cd8a28b23388b3d340f225e2c7a8180d2f
Author: Yuri Wiitala <miu@chromium.org>
Date: Fri Jul 21 00:11:06 2017

Fixit: Add PRESUBMIT check for TimeXXX::To/FromInternalValue().

The fix-it to remove the uses of To/FromInternalValue() is underway.
This change adds a PRESUBMIT warning (BANNED_CPP_FUNCTIONS) and
base/time/time.h header comments, to avoid having new code use these
deprecated functions.

The intention is for this change to be short-lived, and removed once
the deprecated functions are removed from the time classes.

One fault in the check: Since the script can only do regexp matching,
it's impossible to check that the ToInternalValue() method is being
called on one of the TimeXXX types. However, there are relatively few
non-TimeXXX types that have a ToInternalValue() method. Therefore, it
should be rare for the script to trigger a warning erroneously (and
this would be easy to bypass, anyway).

Bug: 634507
Change-Id: I48d75435703771107c87935cb3ea90a5a956989e
Reviewed-on: https://chromium-review.googlesource.com/577980
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488505}
[modify] https://crrev.com/2f8de5cd8a28b23388b3d340f225e2c7a8180d2f/PRESUBMIT.py
[modify] https://crrev.com/2f8de5cd8a28b23388b3d340f225e2c7a8180d2f/base/time/time.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d061176d905278c4090dd8671e61a2a7cfde6e63

commit d061176d905278c4090dd8671e61a2a7cfde6e63
Author: Yuri Wiitala <miu@chromium.org>
Date: Sat Jul 22 02:33:21 2017

Fixit: Remove uses of To/FromInternalValue() in cc/...

Replaces non-serialization uses of To/FromInternalValue() with
reasonable alternatives. No behavior changes intended.

Also includes a few minor clean-ups, such as deleting the
cc::TimeUtil::Scale() and Mod() methods in favor of built-in
operator overloads.

Testing: Ran cc_unittests and a local chrome browser build to
confirm correctness.

Bug: 634507
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I1e9360d707aacad3a88474058f0d0c96d57f0d80
Reviewed-on: https://chromium-review.googlesource.com/578691
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488841}
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/base/time/time.h
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/animation/animation.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/animation/animation_delegate.h
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/animation/animation_events.h
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/animation/animation_player.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/animation/animation_unittest.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/animation/element_animations_unittest.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/animation/keyframed_animation_curve.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/animation/scroll_offset_animation_curve_unittest.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/base/delayed_unique_notifier_unittest.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/base/lap_timer.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/base/time_util.h
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/scheduler/begin_frame_source_unittest.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/scheduler/begin_frame_tracker.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/scheduler/delay_based_time_source.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/scheduler/scheduler.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/test/fake_layer_tree_host_impl.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/test/ordered_simple_task_runner.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/test/ordered_simple_task_runner.h
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/test/ordered_simple_task_runner_unittest.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/trees/layer_tree_host_unittest_animation.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/cc/trees/layer_tree_host_unittest_context.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/components/viz/common/frame_sinks/begin_frame_args.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/components/viz/common/frame_sinks/begin_frame_args_unittest.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/components/viz/test/begin_frame_args_test.cc
[modify] https://crrev.com/d061176d905278c4090dd8671e61a2a7cfde6e63/ui/compositor/transform_animation_curve_adapter_unittest.cc

So, just making sure: for serialization purposes is it still OK to use FromInternalValue() and ToInternalValue()? I'm converting time values to store in a SQLite database and was relying on those.

carlosk: Don't use To/FromInternalValue(). First, think about your data model: Do you really want to persist a calendar time value as the number of microseconds since the Windows epoch (year 1601)?

I would suggest considering one of the common formats described in section 2.2 of the SQLite data type documentation (https://sqlite.org/datatype3.html) instead. That way, SQLite's date and time functions can be used in queries.

If you're still set on using Windows Epoch time, here's an example: https://chromium-review.googlesource.com/c/583868/4/ui/app_list/search/history_data_store.cc

Note, however, that there is very little code persisting values this way.

On my team, we're being hit by this. We used To/FromInternalValue() to serialize a time in our database (examples https://cs.chromium.org/search/?q=file:components/ntp+toInternalvalue&type=cs). We actually don't care about the internals of the format, as long as it stays *stable*.

Instead of removing the method, what about deprecating the old one and adding new ones for serializing/deserializing?

Otherwise, we now have to deal with implementation details which actually got hidden by the Time library (why would we have chosen windows time instead).

Concerning the *scrambling* idea. This sounds like the wrong approach to me. We should make it easier to do the right thing. Serialization should be predictable and stable. If misuse happened, it's much more likely that there's an actual feature gap, that could be addressed by better libraries (or if those already exist, with comments pointing to those).

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/73904e341bdcae5499cde50bdcdebf2f6c8b0b00

commit 73904e341bdcae5499cde50bdcdebf2f6c8b0b00
Author: Yuri Wiitala <miu@chromium.org>
Date: Thu Jul 27 00:50:20 2017

Fixit: Remove uses of To/FromInternalValue() in ui/...

Replaces non-serialization uses of To/FromInternalValue() with
reasonable alternatives. No behavior changes intended. In order to
accomplish this, a number of less-than-trivial changes changes were made
to adapt existing code:

1. Fixed display_link_mac.cc to use Mach time conversion function in
base::TimeTicks and to perform safer math around the interval
calculation by using base::CheckNumeric<int64_t>.

2. Added warning comments and extra checks around code in
window_android.cc and ozone/.../drm_device.cc where external modules are
providing time values that are supposed to be from the same clock as
base::TimeTicks.

3. Added base::TimeDelta as a supported Aura ClassProperty to avoid
burdening client code with the TimeDelta memory storage details.

4. Update ui.mojom.Event interface to let Mojo handle the TimeTicks type
rather than manually convert to/from int64_t in struct traits.

5. Fixed time plumbing in ozone/platform/drm/gpu/... to use
base::TimeTicks in Chromium-side code instead of tv_sec + tv_usec values
everywhere.

Bug: 634507
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ie93037c636b3c196344c91d3cc489fed9b50df1d
Reviewed-on: https://chromium-review.googlesource.com/583868
Reviewed-by: Michael Spang <spang@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Reviewed-by: ccameron chromium <ccameron@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489820}
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/accelerated_widget_mac/display_link_mac.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/android/window_android.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/app_list/search/history_data_store.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/aura/client/aura_constants.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/aura/mus/user_activity_forwarder_unittest.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/base/class_property.h
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/base/user_activity/user_activity_detector_unittest.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/events/android/scroller.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/events/blink/input_scroll_elasticity_controller_unittest.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/events/event.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/events/gesture_detection/motion_event_buffer_unittest.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/events/gesture_detection/motion_event_generic.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/events/mojo/BUILD.gn
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/events/mojo/event.mojom
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/events/mojo/event_struct_traits.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/events/mojo/event_struct_traits.h
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/events/mojo/struct_traits_unittest.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/events/ozone/evdev/touch_event_converter_evdev_unittest.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/events/ozone/evdev/touch_filter/horizontally_aligned_touch_noise_filter.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/events/test/event_generator.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/events/test/motion_event_test_utils.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/events/x/events_x_unittest.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/gfx/animation/tween.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/gfx/paint_vector_icon.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/gl/sync_control_vsync_provider.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/gl/vsync_provider_win.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/latency/latency_info.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/latency/latency_info_unittest.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/ozone/platform/drm/gpu/crtc_controller.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/ozone/platform/drm/gpu/crtc_controller.h
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/ozone/platform/drm/gpu/drm_device.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/ozone/platform/drm/gpu/drm_device.h
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/ozone/platform/drm/gpu/drm_window.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/ozone/platform/drm/gpu/hardware_display_controller.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/ozone/platform/drm/gpu/hardware_display_controller.h
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/ozone/platform/drm/gpu/hardware_display_plane_manager_atomic.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/ozone/platform/drm/gpu/mock_drm_device.cc
[modify] https://crrev.com/73904e341bdcae5499cde50bdcdebf2f6c8b0b00/ui/wm/core/window_animations.cc

tschumann: One purpose of this change is to make it much harder for developers to do the wrong thing, which isn't quite the same as "easy to do the right way" (more on this below). It sounds like your team has written code to persist base::Time values using To/FromInternalValue(), and that's actually perfectly reasonable: The intended use case! :-)

Upon re-reading my c#16, I suppose I was making it sound like I felt serializing was a bad use of To/FromInternalValue(). Instead, what I was trying to do was have carlosk@ think more about their storage data model. They mentioned using SQLite as a storage engine, which is why I was questioning whether they really *wanted* to use Windows epoch time: For instance, that choice would make it much more cumbersome to use SQLite's built-in date/time functions.

I respect you may feel you are being punished by our elimination of these functions, but that's not our intention. Our decision is based on a simple fact: For every one time To/FromInternalValue() is used correctly, there are quite literally about 50-100 other cases that abuse it. Lots of code has been written is a dangerous way, and that code is difficult to maintain/modify while ensuring correctness (e.g., time math and semantics). There are several cases where this has directly led to some very hard-to-resolve bugs. I have been involved in diagnosing several, myself. For example, one time, someone was using int's for millisecond-resolution time, then later changed that to a microsecond timebase but forgot to rename a variable in another class: This led a future code change, in that other class, to erroneously interpret the usec as millis, with unfortunate user-facing consequences.

So, part of this project is also to lead us to a point where we can figure out how to make things "easy to do the right way." I agree with you when you say, "there may be feature gap." There are a plethora of time conversion functions in base/time/time.h, and there is a rather alarming lack of consistency in API and behavior (and failed prior attempts to fix those things!). The initial goal (see c#5) is to try to eliminate all obvious misuses of To/FromInternalValue(), and see what's left. At that point, we will be in a good position to fix our API to make the remaining use cases simple, self-describing, and otherwise "easy for developers to do things the right way."

My problem is that you are deprecating functionality and don't give a clear path forward for the use case the function was originally meant for.

If there was a replacement in place before, users wouldn't have to do workarounds like those and duplicate implementation details (https://chromium-review.googlesource.com/c/586601/3/components/ntp_snippets/time_serialization.cc).

So, if you plan to support this functionality going forward, can we find a proper place for it now?

Hi I'm on the same team as carlosk@ above.

I have a question while using the serialized value in database: say the value stored in DB is |ms| (which was generated by ToInternalValue()), are the following two base::Time equivalent?
base::Time::FromInternalValue(ms)
and
base::Time() + base::TimeDelta::FromMicroseconds(ms)

I read some of the implementation code and I tend to believe they're the same, but not sure. If not, it seems the values in the DB needs to be 'upgraded' and the new correct value to be stored in DB should be
(base::Time() + base::TimeDelta::FromMicroseconds(ms)).since_origin().InMicroseconds()

Please correct me if i'm wrong. Thanks!

I'm currently implementing NTLMv2 and I need to serialize/deserialize this value 
 too as part of the protocol.

Is it accurate that this is recommending to write a wrapper function that essentially recreates the raw value by creating a time delta from 0?

Re #c21: Yes, base::Time::FromInternalValue(raw_value) should be equivalent to base::Time() + base::TimeDelta::FromMicroseconds(raw_value). So, you shouldn't need to upgrade the DB values. :)

Re #c22: Because NTLMv2 is tied to the Windows platform, it's likely you would be wanting to convert to/from Windows time types, to interface with Windows APIs, right? Or, is this a Chromium implementation of NTSMv2 meant for cross-platform use? (I can't really give a recommendation without knowing more about what it is you're trying to do. Feel free to e-mail details and we can discuss.)

FWIW, there are likely other instances of handing Windows time type conversion in Chromium code (you'd just need to `git grep` for examples probably). Though, please scrutinize them for your particular use case before doing things the same way. Also, the header comments at the top of src/base/time/time.h provide some guidance about Time vs TimeTicks, as I can envision you possibly needing both for different aspects of your project: https://cs.chromium.org/chromium/src/base/time/time.h?rcl=e99ddc1455117eba392cc9cd354a8f49fe3d6838&l=34

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c8f9063d724f60d8c75fbb4a3ff147587f6845d

commit 8c8f9063d724f60d8c75fbb4a3ff147587f6845d
Author: Sebastien Marchand <sebmarchand@chromium.org>
Date: Mon Aug 21 20:27:12 2017

Stop using Time::[To|From]InternalValue in DailyEventTest

These functions have been deprecated and there's some ongoing effort to
remove them (see crbug.com/634507)

I'm using the DailyEvent class in the TabStatsTracker and in the
unittests I need to modify the pref registry value used by my DailyEvent
instance (as we do in the DailyEventTest). I could simply not update
this and use "foo.since_origin().InMicroseconds()" in my new code
(rather than "foo.ToInternalValue()") but I thought that it'd be better
to update this class while I was here, this way my unittests will be
similar to the DailyEvent ones. 


In this CL I replace:
- "base::Time().ToInternalValue()" by 0, as it's what it is in practice.
- "base::Time::FromInternalValue(foo)" by "base::Time() + base::TimeDelta::FromMicroseconds(foo)"
- "foo.ToInternalValue()" by "foo.since_origin().InMicroseconds()"

Bug: 634507
Change-Id: Ia4496fba6fbc052bfd7fb6d3334941142726038e
Reviewed-on: https://chromium-review.googlesource.com/619695
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496048}
[modify] https://crrev.com/8c8f9063d724f60d8c75fbb4a3ff147587f6845d/components/metrics/daily_event.cc
[modify] https://crrev.com/8c8f9063d724f60d8c75fbb4a3ff147587f6845d/components/metrics/daily_event_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/19b23146ae9ba0f88eaf1184b17cb8a379f4a8ee

commit 19b23146ae9ba0f88eaf1184b17cb8a379f4a8ee
Author: Xi Cheng <chengx@chromium.org>
Date: Fri Sep 15 16:13:11 2017

Retire FromInternvalValue() in TopSitesDatabase

This function has been deprecated and there's some ongoing effort to
remove them. See attached bug. This CL retires it in TopSitesDatabase.

Bug: 634507
Change-Id: I99b0d21f5f03b0e904b9987f5f2fc6fa749f15f8
Reviewed-on: https://chromium-review.googlesource.com/668145
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Xi Cheng <chengx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502271}
[modify] https://crrev.com/19b23146ae9ba0f88eaf1184b17cb8a379f4a8ee/components/history/core/browser/top_sites_database.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/221832c882c273cbd7ef3cdd29a7238defed6476

commit 221832c882c273cbd7ef3cdd29a7238defed6476
Author: Xi Cheng <chengx@chromium.org>
Date: Fri Sep 15 21:02:39 2017

Retire ToInternalValue() in TopSitesDatabase

Bug: 634507
Change-Id: Iacc75b06ea2089eca45b798985bff749fc6d17df
Reviewed-on: https://chromium-review.googlesource.com/668753
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Xi Cheng <chengx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502367}
[modify] https://crrev.com/221832c882c273cbd7ef3cdd29a7238defed6476/components/history/core/browser/top_sites_database.cc

Just a data point.
When people discover that ToInternalValue is deprecated, they tend to use Unix epoch serialization, which may lead to subtle bugs:
1) it is inconsistent with ToInternalValue.
2) Time::is_null does not work as expected.

I came across this bug when trying to use eglGetFrameTimestampsANDROID, which returns the raw nanosecond precision value of the MONOTONIC clock source for compositor and display events on Android.

See: https://android.googlesource.com/platform/frameworks/native/+/refs/heads/master/opengl/specs/EGL_ANDROID_get_frame_timestamps.txt

I wanted to convert those to TimeTicks by using TimeTicks::FromInternalValue(ns / 1000).

Instead, should I add something similar to Windows' TimeTicks::FromQPCValue for Android's MONOTONIC clock source? Maybe TimeTicks::FromTimespecNanos()?

brianderson (re #c29): Sounds fine to add a converter similar to FromQPCValue(). Is the clock the same as the POSIX monotonic clock or is this a different system clock? I'm wondering if this converter should be for all POSIX platforms (i.e., to and from "timespec" values), rather than just an Android-specific one. WDYT?

vitaliii (re #c28): Do you mean they are calling TimeTicks::UnixEpoch(), or something else? (FWIW, TimeTicks::UnixEpoch() was something of a mistake because it can be a different value between runs of the application, or even over long periods of time within the same application! See bug 590845.)

FWIW, I've been swamped wrapping up the projects that more-directly contribute to my paycheck. ;-) But, I do plan to re-start the time library clean-ups (this and bug 590845 and others) in the mid-Q4 time frame.

miu (re #c31): It was base::Time::UnixEpoch().

@miu: I just ran into the presubmit warning associated with this issue (ow!  :-}) while doing an unrelated refactor.  To my untutored eye, this is one of the cases for which {From,To}InternalValue() is ok, i.e. it's for serialization/deserialization.  What should I do, just override the presubmit warnings?

I'd be happy to do a transformation on the code if there is a clean, backwards compatible transformation to be done, but if the transformation actually changes the on-disk format, that'll require revving and upgrading a DB in a section of code in which I have no expertise, which seems like a poor choice :-}.

Context is https://chromium-review.googlesource.com/c/chromium/src/+/777954 PS 16, chrome/browser/android/cookies/cookies_fetcher.cc, the last block of which triggers the presubmit warning.  

(This is effectively c#20 from a code non-OWNER in CL form :-}.)

rdsmith (re #33): Please see comments 16 and 23; and then if you don't want to change your data model (totally understandable), you can do it like this: https://chromium-review.googlesource.com/c/583868/4/ui/app_list/search/history_data_store.cc

Thank you!  That makes a lot of sense.  Unfortunately, I simply force landed (I was touching the code rather than changing it).  If I'm back in that area in the future (not impossible) I'll update to the base::Time() + FroMicroseconds() idiom, else I'll leave it to the code OWNERS/you.  But I'm fully armed if I run into this again.

c#34 TL;DR for the next person: If you're persisting, consider using a different SQLite compatible persistence format (see c#16).  If you want to keep the on-disk format the same, use base::Time() + base::TimeDelta::FromMicroseconds(<persisted value>) & its inverse, and that'll keep the on-disk format the same.  But be aware that's an unusual choice for our codebase.

miu: I've got the FromInternalValue() warning on my fresh upload at: https://chromium-review.googlesource.com/c/chromium/src/+/844775

The use is in chrome/browser/android/metrics/uma_utils.cc

Made it to pass the result of clock_gettime(CLOCK_MONOTONIC) from the Java side (the value was obtained when the native library is not initialized yet) into the native side, which can be done only via a raw int64_t (yeah, no unsigned types in Java). On Android base::TimeTicks::Now() also uses clock_gettime(CLOCK_MONOTONIC) as internal value AFAICT, hence we should not lose precision.

I realize this does not look pretty, please recommend a better way if there is one. If there is a way to suppress the warning for legitimate uses, please advise as well.

@miu: Having read through this entire bug history, I still do not see why we would prefer the pattern of "base::Time() + base::TimeDelta::FromMicroseconds(x)" over "base::Time::FromInternalValue(x)".  The prior is more verbose; but since it's functionally equivalent, it's just as abusable.  IMO, CLs like the one in comment #26 reduce the clarity of our codebase.

I'd like to propose that we provide a clearly named convenience function for opaque serialization/deserialization of base::Time values – something identical in functionality to the other two options, but named differently, to allow spot-checking of existing uses of To/FromInternalValue() to continue.  How about naming these functions Serialize() and Deserialize()?  WDYT?

proposal from dskiba@ for Android which looks clean for the case I mentioned in #36:

"""
TimeTicks has several platform-specific "From" methods (e.g. FromQPCValue), we could add Android-specific FromUptimeMillis(). That way we avoid using FromInternalValue(), avoid magic *1000 thing, and overall better state our intent.
"""

Any objections?

reply to c36, c37 and c38: The original statement of the problem was that there were bugs being created due to abuse of From/ToInternalValue(), and this seems to mainly be an issue with base::TimeTicks. However, there seems to be a valid-use pattern for base::Time (not TimeTicks).

I can see how base::Time() + base::TimeDelta(...) is cumbersome. I think the issues surrounding base::Time and base::TimeTicks are different, however; so let me address those separately:

base::Time: for (de)serializing, it would be good if we could improve code readability there. At the same I feel it's important for folks to be considering their data model when they do need to (de)serialize: Meaning, it's important to be aware that the value means "microseconds since Windows epoch." (And, it's also important to be able to change the internal representation of base::Time without breaking existing code that persists such values.) So, WDYT if we replace base::Time's From/ToInternalValue() with something like:

  base::TimeDelta SinceWindowsEpoch() const;
  static constexpr base::Time FromWindowsEpoch(base::TimeDelta);

That way, serialization code becomes rather easy on-the-eyes and has a very clear statement of intent. For example:

  base::Time last_updated = ...;
  SaveToDatabase(last_updated.SinceWindowsEpoch().InMicroseconds());

...and deserialization:

  base::Time last_updated = base::Time::FromWindowsEpoch(
      TimeDelta::FromMicroseconds(LoadFromDatabase()));

base::TimeTicks: Generally, the only code that should be (de)serializing these values is for lower-level platform integration. We currently have platform-specific FromXYZ() methods in TimeTicks for Fuchsia, Windows, and MacOS. I'm not opposed to adding a pair for the Linux monotonic clock (covers Android as well). Would this satisfy your use cases?
 If so, feel free to put up a code patch for review. :)

Thanks for coming back to this conversation ;-)

Updating base::Time's documentation to clearly outline best practices for serialization / deserialization would be useful. Reading through the header, there seems to be some inconsistency:  At the very top (https://cs.chromium.org/chromium/src/base/time/time.h?rcl=36e99a557191d54179c546cc9fdcd323508a2365&l=5) it's prominently stated that the class's internal representation is Windows epoch. However, it seems that the class itself wants to abstract from this.
Maybe this should be changed to say it supports Windows epoch. When serializing, users of the class should explicitly chose a representation and use the ToXXXEpoch() / FromXXXEpoch() to do the conversions (and not rely on internal representations of this class).

miu: thank you for responses. My case is only about base::TimeTicks, so my potential cleanup would ignore the base::Time.

I am not sure sharing the name for Linux and Android would be appropriate. On Android if the name is FromUptimeMillis() then it straight pointing at SystemClock.uptimeMillis(), hence less error-prone.

On Linux it would better sound like FromClockMonotonicSomething() .. which is not ideal for Android. Since there is no point in persisting TimeTicks to disk, I am wondering if there exists a usecase on Linux. Perhaps some libraries we use would spit results of CLOCK_MONOTONIC? I'd prefer to postpone introducing the new API for Linux until it is needed.

If you are OK with Android-only patch, I can send it out ~ tomorrow.

I've uploaded a CL along the lines of comment 39 at [ https://chromium-review.googlesource.com/#/c/chromium/src/+/861345 ].  Please feel free to chime in there if you have any thoughts on it – thanks!

re c41: Sure, feel free. Please try to make it look consistent with what's already been done there for Windows QPC, Mac mach_absolute_time, and Fuchsia zx_time_t.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5476206f4058aedb20c08e20d4470f00e2b0348e

commit 5476206f4058aedb20c08e20d4470f00e2b0348e
Author: Ilya Sherman <isherman@chromium.org>
Date: Fri Jan 12 22:58:24 2018

Add convenience methods for serialization and deserialization of Time values.

These are preferred over To/FromInternalValue(), as they are not inherently tied
to the implementation details of the base::Time class.

BUG=634507
R=miu@chromium.org

Change-Id: I4f245e8a68daeb02a751228c18a21b01c994abf5
Reviewed-on: https://chromium-review.googlesource.com/861345
Commit-Queue: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529098}
[modify] https://crrev.com/5476206f4058aedb20c08e20d4470f00e2b0348e/base/time/time.cc
[modify] https://crrev.com/5476206f4058aedb20c08e20d4470f00e2b0348e/base/time/time.h
[modify] https://crrev.com/5476206f4058aedb20c08e20d4470f00e2b0348e/base/time/time_unittest.cc

For anyone serializing base::Time values to prefs, I just landed https://chromium-review.googlesource.com/c/chromium/src/+/865569, which adds convenience methods SetTime() and GetTime() on the PrefService class.  Please use these going forward.

Moreover, the implementations are (at the time of this writing) logically identical to base::Time::ToInternalValue() and base::Time::FromInternalValue(), so it's appropriate to migrate any code that currently uses those time-serialization methods to this new API.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/57c01825f409616c3112d03679dde0f11f2cb45b

commit 57c01825f409616c3112d03679dde0f11f2cb45b
Author: Egor Pasko <pasko@chromium.org>
Date: Tue Jan 23 11:59:16 2018

Add base::TimeTicks::FromUptimeMillis()

This allows creating a base::TimeTicks from SystemClock.uptimeMillis().
Android-only.

Also add a use this new method once instead of FromInternalValue(), which is
discouraged.

Bug: 634507,  797762 
Change-Id: I8b5e3f4d0da1919f21c1f96918ca36a4e434ae0d
Reviewed-on: https://chromium-review.googlesource.com/865314
Commit-Queue: Egor Pasko <pasko@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531208}
[modify] https://crrev.com/57c01825f409616c3112d03679dde0f11f2cb45b/base/BUILD.gn
[modify] https://crrev.com/57c01825f409616c3112d03679dde0f11f2cb45b/base/time/time.h
[add] https://crrev.com/57c01825f409616c3112d03679dde0f11f2cb45b/base/time/time_android.cc
[modify] https://crrev.com/57c01825f409616c3112d03679dde0f11f2cb45b/base/time/time_unittest.cc
[modify] https://crrev.com/57c01825f409616c3112d03679dde0f11f2cb45b/chrome/browser/android/metrics/uma_utils.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/196496901c07bbbb05e4cee6675f01bef19b6c22

commit 196496901c07bbbb05e4cee6675f01bef19b6c22
Author: Jan Krcal <jkrcal@chromium.org>
Date: Mon Feb 26 16:49:23 2018

[Thumbnails DB] Remove uses of deprecated Time::From/ToInternalValue()

The CL replaces the calls to Time::From/ToInternalValue() by the more
explicit Time::From/ToDeltaSinceWindowsEpoch(). This change is backwards
compatible.

Bug: 634507
Change-Id: Ic5f22baf4c2216eec8080386eefa2e75a8ca5378
Reviewed-on: https://chromium-review.googlesource.com/913609
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539162}
[modify] https://crrev.com/196496901c07bbbb05e4cee6675f01bef19b6c22/components/history/core/browser/thumbnail_database.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/28e6a8be03e651449c8eec48afb50e50a52858f1

commit 28e6a8be03e651449c8eec48afb50e50a52858f1
Author: Mathieu Guay-Paquet <mguaypaq@chromium.org>
Date: Wed Aug 08 19:22:25 2018

Remove deprecated uses of Time::FromInternalValue()

The more explicit Time::FromDeltaSinceWindowsEpoch() is used instead,
and produces the same value given the same int64_t.

The registry keys consumed by this code are written by code in
chrome/chrome_cleaner/logging/registry_logger.cc (not yet moved there
from the internal repo), which already uses the corresponding
Time::ToDeltaSinceWindowsEpoch().

Bug: 634507
Change-Id: I75348bf8e55a0cc3d93412563e5527ddb04ce390
Reviewed-on: https://chromium-review.googlesource.com/1156861
Commit-Queue: Mathieu Guay-Paquet <mguaypaq@chromium.org>
Reviewed-by: Joe Mason <joenotcharles@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581657}
[modify] https://crrev.com/28e6a8be03e651449c8eec48afb50e50a52858f1/chrome/browser/component_updater/sw_reporter_installer_win.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/739f73729badfb5249dbea2819b99cdde1ace0b8

commit 739f73729badfb5249dbea2819b99cdde1ace0b8
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Fri Aug 24 12:25:19 2018

Use PrefService support of base::Time

base::Time::{To,From}InternalValue is deprecated. Use the
new (https://crrev.com/c/865569) methods of PrefService to
register and manipulation the preference as base::Time.

Bug: 634507
Change-Id: I4093149e776d6924d7aaa49a617d4b01c2f3d862
Reviewed-on: https://chromium-review.googlesource.com/1188308
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585809}
[modify] https://crrev.com/739f73729badfb5249dbea2819b99cdde1ace0b8/components/signin/core/browser/account_fetcher_service.cc
[modify] https://crrev.com/739f73729badfb5249dbea2819b99cdde1ace0b8/components/signin/core/browser/account_tracker_service_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da4aabbfc7beda8b1470d38509aa68fdacd7f2d4

commit da4aabbfc7beda8b1470d38509aa68fdacd7f2d4
Author: Carlos Knippschild <carlosk@chromium.org>
Date: Fri Oct 12 23:56:28 2018

Updates usage of deprecated Time methods in offline pages code

With the somewhat recent deprecation of (To|From)InternalValue methods
from base::Time classes, the offline pages code base wasn't yet updated
to comply with that. This change fixes that.

Bug: 634507
Change-Id: I0fd5d41c492e2f103b4d21cb38eedbd5b65b1bcf
Reviewed-on: https://chromium-review.googlesource.com/c/1273994
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Commit-Queue: Carlos Knippschild <carlosk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599427}
[modify] https://crrev.com/da4aabbfc7beda8b1470d38509aa68fdacd7f2d4/chrome/browser/offline_pages/prefetch/offline_metrics_collector_impl.cc
[modify] https://crrev.com/da4aabbfc7beda8b1470d38509aa68fdacd7f2d4/chrome/browser/offline_pages/prefetch/offline_metrics_collector_impl_unittest.cc
[modify] https://crrev.com/da4aabbfc7beda8b1470d38509aa68fdacd7f2d4/components/offline_pages/core/offline_store_utils.cc
[modify] https://crrev.com/da4aabbfc7beda8b1470d38509aa68fdacd7f2d4/components/offline_pages/core/prefetch/mark_operation_done_task.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e5e23f0b861e0cef2c646d352abd45d7e90662e0

commit e5e23f0b861e0cef2c646d352abd45d7e90662e0
Author: Gabriel Marin <gmx@chromium.org>
Date: Mon Nov 19 21:03:29 2018

PerfProvider: Remove conversion to TimeDeltaInternalType

Store base::TimeDeltas directly instead of converting to and from
TimeDeltaInternalType. This removes the use of To/FromInternalValue,
which are deprecated. No logic changes.

BUG=634507,b:116527691
TEST=Unit tests pass.

Change-Id: I7fe53e4c418651ff88da2a123c1ab943a590679b
Reviewed-on: https://chromium-review.googlesource.com/c/1340604
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Gabriel Marin <gmx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609432}
[modify] https://crrev.com/e5e23f0b861e0cef2c646d352abd45d7e90662e0/chrome/browser/metrics/perf/perf_provider_chromeos.cc
[modify] https://crrev.com/e5e23f0b861e0cef2c646d352abd45d7e90662e0/chrome/browser/metrics/perf/perf_provider_chromeos.h

Would it be acceptable to add the new "base::TimeTicks::FromUptimeMillis()" to the Chrome OS build? It's currently only in the Android build, but on Chrome OS, we want to de-serialize a time value coming from the Android (ARC++)