From 59fca1cd30487a013b942acf9d660dfcc639e25a Mon Sep 17 00:00:00 2001 From: miu Date: Thu, 7 May 2015 14:42:09 -0700 Subject: [PATCH] Revert of Revert of Fixit: Factor out common base::Time* math operator overloads. (patchset #1 id:1 of https://codereview.chromium.org/1130953002/) Reason for revert: iOS_Device builder was not rebuilding certain modules dependent on changes in base::Time. http://crbug.com/485435 Original issue's description: > Revert of Fixit: Factor out common base::Time* math operator overloads. (patchset #2 id:40001 of https://codereview.chromium.org/1122443004/) > > Reason for revert: > Broke iOS build. > > http://build.chromium.org/p/chromium.mac/builders/iOS_Device/builds/5621/steps/compile/logs/stdio > > Original issue's description: > > Fixit: Factor out common base::Time* math operator overloads. > > > > This is part 1 of a 2-part change to fork base::TimeTicks into three > > type-checked time classes (TimeTicks + ThreadTicks + TraceTicks). The > > forking of TimeTicks will ensure values from different clocks are not > > erroneously being mixed together when doing time math. > > > > In this change, the identical comparison and math operator overloads > > found in base::Time and base::TimeTicks are being factored-out into a > > templated base class. In a following change, this base class will also > > be used to de-dupe this common functionality in the two new time > > classes. > > > > BUG=467417 > > > > Committed: https://crrev.com/7ca717095b4758cb76e53e904b775852e46d646d > > Cr-Commit-Position: refs/heads/master@{#328696} > > TBR=thestig@chromium.org,miu@chromium.org > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG=467417 > > Committed: https://crrev.com/a365825583412c355a449274bec70e41e992ffd7 > Cr-Commit-Position: refs/heads/master@{#328706} TBR=thestig@chromium.org,ksakamoto@chromium.org NOPRESUBMIT=true BUG=467417,485435 Review URL: https://codereview.chromium.org/1128273004 Cr-Commit-Position: refs/heads/master@{#328843} --- base/time/time.cc | 22 +-- base/time/time.h | 364 ++++++++++++++++--------------------- base/time/time_unittest.cc | 4 +- 3 files changed, 171 insertions(+), 219 deletions(-) diff --git a/base/time/time.cc b/base/time/time.cc index bf6c998ebf8cd2..9834188597d479 100644 --- a/base/time/time.cc +++ b/base/time/time.cc @@ -97,20 +97,21 @@ int64 TimeDelta::InMicroseconds() const { return delta_; } -int64 TimeDelta::SaturatedAdd(int64 value) const { - CheckedNumeric rv(delta_); +namespace time_internal { + +int64 SaturatedAdd(TimeDelta delta, int64 value) { + CheckedNumeric rv(delta.delta_); rv += value; return FromCheckedNumeric(rv); } -int64 TimeDelta::SaturatedSub(int64 value) const { - CheckedNumeric rv(delta_); +int64 SaturatedSub(TimeDelta delta, int64 value) { + CheckedNumeric rv(delta.delta_); rv -= value; return FromCheckedNumeric(rv); } -// static -int64 TimeDelta::FromCheckedNumeric(const CheckedNumeric value) { +int64 FromCheckedNumeric(const CheckedNumeric value) { if (value.IsValid()) return value.ValueUnsafe(); @@ -124,6 +125,8 @@ int64 TimeDelta::FromCheckedNumeric(const CheckedNumeric value) { return value.ValueOrDefault(limit); } +} // namespace time_internal + std::ostream& operator<<(std::ostream& os, TimeDelta time_delta) { return os << time_delta.InSecondsF() << "s"; } @@ -305,15 +308,12 @@ TimeTicks TimeTicks::SnappedToNextTick(TimeTicks tick_phase, TimeDelta tick_interval) const { // |interval_offset| is the offset from |this| to the next multiple of // |tick_interval| after |tick_phase|, possibly negative if in the past. - TimeDelta interval_offset = TimeDelta::FromInternalValue( - (tick_phase - *this).ToInternalValue() % tick_interval.ToInternalValue()); + TimeDelta interval_offset = (tick_phase - *this) % tick_interval; // If |this| is exactly on the interval (i.e. offset==0), don't adjust. // Otherwise, if |tick_phase| was in the past, adjust forward to the next // tick after |this|. - if (interval_offset.ToInternalValue() != 0 && tick_phase < *this) { + if (!interval_offset.is_zero() && tick_phase < *this) interval_offset += tick_interval; - } - return *this + interval_offset; } diff --git a/base/time/time.h b/base/time/time.h index 5c8b89c18cf174..0a2677838617e7 100644 --- a/base/time/time.h +++ b/base/time/time.h @@ -57,8 +57,23 @@ namespace base { -class Time; -class TimeTicks; +class TimeDelta; + +// The functions in the time_internal namespace are meant to be used only by the +// time classes and functions. Please use the math operators defined in the +// time classes instead. +namespace time_internal { + +// Add or subtract |value| from a TimeDelta. The int64 argument and return value +// are in terms of a microsecond timebase. +BASE_EXPORT int64 SaturatedAdd(TimeDelta delta, int64 value); +BASE_EXPORT int64 SaturatedSub(TimeDelta delta, int64 value); + +// Clamp |value| on overflow and underflow conditions. The int64 argument and +// return value are in terms of a microsecond timebase. +BASE_EXPORT int64 FromCheckedNumeric(const CheckedNumeric value); + +} // namespace time_internal // TimeDelta ------------------------------------------------------------------ @@ -110,6 +125,11 @@ class BASE_EXPORT TimeDelta { return TimeDelta((delta_ + mask) ^ mask); } + // Returns true if the time delta is zero. + bool is_zero() const { + return delta_ == 0; + } + // Returns true if the time delta is the maximum time delta. bool is_max() const { return delta_ == std::numeric_limits::max(); @@ -141,19 +161,17 @@ class BASE_EXPORT TimeDelta { // Computations with other deltas. TimeDelta operator+(TimeDelta other) const { - return TimeDelta(SaturatedAdd(other.delta_)); + return TimeDelta(time_internal::SaturatedAdd(*this, other.delta_)); } TimeDelta operator-(TimeDelta other) const { - return TimeDelta(SaturatedSub(other.delta_)); + return TimeDelta(time_internal::SaturatedSub(*this, other.delta_)); } TimeDelta& operator+=(TimeDelta other) { - delta_ = SaturatedAdd(other.delta_); - return *this; + return *this = (*this + other); } TimeDelta& operator-=(TimeDelta other) { - delta_ = SaturatedSub(other.delta_); - return *this; + return *this = (*this - other); } TimeDelta operator-() const { return TimeDelta(-delta_); @@ -164,36 +182,29 @@ class BASE_EXPORT TimeDelta { TimeDelta operator*(T a) const { CheckedNumeric rv(delta_); rv *= a; - return TimeDelta(FromCheckedNumeric(rv)); + return TimeDelta(time_internal::FromCheckedNumeric(rv)); } template TimeDelta operator/(T a) const { CheckedNumeric rv(delta_); rv /= a; - return TimeDelta(FromCheckedNumeric(rv)); + return TimeDelta(time_internal::FromCheckedNumeric(rv)); } template TimeDelta& operator*=(T a) { - CheckedNumeric rv(delta_); - rv *= a; - delta_ = FromCheckedNumeric(rv); - return *this; + return *this = (*this * a); } template TimeDelta& operator/=(T a) { - CheckedNumeric rv(delta_); - rv /= a; - delta_ = FromCheckedNumeric(rv); - return *this; + return *this = (*this / a); } int64 operator/(TimeDelta a) const { return delta_ / a.delta_; } - - // Defined below because it depends on the definition of the other classes. - Time operator+(Time t) const; - TimeTicks operator+(TimeTicks t) const; + TimeDelta operator%(TimeDelta a) const { + return TimeDelta(delta_ % a.delta_); + } // Comparison operators. bool operator==(TimeDelta other) const { @@ -216,8 +227,8 @@ class BASE_EXPORT TimeDelta { } private: - friend class Time; - friend class TimeTicks; + friend int64 time_internal::SaturatedAdd(TimeDelta delta, int64 value); + friend int64 time_internal::SaturatedSub(TimeDelta delta, int64 value); // Constructs a delta given the duration in microseconds. This is private // to avoid confusion by callers with an integer constructor. Use @@ -225,13 +236,6 @@ class BASE_EXPORT TimeDelta { explicit TimeDelta(int64 delta_us) : delta_(delta_us) { } - // Add or subtract |value| from this delta. - int64 SaturatedAdd(int64 value) const; - int64 SaturatedSub(int64 value) const; - - // Clamp |value| on overflow and underflow conditions. - static int64 FromCheckedNumeric(const CheckedNumeric value); - // Delta in microseconds. int64 delta_; }; @@ -244,10 +248,19 @@ inline TimeDelta operator*(T a, TimeDelta td) { // For logging use only. BASE_EXPORT std::ostream& operator<<(std::ostream& os, TimeDelta time_delta); -// Time ----------------------------------------------------------------------- +// Do not reference the time_internal::TimeBase template class directly. Please +// use one of the time subclasses instead, and only reference the public +// TimeBase members via those classes. +namespace time_internal { + +// TimeBase-------------------------------------------------------------------- -// Represents a wall clock time in UTC. -class BASE_EXPORT Time { +// Provides value storage and comparison/math operations common to all time +// classes. Each subclass provides for strong type-checking to ensure +// semantically meaningful comparison/math of time values from the same clock +// source or timeline. +template +class TimeBase { public: static const int64 kHoursPerDay = 24; static const int64 kMillisecondsPerSecond = 1000; @@ -264,6 +277,102 @@ class BASE_EXPORT Time { static const int64 kNanosecondsPerSecond = kNanosecondsPerMicrosecond * kMicrosecondsPerSecond; + // Returns true if this object has not been initialized. + // + // Warning: Be careful when writing code that performs math on time values, + // since it's possible to produce a valid "zero" result that should not be + // interpreted as a "null" value. + bool is_null() const { + return us_ == 0; + } + + // Returns true if this object represents the maximum time. + bool is_max() const { + return us_ == std::numeric_limits::max(); + } + + // For serializing only. Use FromInternalValue() to reconstitute. Please don't + // use this and do arithmetic on it, as it is more error prone than using the + // provided operators. + int64 ToInternalValue() const { + return us_; + } + + TimeClass& operator=(TimeClass other) { + us_ = other.us_; + return *(static_cast(this)); + } + + // Compute the difference between two times. + TimeDelta operator-(TimeClass other) const { + return TimeDelta::FromMicroseconds(us_ - other.us_); + } + + // Return a new time modified by some delta. + TimeClass operator+(TimeDelta delta) const { + return TimeClass(time_internal::SaturatedAdd(delta, us_)); + } + TimeClass operator-(TimeDelta delta) const { + return TimeClass(-time_internal::SaturatedSub(delta, us_)); + } + + // Modify by some time delta. + TimeClass& operator+=(TimeDelta delta) { + return static_cast(*this = (*this + delta)); + } + TimeClass& operator-=(TimeDelta delta) { + return static_cast(*this = (*this - delta)); + } + + // Comparison operators + bool operator==(TimeClass other) const { + return us_ == other.us_; + } + bool operator!=(TimeClass other) const { + return us_ != other.us_; + } + bool operator<(TimeClass other) const { + return us_ < other.us_; + } + bool operator<=(TimeClass other) const { + return us_ <= other.us_; + } + bool operator>(TimeClass other) const { + return us_ > other.us_; + } + bool operator>=(TimeClass other) const { + return us_ >= other.us_; + } + + // Converts an integer value representing TimeClass to a class. This is used + // when deserializing a |TimeClass| structure, using a value known to be + // compatible. It is not provided as a constructor because the integer type + // may be unclear from the perspective of a caller. + static TimeClass FromInternalValue(int64 us) { + return TimeClass(us); + } + + protected: + explicit TimeBase(int64 us) : us_(us) { + } + + // Time value in a microsecond timebase. + int64 us_; +}; + +} // namespace time_internal + +template +inline TimeClass operator+(TimeDelta delta, TimeClass t) { + return t + delta; +} + +// Time ----------------------------------------------------------------------- + +// Represents a wall clock time in UTC. Values are not guaranteed to be +// monotonically non-decreasing and are subject to large amounts of skew. +class BASE_EXPORT Time : public time_internal::TimeBase