Fix InterProcessTimeTicksConverter in the case that

remote range < local range.  The new code maps the entire
remote range onto the entire local range.

Also accepts "null" input times and leaves them untouched.

This is needed before the new load timing code can be hooked
up (https://codereview.chromium.org/12094085/), as it will
sometimes result in requestStart being before fetchStart
otherwise.

BUG=174170

Review URL: https://chromiumcodereview.appspot.com/12453003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@187363 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 123 insertions, 79 deletions

diff --git a/content/common/inter_process_time_ticks_converter.cc b/content/common/inter_process_time_ticks_converter.cc
index 9e1ba0c..6ae309c 100644
--- a/content/common/inter_process_time_ticks_converter.cc
+++ b/content/common/inter_process_time_ticks_converter.cc
@@ -5,6 +5,7 @@
 #include "content/common/inter_process_time_ticks_converter.h"
 
 #include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
 
 namespace content {
 
@@ -15,37 +16,46 @@ InterProcessTimeTicksConverter::InterProcessTimeTicksConverter(
     const RemoteTimeTicks& remote_upper_bound)
     : remote_lower_bound_(remote_lower_bound.value_),
       remote_upper_bound_(remote_upper_bound.value_) {
-#define CONVERTER_DISABLED_DUE_TO_BUG_174170
-#ifdef CONVERTER_DISABLED_DUE_TO_BUG_174170
-  numerator_ = 1;
-  denominator_ = 1;
-  offset_ = 0;
-  return;
-#endif
   int64 target_range = local_upper_bound.value_ - local_lower_bound.value_;
   int64 source_range = remote_upper_bound.value_ - remote_lower_bound.value_;
+  DCHECK_GE(target_range, 0);
+  DCHECK_GE(source_range, 0);
   if (source_range <= target_range) {
-    // We fit!  Just shift the midpoints to match.
+    // We fit!  Center the source range on the target range.
     numerator_ = 1;
     denominator_ = 1;
-    offset_ = ((local_upper_bound.value_ + local_lower_bound.value_) -
-               (remote_upper_bound.value_ + remote_lower_bound.value_)) / 2;
+    local_base_time_ =
+        local_lower_bound.value_ + (target_range - source_range) / 2;
+    // When converting times, remote bounds should fall within local bounds.
+    DCHECK_LE(local_lower_bound.value_,
+              ToLocalTimeTicks(remote_lower_bound).value_);
+    DCHECK_GE(local_upper_bound.value_,
+              ToLocalTimeTicks(remote_upper_bound).value_);
     return;
   }
-  // Set up scaling factors, and then deduce shift.
+
+  // Interpolate values so that remote range will be will exactly fit into the
+  // local range, if possible.
   numerator_ = target_range;
   denominator_ = source_range;
-  // Find out what we need to shift by to make this really work.
-  offset_ = local_lower_bound.value_ - Convert(remote_lower_bound.value_);
-  DCHECK_GE(local_upper_bound.value_, Convert(remote_upper_bound.value_));
+  local_base_time_ = local_lower_bound.value_;
+  // When converting times, remote bounds should equal local bounds.
+  DCHECK_EQ(local_lower_bound.value_,
+            ToLocalTimeTicks(remote_lower_bound).value_);
+  DCHECK_EQ(local_upper_bound.value_,
+            ToLocalTimeTicks(remote_upper_bound).value_);
+  DCHECK_EQ(target_range, Convert(source_range));
 }
 
 LocalTimeTicks InterProcessTimeTicksConverter::ToLocalTimeTicks(
     const RemoteTimeTicks& remote_ms) {
+  // If input time is "null", return another "null" time.
+  if (remote_ms.value_ == 0)
+    return LocalTimeTicks(0);
   DCHECK_LE(remote_lower_bound_, remote_ms.value_);
   DCHECK_GE(remote_upper_bound_, remote_ms.value_);
   RemoteTimeDelta remote_delta = remote_ms - remote_lower_bound_;
-  return LocalTimeTicks(remote_lower_bound_ + offset_ +
+  return LocalTimeTicks(local_base_time_ +
                         ToLocalTimeDelta(remote_delta).value_);
 }
 
diff --git a/content/common/inter_process_time_ticks_converter.h b/content/common/inter_process_time_ticks_converter.h
index 49a7c70..cd95594 100644
--- a/content/common/inter_process_time_ticks_converter.h
+++ b/content/common/inter_process_time_ticks_converter.h
@@ -64,7 +64,9 @@ class CONTENT_EXPORT InterProcessTimeTicksConverter {
  private:
   int64 Convert(int64 value);
 
-  int64 offset_;
+  // The local time which |remote_lower_bound_| is mapped to.
+  int64 local_base_time_;
+
   int64 numerator_;
   int64 denominator_;
 
diff --git a/content/common/inter_process_time_ticks_converter_unittest.cc b/content/common/inter_process_time_ticks_converter_unittest.cc
index 83433e0..84db29d 100644
--- a/content/common/inter_process_time_ticks_converter_unittest.cc
+++ b/content/common/inter_process_time_ticks_converter_unittest.cc
@@ -2,11 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// NOTE: All of these tests have been disabled due to the
-// InterProcessTimeTicksConverter being disabled due to bug 174170.
+#include "content/common/inter_process_time_ticks_converter.h"
 
 #include "base/time.h"
-#include "content/common/inter_process_time_ticks_converter.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::TimeTicks;
@@ -55,119 +53,133 @@ TestResults RunTest(const TestParams& params) {
   return results;
 }
 
-TEST(InterProcessTimeTicksConverterTest, DISABLED_NoSkew) {
+TEST(InterProcessTimeTicksConverterTest, NullTime) {
+  // Null / zero times should remain null.
+  TestParams p;
+  p.local_lower_bound = 1;
+  p.remote_lower_bound = 2;
+  p.remote_upper_bound = 5;
+  p.local_upper_bound = 6;
+  p.test_time = 0;
+  p.test_delta = 0;
+  TestResults results = RunTest(p);
+  EXPECT_EQ(0, results.result_time);
+  EXPECT_EQ(0, results.result_delta);
+}
+
+TEST(InterProcessTimeTicksConverterTest, NoSkew) {
   // All times are monotonic and centered, so no adjustment should occur.
   TestParams p;
-  p.local_lower_bound = 0;
-  p.remote_lower_bound = 1;
-  p.remote_upper_bound = 4;
-  p.local_upper_bound = 5;
-  p.test_time = 2;
+  p.local_lower_bound = 1;
+  p.remote_lower_bound = 2;
+  p.remote_upper_bound = 5;
+  p.local_upper_bound = 6;
+  p.test_time = 3;
   p.test_delta = 1;
   TestResults results = RunTest(p);
-  EXPECT_EQ(2, results.result_time);
+  EXPECT_EQ(3, results.result_time);
   EXPECT_EQ(1, results.result_delta);
 }
 
-TEST(InterProcessTimeTicksConverterTest, DISABLED_OffsetMidpoints) {
+TEST(InterProcessTimeTicksConverterTest, OffsetMidpoints) {
   // All times are monotonic, but not centered. Adjust the |remote_*| times so
   // they are centered within the |local_*| times.
   TestParams p;
-  p.local_lower_bound = 0;
-  p.remote_lower_bound = 2;
-  p.remote_upper_bound = 5;
-  p.local_upper_bound = 5;
-  p.test_time = 3;
+  p.local_lower_bound = 1;
+  p.remote_lower_bound = 3;
+  p.remote_upper_bound = 6;
+  p.local_upper_bound = 6;
+  p.test_time = 4;
   p.test_delta = 1;
   TestResults results = RunTest(p);
-  EXPECT_EQ(2, results.result_time);
+  EXPECT_EQ(3, results.result_time);
   EXPECT_EQ(1, results.result_delta);
 }
 
-TEST(InterProcessTimeTicksConverterTest, DISABLED_DoubleEndedSkew) {
+TEST(InterProcessTimeTicksConverterTest, DoubleEndedSkew) {
   // |remote_lower_bound| occurs before |local_lower_bound| and
   // |remote_upper_bound| occurs after |local_upper_bound|. We must adjust both
   // bounds and scale down the delta. |test_time| is on the midpoint, so it
   // doesn't change. The ratio of local time to network time is 1:2, so we scale
   // |test_delta| to half.
   TestParams p;
-  p.local_lower_bound = 2;
-  p.remote_lower_bound = 0;
-  p.remote_upper_bound = 8;
-  p.local_upper_bound = 6;
-  p.test_time = 4;
+  p.local_lower_bound = 3;
+  p.remote_lower_bound = 1;
+  p.remote_upper_bound = 9;
+  p.local_upper_bound = 7;
+  p.test_time = 5;
   p.test_delta = 2;
   TestResults results = RunTest(p);
-  EXPECT_EQ(4, results.result_time);
+  EXPECT_EQ(5, results.result_time);
   EXPECT_EQ(1, results.result_delta);
 }
 
-TEST(InterProcessTimeTicksConverterTest, DISABLED_FrontEndSkew) {
+TEST(InterProcessTimeTicksConverterTest, FrontEndSkew) {
   // |remote_upper_bound| is coherent, but |remote_lower_bound| is not. So we
   // adjust the lower bound and move |test_time| out. The scale factor is 2:3,
   // but since we use integers, the numbers truncate from 3.33 to 3 and 1.33
   // to 1.
   TestParams p;
-  p.local_lower_bound = 2;
-  p.remote_lower_bound = 0;
-  p.remote_upper_bound = 6;
-  p.local_upper_bound = 6;
-  p.test_time = 2;
+  p.local_lower_bound = 3;
+  p.remote_lower_bound = 1;
+  p.remote_upper_bound = 7;
+  p.local_upper_bound = 7;
+  p.test_time = 3;
   p.test_delta = 2;
   TestResults results = RunTest(p);
-  EXPECT_EQ(3, results.result_time);
+  EXPECT_EQ(4, results.result_time);
   EXPECT_EQ(1, results.result_delta);
 }
 
-TEST(InterProcessTimeTicksConverterTest, DISABLED_BackEndSkew) {
+TEST(InterProcessTimeTicksConverterTest, BackEndSkew) {
   // Like the previous test, but |remote_lower_bound| is coherent and
   // |remote_upper_bound| is skewed.
   TestParams p;
-  p.local_lower_bound = 0;
-  p.remote_lower_bound = 0;
-  p.remote_upper_bound = 6;
-  p.local_upper_bound = 4;
-  p.test_time = 2;
+  p.local_lower_bound = 1;
+  p.remote_lower_bound = 1;
+  p.remote_upper_bound = 7;
+  p.local_upper_bound = 5;
+  p.test_time = 3;
   p.test_delta = 2;
   TestResults results = RunTest(p);
-  EXPECT_EQ(1, results.result_time);
+  EXPECT_EQ(2, results.result_time);
   EXPECT_EQ(1, results.result_delta);
 }
 
-TEST(InterProcessTimeTicksConverterTest, DISABLED_Instantaneous) {
+TEST(InterProcessTimeTicksConverterTest, Instantaneous) {
   // The bounds are all okay, but the |remote_lower_bound| and
   // |remote_upper_bound| have the same value. No adjustments should be made and
   // no divide-by-zero errors should occur.
   TestParams p;
-  p.local_lower_bound = 0;
-  p.remote_lower_bound = 1;
-  p.remote_upper_bound = 1;
-  p.local_upper_bound = 2;
-  p.test_time = 1;
+  p.local_lower_bound = 1;
+  p.remote_lower_bound = 2;
+  p.remote_upper_bound = 2;
+  p.local_upper_bound = 3;
+  p.test_time = 2;
   p.test_delta = 0;
   TestResults results = RunTest(p);
-  EXPECT_EQ(1, results.result_time);
+  EXPECT_EQ(2, results.result_time);
   EXPECT_EQ(0, results.result_delta);
 }
 
-TEST(InterProcessTimeTicksConverterTest, DISABLED_OffsetInstantaneous) {
+TEST(InterProcessTimeTicksConverterTest, OffsetInstantaneous) {
   // The bounds are all okay, but the |remote_lower_bound| and
   // |remote_upper_bound| have the same value and are offset from the midpoint
   // of |local_lower_bound| and |local_upper_bound|. An offset should be applied
   // to make the midpoints line up.
   TestParams p;
-  p.local_lower_bound = 0;
-  p.remote_lower_bound = 2;
-  p.remote_upper_bound = 2;
-  p.local_upper_bound = 2;
-  p.test_time = 2;
+  p.local_lower_bound = 1;
+  p.remote_lower_bound = 3;
+  p.remote_upper_bound = 3;
+  p.local_upper_bound = 3;
+  p.test_time = 3;
   p.test_delta = 0;
   TestResults results = RunTest(p);
-  EXPECT_EQ(1, results.result_time);
+  EXPECT_EQ(2, results.result_time);
   EXPECT_EQ(0, results.result_delta);
 }
 
-TEST(InterProcessTimeTicksConverterTest, DISABLED_DisjointInstantaneous) {
+TEST(InterProcessTimeTicksConverterTest, DisjointInstantaneous) {
   // |local_lower_bound| and |local_upper_bound| are the same. No matter what
   // the other values are, they must fit within [local_lower_bound,
   // local_upper_bound].  So, all of the values should be adjusted so they are
@@ -184,26 +196,46 @@ TEST(InterProcessTimeTicksConverterTest, DISABLED_DisjointInstantaneous) {
   EXPECT_EQ(0, results.result_delta);
 }
 
-TEST(InterProcessTimeTicksConverterTest, DISABLED_RoundingNearEdges) {
+TEST(InterProcessTimeTicksConverterTest, RoundingNearEdges) {
   // Verify that rounding never causes a value to appear outside the given
   // |local_*| range.
-  const int kMaxRange = 100;
-  for (int i = 0; i < kMaxRange; ++i) {
-    for (int j = 0; j < kMaxRange; ++j) {
+  const int kMaxRange = 101;
+  for (int i = 1; i < kMaxRange; ++i) {
+    for (int j = 1; j < kMaxRange; ++j) {
       TestParams p;
-      p.local_lower_bound = 0;
-      p.remote_lower_bound = 0;
+      p.local_lower_bound = 1;
+      p.remote_lower_bound = 1;
       p.remote_upper_bound = j;
       p.local_upper_bound = i;
-      p.test_time = 0;
-      p.test_delta = j;
+
+      p.test_time = 1;
+      p.test_delta = 0;
       TestResults results = RunTest(p);
-      EXPECT_LE(0, results.result_time);
-      EXPECT_GE(i, results.result_delta);
+      EXPECT_LE(1, results.result_time);
+      EXPECT_EQ(0, results.result_delta);
+
+      p.test_time = j;
+      p.test_delta = j - 1;
+      results = RunTest(p);
+      EXPECT_GE(i, results.result_time);
+      EXPECT_GE(i - 1, results.result_delta);
     }
   }
 }
 
+TEST(InterProcessTimeTicksConverterTest, DisjointRanges) {
+  TestParams p;
+  p.local_lower_bound = 10;
+  p.remote_lower_bound = 30;
+  p.remote_upper_bound = 41;
+  p.local_upper_bound = 20;
+  p.test_time = 41;
+  p.test_delta = 0;
+  TestResults results = RunTest(p);
+  EXPECT_EQ(20, results.result_time);
+  EXPECT_EQ(0, results.result_delta);
+}
+
 }  // anonymous namespace
 
 }  // namespace content