Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-08 02:14:43 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1626734 - Fix edge case that can cause TimeStamps to go backwards by fractional milliseconds. r=karlt

Browse Source 
Differential Revision: https://phabricator.services.mozilla.com/D72954
This commit is contained in:
Kartikaya Gupta 2020-05-07 12:56:09 +00:00
parent 2c6ee4a761
commit 55681e438e
2 changed files with 50 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
widget/SystemTimeConverter.h
View File

@ -73,7 +73,8 @@ class SystemTimeConverter {
// the case when we have a backlog of events and by the time we process
// them, the time given by the system is comparatively "old".
//
// We call the absolute difference between (i) and (ii), "deltaFromNow".
// The IsNewerThanTimestamp function computes the equivalent of |aTime| in
// the TimeStamp scale and returns that in |timeAsTimeStamp|.
//
// Graphically:
//
@ -86,13 +87,13 @@ class SystemTimeConverter {
// |------timeStampDelta-------|
//
// |---|
// deltaFromNow
// roughlyNow-timeAsTimeStamp
//
Time deltaFromNow;
bool newer = IsTimeNewerThanTimestamp(aTime, roughlyNow, &deltaFromNow);
TimeStamp timeAsTimeStamp;
bool newer = IsTimeNewerThanTimestamp(aTime, roughlyNow, &timeAsTimeStamp);
// Tolerance when detecting clock skew.
static const Time kTolerance = 30;
static const TimeDuration kTolerance = TimeDuration::FromMilliseconds(30.0);
// Check for forwards skew
if (newer) {
@ -106,7 +107,7 @@ class SystemTimeConverter {
return roughlyNow;
}
if (deltaFromNow <= kTolerance) {
if (roughlyNow - timeAsTimeStamp <= kTolerance) {
// If the time between event times and TimeStamp values is within
// the tolerance then assume we don't have clock skew so we can
// avoid checking for backwards skew for a while.
@ -117,7 +118,7 @@ class SystemTimeConverter {
}
// Finally, calculate the timestamp
return roughlyNow - TimeDuration::FromMilliseconds(deltaFromNow);
return timeAsTimeStamp;
}
void CompensateForBackwardsSkew(Time aReferenceTime,
@ -155,8 +156,7 @@ class SystemTimeConverter {
//
// If that's not the case, then we probably just got caught behind
// temporarily.
Time delta;
if (IsTimeNewerThanTimestamp(aReferenceTime, aLowerBound, &delta)) {
if (IsTimeNewerThanTimestamp(aReferenceTime, aLowerBound, nullptr)) {
return;
}
@ -190,7 +190,7 @@ class SystemTimeConverter {
}
bool IsTimeNewerThanTimestamp(Time aTime, TimeStamp aTimeStamp,
Time* aDelta) {
TimeStamp* aTimeAsTimeStamp) {
Time timeDelta = aTime - mReferenceTime;
// Cast the result to signed 64-bit integer first since that should be
@ -199,18 +199,23 @@ class SystemTimeConverter {
// is outside the integer range is undefined.
// Then we do an implicit cast to Time (typically an unsigned 32-bit
// integer) which wraps times outside that range.
Time timeStampDelta = static_cast<int64_t>(
(aTimeStamp - mReferenceTimeStamp).ToMilliseconds());
TimeDuration timeStampDelta = (aTimeStamp - mReferenceTimeStamp);
int64_t wholeMillis = static_cast<int64_t>(timeStampDelta.ToMilliseconds());
Time wrappedTimeStampDelta = wholeMillis; // truncate to unsigned
Time timeToTimeStamp = timeStampDelta - timeDelta;
Time timeToTimeStamp = wrappedTimeStampDelta - timeDelta;
bool isNewer = false;
if (timeToTimeStamp == 0) {
*aDelta = 0;
// wholeMillis needs no adjustment
} else if (timeToTimeStamp < kTimeHalfRange) {
*aDelta = timeToTimeStamp;
wholeMillis -= timeToTimeStamp;
} else {
isNewer = true;
*aDelta = timeDelta - timeStampDelta;
wholeMillis += (-timeToTimeStamp);
}
if (aTimeAsTimeStamp) {
*aTimeAsTimeStamp =
mReferenceTimeStamp + TimeDuration::FromMilliseconds(wholeMillis);
}
return isNewer;

29
widget/tests/gtest/TestTimeConverter.cpp
View File

@ -234,3 +234,32 @@ TEST(TimeConverter, HalfRangeBoundary)
ts = converter.GetTimeStampFromSystemTime(secondEvent, unused);
EXPECT_TS(ts, 0);
}
TEST(TimeConverter, FractionalMillisBug1626734)
{
MockTimeStamp::Init();
TimeConverter converter;
GTestTime eventTime = 10;
MockCurrentTimeGetter timeGetter(eventTime);
UnusedCurrentTimeGetter<GTestTime> unused;
TimeStamp ts = converter.GetTimeStampFromSystemTime(eventTime, timeGetter);
EXPECT_TS(ts, 0);
MockTimeStamp::Advance(0.2);
ts = converter.GetTimeStampFromSystemTime(eventTime, unused);
EXPECT_TS(ts, 0);
MockTimeStamp::Advance(0.9);
TimeStamp ts2 = converter.GetTimeStampFromSystemTime(eventTime, unused);
EXPECT_TS(ts2, 0);
// Since ts2 came from a "future" call relative to ts, we expect ts2 to not
// be "before" ts. (i.e. time shouldn't go backwards, even by fractional
// milliseconds). This assertion is technically already implied by the
// EXPECT_TS checks above, but fixing this assertion is the end result that
// we wanted in bug 1626734 so it feels appropriate to recheck it explicitly.
EXPECT_TRUE(ts <= ts2);
}