Add: check exploded time is properly converted

base/time/time.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Time represents an absolute point in coordinated universal time (UTC),
 // internally represented as microseconds (s/1,000,000) since the Windows epoch
 // (1601-01-01 00:00:00 UTC). System-dependent clock interface routines are
 // defined in time_PLATFORM.cc. Note that values for Time may skew and jump
 // around as the operating system makes adjustments to synchronize (e.g., with
 // NTP servers). Thus, client code that uses the Time class must account for
@@ skipping 38 matching lines @@
 #ifndef BASE_TIME_TIME_H_
 #define BASE_TIME_TIME_H_
 
 #include <stdint.h>
 #include <time.h>
 
 #include <iosfwd>
 #include <limits>
 
 #include "base/base_export.h"
+#include "base/compiler_specific.h"
 #include "base/numerics/safe_math.h"
 #include "build/build_config.h"
 
 #if defined(OS_MACOSX)
 #include <CoreFoundation/CoreFoundation.h>
 // Avoid Mac system header macro leak.
 #undef TYPE_BOOL
 #endif
 
 #if defined(OS_POSIX)
@@ skipping 443 matching lines @@
   static bool ActivateHighResolutionTimer(bool activate);
 
   // Returns true if the high resolution timer is both enabled and activated.
   // This is provided for testing only, and is not tracked in a thread-safe
   // way.
   static bool IsHighResolutionTimerInUse();
 #endif
 
   // Converts an exploded structure representing either the local time or UTC
   // into a Time class.
+  // TODO(maksims): Get rid of these in favor of the methods below when
+  // all the callers stop using these ones.
   static Time FromUTCExploded(const Exploded& exploded) {
-    return FromExploded(false, exploded);
+    base::Time time;
+    ignore_result(FromExploded(false, exploded, &time));

[Mark Mentovai, 2016/05/26 14:08:39]

Centralize the “false”/“true”. This should be
ignore_result(FromUTCExploded(exploded, &time)). Similar on line 532.

Rationale: “false”/“true” require reading the definition to understand whether
it’s “is_local” or “is_utc” or something else entirely. Since it’s a private
implementation detail, it’s not worth adding an enum to clarify this at call
sites, but since equivalent function names are available, you should use them.

+    return time;
   }
   static Time FromLocalExploded(const Exploded& exploded) {
-    return FromExploded(true, exploded);
+    base::Time time;
+    ignore_result(FromExploded(true, exploded, &time));
+    return time;
+  }
+
+  // Converts an exploded structure representing either the local time or UTC
+  // into a Time class. Returns false on failure. For example,
+  // FromExploded can fail when 31st of a month is set on a 28-30 day month,
+  // which results in 1st day of the next month.

[Mark Mentovai, 2016/05/26 14:08:39]

Nuke “which results in 1st day of the next month.” We’re not defining what the
31st day of a 30-day month means, we’re saying that it’s a failure.

+  static bool FromUTCExploded(const Exploded& exploded,
+                              Time* time) WARN_UNUSED_RESULT {
+    return FromExploded(false, exploded, time);
+  }
+  static bool FromLocalExploded(const Exploded& exploded,
+                                Time* time) WARN_UNUSED_RESULT {
+    return FromExploded(true, exploded, time);
   }
 
   // Converts a string representation of time to a Time object.
   // An example of a time string which is converted is as below:-
   // "Tue, 15 Nov 1994 12:45:26 GMT". If the timezone is not specified
   // in the input string, FromString assumes local time and FromUTCString
   // assumes UTC. A timezone that cannot be parsed (e.g. "UTC" which is not
   // specified in RFC822) is treated as if the timezone is not specified.
   // TODO(iyengar) Move the FromString/FromTimeT/ToTimeT/FromFileTime to
   // a new time converter class.
@@ skipping 20 matching lines @@
  private:
   friend class time_internal::TimeBase<Time>;
 
   explicit Time(int64_t us) : TimeBase(us) {}
 
   // Explodes the given time to either local time |is_local = true| or UTC
   // |is_local = false|.
   void Explode(bool is_local, Exploded* exploded) const;
 
   // Unexplodes a given time assuming the source is either local time
-  // |is_local = true| or UTC |is_local = false|.
-  static Time FromExploded(bool is_local, const Exploded& exploded);
+  // |is_local = true| or UTC |is_local = false|. Function returns false on
+  // failure and sets |time| to Time(0). Otherwise returns true and sets |time|
+  // to non-exploded time.
+  static bool FromExploded(bool is_local,
+                           const Exploded& exploded,
+                           Time* time) WARN_UNUSED_RESULT;
 
   // Converts a string representation of time to a Time object.
   // An example of a time string which is converted is as below:-
   // "Tue, 15 Nov 1994 12:45:26 GMT". If the timezone is not specified
   // in the input string, local time |is_local = true| or
   // UTC |is_local = false| is assumed. A timezone that cannot be parsed
   // (e.g. "UTC" which is not specified in RFC822) is treated as if the
   // timezone is not specified.
   static bool FromStringInternal(const char* time_string,
                                  bool is_local,
                                  Time* parsed_time);
+
+  // Comparison does not consider |day_of_week| when doing the operation.
+  static bool ExplodedMostlyEquals(const Exploded& lhs, const Exploded& rhs);

[Mark Mentovai, 2016/05/26 14:08:39]

Not sure why this isn’t a non-static MostlyEquals() defined as a member of
Exploded. Is there a reason?

[mmenke, 2016/05/26 14:45:08]

If we put it there, we'd either need to make it public, which seems not great,
or make this class a friend of Exploded.

[maksims (do not use this acc), 2016/05/27 10:19:08]

Yes, exactly. Should I change it to be a friend then?

 };
 
 // static
 constexpr TimeDelta TimeDelta::FromDays(int days) {
   return days == std::numeric_limits<int>::max()
              ? Max()
              : TimeDelta(days * Time::kMicrosecondsPerDay);
 }
 
 // static
@@ skipping 199 matching lines @@
   static void WaitUntilInitializedWin();
 #endif
 };
 
 // For logging use only.
 BASE_EXPORT std::ostream& operator<<(std::ostream& os, ThreadTicks time_ticks);
 
 }  // namespace base
 
 #endif  // BASE_TIME_TIME_H_