Revert of Add: check exploded time is properly converted (patchset #7 id:340001 of https://codereview.chromium.org/1988663002/ )
Reason for revert:
Suspect of breking Linux Tests bot. https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29%281%29%2832%29?numbuilds=200
I will reland if the revert does not fix the bot.
Original issue's description:
> Add: check exploded time is properly converted.
>
> This cl introduces time checking in posix-like and mac systems.
> base::Time::FromUTCExploded() and
> base::Time::FromLocalExplode() can fail without returning
> a proper error.
>
> This fix does the following:
> 1) After calculations are done, create UTC or local time.
> 2) Convert UTC or local time back to exploded
> 3) Compare original exploded with converted one
> 4) If times are not equal, then return Time(0) indicating
> an error.
>
> Windows implementation already returns Time(0) on error.
>
> BUG=601900
>
> Committed: https://crrev.com/ceba9fb480269695775191d14e98ab23b5918382
> Cr-Commit-Position: refs/heads/master@{#396638}
TBR=mark@chromium.org,mmenke@chromium.org,eroman@chromium.org,maksim.sisov@intel.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=601900
Review-Url: https://codereview.chromium.org/2022913002
Cr-Commit-Position: refs/heads/master@{#396810}
CrOS-Libchrome-Original-Commit: 8c687a70b52123f35c0504a5b3c5c84a639cebaf
diff --git a/base/time/time.cc b/base/time/time.cc
index 8b1a6d7..76ffeb7 100644
--- a/base/time/time.cc
+++ b/base/time/time.cc
@@ -263,14 +263,6 @@
return true;
}
-// static
-bool Time::ExplodedMostlyEquals(const Exploded& lhs, const Exploded& rhs) {
- return lhs.year == rhs.year && lhs.month == rhs.month &&
- lhs.day_of_month == rhs.day_of_month && lhs.hour == rhs.hour &&
- lhs.minute == rhs.minute && lhs.second == rhs.second &&
- lhs.millisecond == rhs.millisecond;
-}
-
std::ostream& operator<<(std::ostream& os, Time time) {
Time::Exploded exploded;
time.UTCExplode(&exploded);
diff --git a/base/time/time.h b/base/time/time.h
index 63583ae..399ec82 100644
--- a/base/time/time.h
+++ b/base/time/time.h
@@ -56,7 +56,6 @@
#include <limits>
#include "base/base_export.h"
-#include "base/compiler_specific.h"
#include "base/numerics/safe_math.h"
#include "build/build_config.h"
@@ -520,29 +519,11 @@
// 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) {
- base::Time time;
- ignore_result(FromUTCExploded(exploded, &time));
- return time;
+ return FromExploded(false, exploded);
}
static Time FromLocalExploded(const Exploded& exploded) {
- base::Time time;
- ignore_result(FromLocalExploded(exploded, &time));
- return time;
- }
-
- // Converts an exploded structure representing either the local time or UTC
- // into a Time class. Returns false on a failure when, for example, a day of
- // month is set to 31 on a 28-30 day month.
- 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);
+ return FromExploded(true, exploded);
}
// Converts a string representation of time to a Time object.
@@ -583,12 +564,8 @@
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|. 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;
+ // |is_local = true| or UTC |is_local = false|.
+ static Time FromExploded(bool is_local, const Exploded& exploded);
// Converts a string representation of time to a Time object.
// An example of a time string which is converted is as below:-
@@ -600,9 +577,6 @@
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);
};
// static
diff --git a/base/time/time_posix.cc b/base/time/time_posix.cc
index 495e249..32614bc 100644
--- a/base/time/time_posix.cc
+++ b/base/time/time_posix.cc
@@ -211,7 +211,7 @@
}
// static
-bool Time::FromExploded(bool is_local, const Exploded& exploded, Time* time) {
+Time Time::FromExploded(bool is_local, const Exploded& exploded) {
struct tm timestruct;
timestruct.tm_sec = exploded.second;
timestruct.tm_min = exploded.minute;
@@ -301,26 +301,8 @@
}
// Adjust from Unix (1970) to Windows (1601) epoch.
- base::Time converted_time =
- Time((milliseconds * kMicrosecondsPerMillisecond) +
- kWindowsEpochDeltaMicroseconds);
-
- // If |exploded.day_of_month| is set to 31 on a 28-30 day month, it will
- // return the first day of the next month. Thus round-trip the time and
- // compare the initial |exploded| with |utc_to_exploded| time.
- base::Time::Exploded to_exploded;
- if (!is_local)
- converted_time.UTCExplode(&to_exploded);
- else
- converted_time.LocalExplode(&to_exploded);
-
- if (ExplodedMostlyEquals(to_exploded, exploded)) {
- *time = converted_time;
- return true;
- }
-
- *time = Time(0);
- return false;
+ return Time((milliseconds * kMicrosecondsPerMillisecond) +
+ kWindowsEpochDeltaMicroseconds);
}
// TimeTicks ------------------------------------------------------------------
diff --git a/base/time/time_unittest.cc b/base/time/time_unittest.cc
index ce68b04..25c6ca5 100644
--- a/base/time/time_unittest.cc
+++ b/base/time/time_unittest.cc
@@ -21,52 +21,6 @@
namespace {
-TEST(TimeTestOutOfBounds, FromExplodedOutOfBoundsTime) {
- // FromUTCExploded must set time to Time(0) and failure, if the day is set to
- // 31 on a 28-30 day month. Test |exploded| returns Time(0) on 31st of
- // February and 31st of April. New implementation handles this.
-
- const struct DateTestData {
- Time::Exploded explode;
- bool is_valid;
- } kDateTestData[] = {
- // 31st of February
- {{2016, 2, 0, 31, 12, 30, 0, 0}, true},
- // 31st of April
- {{2016, 4, 0, 31, 8, 43, 0, 0}, true},
- // Negative month
- {{2016, -5, 0, 2, 4, 10, 0, 0}, false},
- // Negative date of month
- {{2016, 6, 0, -15, 2, 50, 0, 0}, false},
- // Negative hours
- {{2016, 7, 0, 10, -11, 29, 0, 0}, false},
- // Negative minutes
- {{2016, 3, 0, 14, 10, -29, 0, 0}, false},
- // Negative seconds
- {{2016, 10, 0, 25, 7, 47, -30, 0}, false},
- // Negative milliseconds
- {{2016, 10, 0, 25, 7, 47, 20, -500}, false},
- // Hours are too large
- {{2016, 7, 0, 10, 26, 29, 0, 0}, false},
- // Minutes are too large
- {{2016, 3, 0, 14, 10, 78, 0, 0}, false},
- // Seconds are too large
- {{2016, 10, 0, 25, 7, 47, 234, 0}, false},
- // Milliseconds are too large
- {{2016, 10, 0, 25, 6, 31, 23, 1643}, false},
- };
-
- for (const auto& test : kDateTestData) {
- EXPECT_EQ(test.explode.HasValidValues(), test.is_valid);
-
- base::Time result;
- EXPECT_FALSE(base::Time::FromUTCExploded(test.explode, &result));
- EXPECT_TRUE(result.is_null());
- EXPECT_FALSE(base::Time::FromLocalExploded(test.explode, &result));
- EXPECT_TRUE(result.is_null());
- }
-}
-
// Specialized test fixture allowing time strings without timezones to be
// tested by comparing them to a known time in the local zone.
// See also pr_time_unittests.cc