Removed static const network units.

Static const objects can cause what's called a "static initialization order fiasco". This CL removes the statically initialized network units in favor of constexpr defined versions available via static functions. Bug: webrtc:8415 Change-Id: Ib1b316ae007481c52a53b2d1bb0352a630a220e2 Reviewed-on: https://webrtc-review.googlesource.com/65164 Commit-Queue: Sebastian Jansson <srte@webrtc.org> Reviewed-by: Björn Terelius <terelius@webrtc.org> Cr-Commit-Position: refs/heads/master@{#22670}
2018-03-29 11:00:07 +02:00
parent 9d2788f745
commit 63b48df334
7 changed files with 77 additions and 94 deletions
--- a/modules/congestion_controller/bbr/rtt_stats_unittest.cc
+++ b/modules/congestion_controller/bbr/rtt_stats_unittest.cc
@ -131,7 +131,7 @@ TEST_F(RttStatsTest, UpdateRttWithBadSendDeltas) {

  std::vector<TimeDelta> bad_send_deltas;
  bad_send_deltas.push_back(TimeDelta::Zero());
-  bad_send_deltas.push_back(TimeDelta::Infinity());
+  bad_send_deltas.push_back(TimeDelta::PlusInfinity());
  bad_send_deltas.push_back(TimeDelta::us(-1000));

  for (TimeDelta bad_send_delta : bad_send_deltas) {
--- a/modules/congestion_controller/bbr/windowed_filter_unittest.cc
+++ b/modules/congestion_controller/bbr/windowed_filter_unittest.cc
@ -292,7 +292,7 @@ TEST_F(WindowedFilterTest, ExpireAllMins) {
  TimeDelta rtt_sample = windowed_min_rtt_.GetThirdBest() + TimeDelta::ms(5);
  // This assert is necessary to avoid triggering -Wstrict-overflow
  // See crbug/616957
-  ASSERT_LT(windowed_min_rtt_.GetThirdBest(), TimeDelta::Infinity());
+  ASSERT_LT(windowed_min_rtt_.GetThirdBest(), TimeDelta::PlusInfinity());
  // Third best min sample was recorded at 100ms, so expiry time is 199ms.
  int64_t now_ms = 200;
  windowed_min_rtt_.Update(rtt_sample, now_ms);
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
@ -218,7 +218,7 @@ void GoogCcNetworkController::OnStreamsConfig(StreamsConfig msg) {

 void GoogCcNetworkController::OnTargetRateConstraints(
    TargetRateConstraints constraints) {
-  UpdateBitrateConstraints(constraints, DataRate::kNotInitialized);
+  UpdateBitrateConstraints(constraints, DataRate());
 }

 void GoogCcNetworkController::UpdateBitrateConstraints(
--- a/modules/congestion_controller/network_control/include/network_units.h
+++ b/modules/congestion_controller/network_control/include/network_units.h
@ -17,6 +17,11 @@

 namespace webrtc {
 namespace units_internal {
+constexpr int64_t kPlusInfinityVal = std::numeric_limits<int64_t>::max();
+constexpr int64_t kMinusInfinityVal = std::numeric_limits<int64_t>::min();
+constexpr int64_t kSignedNotInitializedVal = kMinusInfinityVal + 1;
+constexpr int64_t kNotInitializedVal = -1;
+
 inline int64_t DivideAndRound(int64_t numerator, int64_t denominators) {
  if (numerator >= 0) {
    return (numerator + (denominators / 2)) / denominators;
@ -26,7 +31,7 @@ inline int64_t DivideAndRound(int64_t numerator, int64_t denominators) {
 }
 }  // namespace units_internal

-// TimeDelta represents the difference between two timestamps. Connomly this can
+// TimeDelta represents the difference between two timestamps. Commonly this can
 // be a duration. However since two Timestamps are not guaranteed to have the
 // same epoch (they might come from different computers, making exact
 // synchronisation infeasible), the duration covered by a TimeDelta can be
@ -35,13 +40,14 @@ inline int64_t DivideAndRound(int64_t numerator, int64_t denominators) {
 // microseconds (us).
 class TimeDelta {
 public:
-  static const TimeDelta kPlusInfinity;
-  static const TimeDelta kMinusInfinity;
-  static const TimeDelta kNotInitialized;
-  static const TimeDelta kZero;
-  TimeDelta() : TimeDelta(kNotInitialized) {}
-  static TimeDelta Zero() { return kZero; }
-  static TimeDelta Infinity() { return kPlusInfinity; }
+  TimeDelta() : TimeDelta(units_internal::kSignedNotInitializedVal) {}
+  static TimeDelta Zero() { return TimeDelta(0); }
+  static TimeDelta PlusInfinity() {
+    return TimeDelta(units_internal::kPlusInfinityVal);
+  }
+  static TimeDelta MinusInfinity() {
+    return TimeDelta(units_internal::kMinusInfinityVal);
+  }
  static TimeDelta seconds(int64_t seconds) { return TimeDelta::s(seconds); }
  static TimeDelta s(int64_t seconds) {
    return TimeDelta::us(seconds * 1000000);
@ -65,10 +71,17 @@ class TimeDelta {
  bool IsZero() const { return microseconds_ == 0; }
  bool IsFinite() const { return IsInitialized() && !IsInfinite(); }
  bool IsInitialized() const {
-    return microseconds_ != kNotInitialized.microseconds_;
+    return microseconds_ != units_internal::kSignedNotInitializedVal;
  }
  bool IsInfinite() const {
-    return *this == kPlusInfinity || *this == kMinusInfinity;
+    return microseconds_ == units_internal::kPlusInfinityVal ||
+           microseconds_ == units_internal::kMinusInfinityVal;
+  }
+  bool IsPlusInfinity() const {
+    return microseconds_ == units_internal::kPlusInfinityVal;
+  }
+  bool IsMinusInfinity() const {
+    return microseconds_ == units_internal::kMinusInfinityVal;
  }
  TimeDelta operator+(const TimeDelta& other) const {
    return TimeDelta::us(us() + other.us());
@ -133,10 +146,10 @@ inline TimeDelta operator*(const int32_t& scalar, const TimeDelta& delta) {
 // difference of two Timestamps results in a TimeDelta.
 class Timestamp {
 public:
-  static const Timestamp kPlusInfinity;
-  static const Timestamp kNotInitialized;
-  Timestamp() : Timestamp(kNotInitialized) {}
-  static Timestamp Infinity() { return kPlusInfinity; }
+  Timestamp() : Timestamp(units_internal::kNotInitializedVal) {}
+  static Timestamp Infinity() {
+    return Timestamp(units_internal::kPlusInfinityVal);
+  }
  static Timestamp seconds(int64_t seconds) { return Timestamp::s(seconds); }
  static Timestamp s(int64_t seconds) {
    return Timestamp::us(seconds * 1000000);
@ -153,10 +166,10 @@ class Timestamp {
    return microseconds_;
  }
  bool IsInfinite() const {
-    return microseconds_ == kPlusInfinity.microseconds_;
+    return microseconds_ == units_internal::kPlusInfinityVal;
  }
  bool IsInitialized() const {
-    return microseconds_ != kNotInitialized.microseconds_;
+    return microseconds_ != units_internal::kNotInitializedVal;
  }
  bool IsFinite() const { return IsInitialized() && !IsInfinite(); }
  TimeDelta operator-(const Timestamp& other) const {
@ -198,12 +211,11 @@ class Timestamp {
 // truncated to fit.
 class DataSize {
 public:
-  static const DataSize kZero;
-  static const DataSize kPlusInfinity;
-  static const DataSize kNotInitialized;
-  DataSize() : DataSize(kNotInitialized) {}
-  static DataSize Zero() { return kZero; }
-  static DataSize Infinity() { return kPlusInfinity; }
+  DataSize() : DataSize(units_internal::kNotInitializedVal) {}
+  static DataSize Zero() { return DataSize(0); }
+  static DataSize Infinity() {
+    return DataSize(units_internal::kPlusInfinityVal);
+  }
  static DataSize bytes(int64_t bytes) {
    RTC_DCHECK_GE(bytes, 0);
    return DataSize(bytes);
@ -224,8 +236,10 @@ class DataSize {
    return units_internal::DivideAndRound(bits(), 1000);
  }
  bool IsZero() const { return bytes_ == 0; }
-  bool IsInfinite() const { return bytes_ == kPlusInfinity.bytes_; }
-  bool IsInitialized() const { return bytes_ != kNotInitialized.bytes_; }
+  bool IsInfinite() const { return bytes_ == units_internal::kPlusInfinityVal; }
+  bool IsInitialized() const {
+    return bytes_ != units_internal::kNotInitializedVal;
+  }
  bool IsFinite() const { return IsInitialized() && !IsInfinite(); }
  DataSize operator-(const DataSize& other) const {
    return DataSize::bytes(bytes() - other.bytes());
@ -288,12 +302,11 @@ inline DataSize operator*(const int32_t& scalar, const DataSize& size) {
 // resolution should document this by changing this comment.
 class DataRate {
 public:
-  static const DataRate kZero;
-  static const DataRate kPlusInfinity;
-  static const DataRate kNotInitialized;
-  DataRate() : DataRate(kNotInitialized) {}
-  static DataRate Zero() { return kZero; }
-  static DataRate Infinity() { return kPlusInfinity; }
+  DataRate() : DataRate(units_internal::kNotInitializedVal) {}
+  static DataRate Zero() { return DataRate(0); }
+  static DataRate Infinity() {
+    return DataRate(units_internal::kPlusInfinityVal);
+  }
  static DataRate bytes_per_second(int64_t bytes_per_sec) {
    RTC_DCHECK_GE(bytes_per_sec, 0);
    return DataRate(bytes_per_sec * 8);
@ -320,10 +333,10 @@ class DataRate {
  int64_t kbps() const { return units_internal::DivideAndRound(bps(), 1000); }
  bool IsZero() const { return bits_per_sec_ == 0; }
  bool IsInfinite() const {
-    return bits_per_sec_ == kPlusInfinity.bits_per_sec_;
+    return bits_per_sec_ == units_internal::kPlusInfinityVal;
  }
  bool IsInitialized() const {
-    return bits_per_sec_ != kNotInitialized.bits_per_sec_;
+    return bits_per_sec_ != units_internal::kNotInitializedVal;
  }
  bool IsFinite() const { return IsInitialized() && !IsInfinite(); }
  DataRate operator*(double scalar) const;
--- a/modules/congestion_controller/network_control/network_units.cc
+++ b/modules/congestion_controller/network_control/network_units.cc
@ -12,29 +12,6 @@
 #include <cmath>

 namespace webrtc {
-namespace {
-int64_t kPlusInfinityVal = std::numeric_limits<int64_t>::max();
-int64_t kMinusInfinityVal = std::numeric_limits<int64_t>::min();
-int64_t kSignedNotInitializedVal = kMinusInfinityVal + 1;
-int64_t kNotInitializedVal = -1;
-}  // namespace
-const TimeDelta TimeDelta::kZero = TimeDelta(0);
-const TimeDelta TimeDelta::kMinusInfinity = TimeDelta(kMinusInfinityVal);
-const TimeDelta TimeDelta::kPlusInfinity = TimeDelta(kPlusInfinityVal);
-const TimeDelta TimeDelta::kNotInitialized =
-    TimeDelta(kSignedNotInitializedVal);
-
-const Timestamp Timestamp::kPlusInfinity = Timestamp(kPlusInfinityVal);
-const Timestamp Timestamp::kNotInitialized = Timestamp(kNotInitializedVal);
-
-const DataRate DataRate::kZero = DataRate(0);
-const DataRate DataRate::kPlusInfinity = DataRate(kPlusInfinityVal);
-const DataRate DataRate::kNotInitialized = DataRate(kNotInitializedVal);
-
-const DataSize DataSize::kZero = DataSize(0);
-const DataSize DataSize::kPlusInfinity = DataSize(kPlusInfinityVal);
-const DataSize DataSize::kNotInitialized = DataSize(kNotInitializedVal);
-
 TimeDelta TimeDelta::operator*(double scalar) const {
  return TimeDelta::us(std::round(us() * scalar));
 }
@ -74,38 +51,38 @@ DataSize operator*(const TimeDelta& duration, const DataRate& rate) {
 }

 ::std::ostream& operator<<(::std::ostream& os, const DataRate& value) {
-  if (value == DataRate::kPlusInfinity) {
+  if (value.IsInfinite()) {
    return os << "inf bps";
-  } else if (value == DataRate::kNotInitialized) {
+  } else if (!value.IsInitialized()) {
    return os << "? bps";
  } else {
    return os << value.bps() << " bps";
  }
 }
 ::std::ostream& operator<<(::std::ostream& os, const DataSize& value) {
-  if (value == DataSize::kPlusInfinity) {
+  if (value.IsInfinite()) {
    return os << "inf bytes";
-  } else if (value == DataSize::kNotInitialized) {
+  } else if (!value.IsInitialized()) {
    return os << "? bytes";
  } else {
    return os << value.bytes() << " bytes";
  }
 }
 ::std::ostream& operator<<(::std::ostream& os, const Timestamp& value) {
-  if (value == Timestamp::kPlusInfinity) {
+  if (value.IsInfinite()) {
    return os << "inf ms";
-  } else if (value == Timestamp::kNotInitialized) {
+  } else if (!value.IsInitialized()) {
    return os << "? ms";
  } else {
    return os << value.ms() << " ms";
  }
 }
 ::std::ostream& operator<<(::std::ostream& os, const TimeDelta& value) {
-  if (value == TimeDelta::kPlusInfinity) {
+  if (value.IsPlusInfinity()) {
    return os << "+inf ms";
-  } else if (value == TimeDelta::kMinusInfinity) {
+  } else if (value.IsMinusInfinity()) {
    return os << "-inf ms";
-  } else if (value == TimeDelta::kNotInitialized) {
+  } else if (!value.IsInitialized()) {
    return os << "? ms";
  } else {
    return os << value.ms() << " ms";
--- a/modules/congestion_controller/network_control/network_units_unittest.cc
+++ b/modules/congestion_controller/network_control/network_units_unittest.cc
@ -42,16 +42,14 @@ TEST(TimeDeltaTest, IdentityChecks) {
  EXPECT_TRUE(TimeDelta::Zero().IsZero());
  EXPECT_FALSE(TimeDelta::ms(kValue).IsZero());

-  EXPECT_TRUE(TimeDelta::Infinity().IsInfinite());
-  EXPECT_TRUE(TimeDelta::kPlusInfinity.IsInfinite());
-  EXPECT_TRUE(TimeDelta::kMinusInfinity.IsInfinite());
+  EXPECT_TRUE(TimeDelta::PlusInfinity().IsInfinite());
+  EXPECT_TRUE(TimeDelta::MinusInfinity().IsInfinite());
  EXPECT_FALSE(TimeDelta::Zero().IsInfinite());
  EXPECT_FALSE(TimeDelta::ms(-kValue).IsInfinite());
  EXPECT_FALSE(TimeDelta::ms(kValue).IsInfinite());

-  EXPECT_FALSE(TimeDelta::Infinity().IsFinite());
-  EXPECT_FALSE(TimeDelta::kPlusInfinity.IsFinite());
-  EXPECT_FALSE(TimeDelta::kMinusInfinity.IsFinite());
+  EXPECT_FALSE(TimeDelta::PlusInfinity().IsFinite());
+  EXPECT_FALSE(TimeDelta::MinusInfinity().IsFinite());
  EXPECT_TRUE(TimeDelta::ms(-kValue).IsFinite());
  EXPECT_TRUE(TimeDelta::ms(kValue).IsFinite());
  EXPECT_TRUE(TimeDelta::Zero().IsFinite());
@ -63,8 +61,8 @@ TEST(TimeDeltaTest, ComparisonOperators) {
  const TimeDelta small = TimeDelta::ms(kSmall);
  const TimeDelta large = TimeDelta::ms(kLarge);

-  EXPECT_EQ(TimeDelta::Zero(), TimeDelta::Zero());
-  EXPECT_EQ(TimeDelta::Infinity(), TimeDelta::Infinity());
+  EXPECT_EQ(TimeDelta::Zero(), TimeDelta::ms(0));
+  EXPECT_EQ(TimeDelta::PlusInfinity(), TimeDelta::PlusInfinity());
  EXPECT_EQ(small, TimeDelta::ms(kSmall));
  EXPECT_LE(small, TimeDelta::ms(kSmall));
  EXPECT_GE(small, TimeDelta::ms(kSmall));
@ -73,12 +71,12 @@ TEST(TimeDeltaTest, ComparisonOperators) {
  EXPECT_LT(small, TimeDelta::ms(kLarge));
  EXPECT_GE(large, TimeDelta::ms(kSmall));
  EXPECT_GT(large, TimeDelta::ms(kSmall));
-  EXPECT_LT(TimeDelta::kZero, small);
-  EXPECT_GT(TimeDelta::kZero, TimeDelta::ms(-kSmall));
-  EXPECT_GT(TimeDelta::kZero, TimeDelta::ms(-kSmall));
+  EXPECT_LT(TimeDelta::Zero(), small);
+  EXPECT_GT(TimeDelta::Zero(), TimeDelta::ms(-kSmall));
+  EXPECT_GT(TimeDelta::Zero(), TimeDelta::ms(-kSmall));

-  EXPECT_GT(TimeDelta::kPlusInfinity, large);
-  EXPECT_LT(TimeDelta::kMinusInfinity, TimeDelta::kZero);
+  EXPECT_GT(TimeDelta::PlusInfinity(), large);
+  EXPECT_LT(TimeDelta::MinusInfinity(), TimeDelta::Zero());
 }

 TEST(TimeDeltaTest, MathOperations) {
@ -123,7 +121,7 @@ TEST(TimestampTest, IdentityChecks) {
  EXPECT_TRUE(Timestamp::Infinity().IsInfinite());
  EXPECT_FALSE(Timestamp::ms(kValue).IsInfinite());

-  EXPECT_FALSE(Timestamp::kNotInitialized.IsFinite());
+  EXPECT_FALSE(Timestamp().IsFinite());
  EXPECT_FALSE(Timestamp::Infinity().IsFinite());
  EXPECT_TRUE(Timestamp::ms(kValue).IsFinite());
 }
@ -175,12 +173,10 @@ TEST(DataSizeTest, IdentityChecks) {
  EXPECT_FALSE(DataSize::bytes(kValue).IsZero());

  EXPECT_TRUE(DataSize::Infinity().IsInfinite());
-  EXPECT_TRUE(DataSize::kPlusInfinity.IsInfinite());
  EXPECT_FALSE(DataSize::Zero().IsInfinite());
  EXPECT_FALSE(DataSize::bytes(kValue).IsInfinite());

  EXPECT_FALSE(DataSize::Infinity().IsFinite());
-  EXPECT_FALSE(DataSize::kPlusInfinity.IsFinite());
  EXPECT_TRUE(DataSize::bytes(kValue).IsFinite());
  EXPECT_TRUE(DataSize::Zero().IsFinite());
 }
@ -191,7 +187,7 @@ TEST(DataSizeTest, ComparisonOperators) {
  const DataSize small = DataSize::bytes(kSmall);
  const DataSize large = DataSize::bytes(kLarge);

-  EXPECT_EQ(DataSize::Zero(), DataSize::Zero());
+  EXPECT_EQ(DataSize::Zero(), DataSize::bytes(0));
  EXPECT_EQ(DataSize::Infinity(), DataSize::Infinity());
  EXPECT_EQ(small, small);
  EXPECT_LE(small, small);
@ -201,9 +197,8 @@ TEST(DataSizeTest, ComparisonOperators) {
  EXPECT_LT(small, large);
  EXPECT_GE(large, small);
  EXPECT_GT(large, small);
-  EXPECT_LT(DataSize::kZero, small);
-
-  EXPECT_GT(DataSize::kPlusInfinity, large);
+  EXPECT_LT(DataSize::Zero(), small);
+  EXPECT_GT(DataSize::Infinity(), large);
 }

 TEST(DataSizeTest, MathOperations) {
@ -250,12 +245,10 @@ TEST(DataRateTest, IdentityChecks) {
  EXPECT_FALSE(DataRate::bytes_per_second(kValue).IsZero());

  EXPECT_TRUE(DataRate::Infinity().IsInfinite());
-  EXPECT_TRUE(DataRate::kPlusInfinity.IsInfinite());
  EXPECT_FALSE(DataRate::Zero().IsInfinite());
  EXPECT_FALSE(DataRate::bytes_per_second(kValue).IsInfinite());

  EXPECT_FALSE(DataRate::Infinity().IsFinite());
-  EXPECT_FALSE(DataRate::kPlusInfinity.IsFinite());
  EXPECT_TRUE(DataRate::bytes_per_second(kValue).IsFinite());
  EXPECT_TRUE(DataRate::Zero().IsFinite());
 }
@ -266,7 +259,7 @@ TEST(DataRateTest, ComparisonOperators) {
  const DataRate small = DataRate::bytes_per_second(kSmall);
  const DataRate large = DataRate::bytes_per_second(kLarge);

-  EXPECT_EQ(DataRate::Zero(), DataRate::Zero());
+  EXPECT_EQ(DataRate::Zero(), DataRate::bps(0));
  EXPECT_EQ(DataRate::Infinity(), DataRate::Infinity());
  EXPECT_EQ(small, small);
  EXPECT_LE(small, small);
@ -276,8 +269,8 @@ TEST(DataRateTest, ComparisonOperators) {
  EXPECT_LT(small, large);
  EXPECT_GE(large, small);
  EXPECT_GT(large, small);
-  EXPECT_LT(DataRate::kZero, small);
-  EXPECT_GT(DataRate::kPlusInfinity, large);
+  EXPECT_LT(DataRate::Zero(), small);
+  EXPECT_GT(DataRate::Infinity(), large);
 }

 TEST(DataRateTest, MathOperations) {
--- a/modules/congestion_controller/rtp/send_side_congestion_controller.cc
+++ b/modules/congestion_controller/rtp/send_side_congestion_controller.cc
@ -426,8 +426,8 @@ void SendSideCongestionController::OnNetworkRouteChanged(
  msg.at_time = Timestamp::ms(clock_->TimeInMilliseconds());
  msg.constraints =
      ConvertConstraints(min_bitrate_bps, max_bitrate_bps, clock_);
-  msg.starting_rate = start_bitrate_bps > 0 ? DataRate::bps(start_bitrate_bps)
-                                            : DataRate::kNotInitialized;
+  msg.starting_rate =
+      start_bitrate_bps > 0 ? DataRate::bps(start_bitrate_bps) : DataRate();
  task_queue_->PostTask([this, msg]() {
    RTC_DCHECK_RUN_ON(task_queue_ptr_);
    controller_->OnNetworkRouteChange(msg);