Commit 8c687a70 authored by guidou's avatar guidou Committed by Commit bot

Revert of Add: check exploded time is properly converted (patchset #7...

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}
parent 4cbb231e
......@@ -263,14 +263,6 @@ bool Time::FromStringInternal(const char* time_string,
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);
......
......@@ -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 @@ class BASE_EXPORT Time : public time_internal::TimeBase<Time> {
// 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 @@ class BASE_EXPORT Time : public time_internal::TimeBase<Time> {
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 @@ class BASE_EXPORT Time : public time_internal::TimeBase<Time> {
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
......
......@@ -34,7 +34,7 @@ int64_t ComputeCurrentTicks() {
struct timeval boottime;
int mib[2] = {CTL_KERN, KERN_BOOTTIME};
size_t size = sizeof(boottime);
int kr = sysctl(mib, arraysize(mib), &boottime, &size, nullptr, 0);
int kr = sysctl(mib, arraysize(mib), &boottime, &size, NULL, 0);
DCHECK_EQ(KERN_SUCCESS, kr);
base::TimeDelta time_difference = base::Time::Now() -
(base::Time::FromTimeT(boottime.tv_sec) +
......@@ -168,7 +168,7 @@ Time Time::NowFromSystemTime() {
}
// static
bool Time::FromExploded(bool is_local, const Exploded& exploded, Time* time) {
Time Time::FromExploded(bool is_local, const Exploded& exploded) {
base::ScopedCFTypeRef<CFTimeZoneRef> time_zone(
is_local
? CFTimeZoneCopySystem()
......@@ -184,28 +184,8 @@ bool Time::FromExploded(bool is_local, const Exploded& exploded, Time* time) {
exploded.day_of_month, exploded.hour, exploded.minute, exploded.second,
exploded.millisecond);
CFAbsoluteTime seconds = absolute_time + kCFAbsoluteTimeIntervalSince1970;
base::Time converted_time =
Time(static_cast<int64_t>(seconds * kMicrosecondsPerSecond) +
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(static_cast<int64_t>(seconds * kMicrosecondsPerSecond) +
kWindowsEpochDeltaMicroseconds);
}
void Time::Explode(bool is_local, Exploded* exploded) const {
......
......@@ -211,7 +211,7 @@ void Time::Explode(bool is_local, Exploded* exploded) const {
}
// 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 @@ bool Time::FromExploded(bool is_local, const Exploded& exploded, Time* time) {
}
// 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 ------------------------------------------------------------------
......
......@@ -21,52 +21,6 @@ namespace base {
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
......
......@@ -235,7 +235,7 @@ bool Time::IsHighResolutionTimerInUse() {
}
// static
bool Time::FromExploded(bool is_local, const Exploded& exploded, Time* time) {
Time Time::FromExploded(bool is_local, const Exploded& exploded) {
// Create the system struct representing our exploded time. It will either be
// in local time or UTC.
SYSTEMTIME st;
......@@ -253,19 +253,17 @@ bool Time::FromExploded(bool is_local, const Exploded& exploded, Time* time) {
// Ensure that it's in UTC.
if (is_local) {
SYSTEMTIME utc_st;
success = TzSpecificLocalTimeToSystemTime(nullptr, &st, &utc_st) &&
success = TzSpecificLocalTimeToSystemTime(NULL, &st, &utc_st) &&
SystemTimeToFileTime(&utc_st, &ft);
} else {
success = !!SystemTimeToFileTime(&st, &ft);
}
if (!success) {
*time = Time(0);
return false;
NOTREACHED() << "Unable to convert time";
return Time(0);
}
*time = Time(FileTimeToMicroseconds(ft));
return true;
return Time(FileTimeToMicroseconds(ft));
}
void Time::Explode(bool is_local, Exploded* exploded) const {
......@@ -290,7 +288,7 @@ void Time::Explode(bool is_local, Exploded* exploded) const {
// daylight saving time, it will take daylight saving time into account,
// even if the time you are converting is in standard time.
success = FileTimeToSystemTime(&utc_ft, &utc_st) &&
SystemTimeToTzSpecificLocalTime(nullptr, &utc_st, &st);
SystemTimeToTzSpecificLocalTime(NULL, &utc_st, &st);
} else {
success = !!FileTimeToSystemTime(&utc_ft, &st);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment