Base: Make TimeDelta constructors deal with overflow.

base/time/time.h

Index: base/time/time.h
diff --git a/base/time/time.h b/base/time/time.h
index a02fbeb01ffb7e82761d524fd8c8bcacb9f618eb..b972bd723257df73e0407cbfe30b7fff84cbcd9a 100644
--- a/base/time/time.h
+++ b/base/time/time.h
@@ -264,6 +264,9 @@ class BASE_EXPORT TimeDelta {
   explicit TimeDelta(int64 delta_us) : delta_(delta_us) {
   }
 
+  // Private method to build a delta from a double.
+  static TimeDelta FromDouble(double value);
+
   // Delta in microseconds.
   int64 delta_;
 };
@@ -597,7 +600,7 @@ class BASE_EXPORT Time : public time_internal::TimeBase<Time> {
 
 // static
 inline TimeDelta TimeDelta::FromDays(int days) {
-  // Preserve max to prevent overflow.
+  // Special-case max.

[Nico, 2015/07/17 17:00:23]

The previous comment answered "why?". From what I understand, it did so
incorrectly, but the intention was good.

[rvargas (doing something else), 2015/07/17 19:18:10]

yeah... I actually don't like this. FromDays(max_int) returns Max, but
FromDays(max_int - 1) doesn't, and neither value is really overflowing anything.
However I'm afraid of code somewhere relying on that behavior :(

So the real reason of "why" today is "backwards compatibility" (or actually,
fear of breaking things), even though that was never part of the contract.

I could go through the 746 call sites that I see. (+388 FromHours + 464
FromMinutes)

[Nico, 2015/07/17 20:21:20]

Hm, maybe just remove the comment then I guess. As is, it just repeats the line
below.

   if (days == std::numeric_limits<int>::max())
     return Max();
   return TimeDelta(days * Time::kMicrosecondsPerDay);

[Nico, 2015/07/17 17:00:23]

doesn't days * Time::kMicrosecondsPerDay overflow too if days is large but less
than max()?

…ah, kMicrosecondsPerDay is a int64, I see.

@@ -605,7 +608,7 @@ inline TimeDelta TimeDelta::FromDays(int days) {
 
 // static
 inline TimeDelta TimeDelta::FromHours(int hours) {
-  // Preserve max to prevent overflow.
+  // Special-case max.
   if (hours == std::numeric_limits<int>::max())
     return Max();
   return TimeDelta(hours * Time::kMicrosecondsPerHour);
@@ -613,7 +616,7 @@ inline TimeDelta TimeDelta::FromHours(int hours) {
 
 // static
 inline TimeDelta TimeDelta::FromMinutes(int minutes) {
-  // Preserve max to prevent overflow.
+  // Special-case max.
   if (minutes == std::numeric_limits<int>::max())
     return Max();
   return TimeDelta(minutes * Time::kMicrosecondsPerMinute);
@@ -621,44 +624,40 @@ inline TimeDelta TimeDelta::FromMinutes(int minutes) {
 
 // static
 inline TimeDelta TimeDelta::FromSeconds(int64 secs) {
-  // Preserve max to prevent overflow.
-  if (secs == std::numeric_limits<int64>::max())
-    return Max();
-  return TimeDelta(secs * Time::kMicrosecondsPerSecond);
+  return TimeDelta(secs) * Time::kMicrosecondsPerSecond;

[Nico, 2015/07/17 17:00:23]

Does TimeDelta have a saturating operator*?

[rvargas (doing something else), 2015/07/17 19:18:10]

Yes, all TimeDelta operators should do the right thing on overflow.

 }
 
 // static
 inline TimeDelta TimeDelta::FromMilliseconds(int64 ms) {
-  // Preserve max to prevent overflow.
-  if (ms == std::numeric_limits<int64>::max())
-    return Max();
-  return TimeDelta(ms * Time::kMicrosecondsPerMillisecond);
+  return TimeDelta(ms) * Time::kMicrosecondsPerMillisecond;
 }
 
 // static
 inline TimeDelta TimeDelta::FromSecondsD(double secs) {
-  // Preserve max to prevent overflow.
-  if (secs == std::numeric_limits<double>::infinity())
-    return Max();
-  return TimeDelta(static_cast<int64>(secs * Time::kMicrosecondsPerSecond));
+  return FromDouble(secs * Time::kMicrosecondsPerSecond);
 }
 
 // static
 inline TimeDelta TimeDelta::FromMillisecondsD(double ms) {
-  // Preserve max to prevent overflow.
-  if (ms == std::numeric_limits<double>::infinity())
-    return Max();
-  return TimeDelta(static_cast<int64>(ms * Time::kMicrosecondsPerMillisecond));
+  return FromDouble(ms * Time::kMicrosecondsPerMillisecond);
 }
 
 // static
 inline TimeDelta TimeDelta::FromMicroseconds(int64 us) {
-  // Preserve max to prevent overflow.
-  if (us == std::numeric_limits<int64>::max())
-    return Max();
   return TimeDelta(us);
 }
 
+// static
+inline TimeDelta TimeDelta::FromDouble(double value) {
+  double max_magnitude = std::numeric_limits<int64>::max();
+  TimeDelta delta = TimeDelta(static_cast<int64>(value));
+  if (value > max_magnitude)
+    delta = Max();
+  else if (value < -max_magnitude)
+    delta = -Max();
+  return delta;
+}
+
 // For logging use only.
 BASE_EXPORT std::ostream& operator<<(std::ostream& os, Time time);