Add: check exploded time is properly converted

base/time/time.h

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2016 The Chromium Authors. All rights reserved.

[mmenke, 2016/05/18 17:32:00]

You shouldn't update copyright dates when updating files.  We used to do that,
but then the word came not to do it.  I'm not a lawyer, so can't explain why.

This goes for all files.

[maksims (do not use this acc), 2016/05/19 09:57:20]

Done.

 // 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
 // this.
@@ skipping 401 matching lines @@
     int hour;          // Hour within the current day (0-23)
     int minute;        // Minute within the current hour (0-59)
     int second;        // Second within the current minute (0-59 plus leap
                        //   seconds which may take it up to 60).
     int millisecond;   // Milliseconds within the current second (0-999)
 
     // A cursory test for whether the data members are within their
     // respective ranges. A 'true' return value does not guarantee the
     // Exploded value can be successfully converted to a Time value.
     bool HasValidValues() const;
+    bool operator!=(const Exploded& rhs);

[eroman, 2016/05/18 19:35:17]

I don't think we want to call this operator!=().

The issue is that with a == or != one expects every field to be compared for
equality.

Whereas for this application, we do *not* want to compare equality of
|day_of_week|, since that is a computed value.

Other naming ideas:

  IsEquivalent()
  Matches()

[Mark Mentovai, 2016/05/18 20:07:15]

I had the same comment as a draft, then I went to recheck what the style guide
said to get a good link to it, and I found that the recommendation has changed.

https://google.github.io/styleguide/cppguide.html#Operator_Overloading

Notably:
So I’m OK with this overload now.

[maksims (do not use this acc), 2016/05/19 09:57:20]

Ok!

   };
 
   // Contains the NULL time. Use Time::Now() to get the current time.
   Time() : TimeBase(0) {
   }
 
   // Returns the time for epoch in Unix-like system (Jan 1, 1970).
   static Time UnixEpoch();
 
   // Returns the current time. Watch out, the system might adjust its clock
@@ skipping 74 matching lines @@
   // this function to deactivate the high resolution timer.
   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.

[eroman, 2016/05/18 19:35:17]

This documentation should also be updated to clarify what happens on errors, and
what is considered an error.

While this is definitely an improvement over the current situation, (and I am on
board with making this change) I do feel that an explicit error condition would
be better (i.e. return a boolean and assign to an Exploded).

With a sentinel value based approach like Time(0) there is the usual problem of
ambiguity arising as to whether Time(0) means the earliest possible time, or an
error.

The following is a contrived example, but consider the case of certificate
verification -- if we parse the "notBefore" and get Time(0) we would have to
treat that as meaning parsing failed and reject the certificate. Whereas it may
have been the notBefore truly was Time(0) and parsing succeeded, and the
certificate is subsequently valid. ... Admittedly this is kind of a bogus
example, since Time(0) is 1601, and I am pretty sure our posix time exploding
implementations can't go earlier than 1970. But maybe this sequence of events
would happen on Windows.

[maksims (do not use this acc), 2016/05/19 09:57:19]

Well, can you please check the patch? Your proposal sounds very reasonable and
in order to transform other code, I overloaded the functions to provide callers
with true or false and the converted time.

   static Time FromUTCExploded(const Exploded& exploded) {
     return FromExploded(false, exploded);
   }
   static Time FromLocalExploded(const Exploded& exploded) {
     return FromExploded(true, exploded);
   }
 
   // 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
@@ skipping 248 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_