Commit 2f8de5cd authored by Yuri Wiitala's avatar Yuri Wiitala Committed by Commit Bot

Fixit: Add PRESUBMIT check for TimeXXX::To/FromInternalValue().

The fix-it to remove the uses of To/FromInternalValue() is underway.
This change adds a PRESUBMIT warning (BANNED_CPP_FUNCTIONS) and
base/time/time.h header comments, to avoid having new code use these
deprecated functions.

The intention is for this change to be short-lived, and removed once
the deprecated functions are removed from the time classes.

One fault in the check: Since the script can only do regexp matching,
it's impossible to check that the ToInternalValue() method is being
called on one of the TimeXXX types. However, there are relatively few
non-TimeXXX types that have a ToInternalValue() method. Therefore, it
should be rare for the script to trigger a warning erroneously (and
this would be easy to bypass, anyway).

Bug: 634507
Change-Id: I48d75435703771107c87935cb3ea90a5a956989e
Reviewed-on: https://chromium-review.googlesource.com/577980Reviewed-by: default avatarPaweł Hajdan Jr. <phajdan.jr@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488505}
parent 63874f8d
...@@ -369,6 +369,21 @@ _BANNED_CPP_FUNCTIONS = ( ...@@ -369,6 +369,21 @@ _BANNED_CPP_FUNCTIONS = (
False, False,
(), (),
), ),
(
r'/(Time(|Delta|Ticks)|ThreadTicks)::FromInternalValue|ToInternalValue',
(
'base::TimeXXX::FromInternalValue() and ToInternalValue() are',
'deprecated (http://crbug.com/634507). Please avoid converting away',
'from the Time types in Chromium code, especially if any math is',
'being done on time values. For interfacing with platform/library',
'APIs, use FromMicroseconds() or InMicroseconds(), or one of the other',
'type converter methods instead. For faking TimeXXX values (for unit',
'testing only), use TimeXXX() + TimeDelta::FromMicroseconds(N). For',
'other use cases, please contact base/time/OWNERS.',
),
False,
(),
),
( (
'CallJavascriptFunctionUnsafe', 'CallJavascriptFunctionUnsafe',
( (
......
...@@ -129,6 +129,8 @@ class BASE_EXPORT TimeDelta { ...@@ -129,6 +129,8 @@ class BASE_EXPORT TimeDelta {
// when deserializing a |TimeDelta| structure, using a value known to be // when deserializing a |TimeDelta| structure, using a value known to be
// compatible. It is not provided as a constructor because the integer type // compatible. It is not provided as a constructor because the integer type
// may be unclear from the perspective of a caller. // may be unclear from the perspective of a caller.
//
// DEPRECATED - Do not use in new code. http://crbug.com/634507
static TimeDelta FromInternalValue(int64_t delta) { return TimeDelta(delta); } static TimeDelta FromInternalValue(int64_t delta) { return TimeDelta(delta); }
// Returns the maximum time delta, which should be greater than any reasonable // Returns the maximum time delta, which should be greater than any reasonable
...@@ -145,6 +147,8 @@ class BASE_EXPORT TimeDelta { ...@@ -145,6 +147,8 @@ class BASE_EXPORT TimeDelta {
// use this and do arithmetic on it, as it is more error prone than using the // use this and do arithmetic on it, as it is more error prone than using the
// provided operators. // provided operators.
// For serializing, use FromInternalValue to reconstitute. // For serializing, use FromInternalValue to reconstitute.
//
// DEPRECATED - Do not use in new code. http://crbug.com/634507
int64_t ToInternalValue() const { return delta_; } int64_t ToInternalValue() const { return delta_; }
// Returns the magnitude (absolute value) of this TimeDelta. // Returns the magnitude (absolute value) of this TimeDelta.
...@@ -355,6 +359,8 @@ class TimeBase { ...@@ -355,6 +359,8 @@ class TimeBase {
// For serializing only. Use FromInternalValue() to reconstitute. Please don't // 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 // use this and do arithmetic on it, as it is more error prone than using the
// provided operators. // provided operators.
//
// DEPRECATED - Do not use in new code. http://crbug.com/634507
int64_t ToInternalValue() const { return us_; } int64_t ToInternalValue() const { return us_; }
TimeClass& operator=(TimeClass other) { TimeClass& operator=(TimeClass other) {
...@@ -407,6 +413,8 @@ class TimeBase { ...@@ -407,6 +413,8 @@ class TimeBase {
// when deserializing a |TimeClass| structure, using a value known to be // when deserializing a |TimeClass| structure, using a value known to be
// compatible. It is not provided as a constructor because the integer type // compatible. It is not provided as a constructor because the integer type
// may be unclear from the perspective of a caller. // may be unclear from the perspective of a caller.
//
// DEPRECATED - Do not use in new code. http://crbug.com/634507
static TimeClass FromInternalValue(int64_t us) { return TimeClass(us); } static TimeClass FromInternalValue(int64_t us) { return TimeClass(us); }
protected: protected:
......
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