Refactor MediumEnvironment to improve FakeClock usage.

PiperOrigin-RevId: 904891357

internal/platform/implementation/g3/scheduled_executor.cc

@@ -16,16 +16,15 @@
 
 #include <atomic>
 #include <memory>
-#include <optional>
 #include <utility>
 
 #include "absl/strings/str_format.h"
 #include "absl/synchronization/mutex.h"
 #include "absl/time/time.h"
 #include "internal/platform/implementation/cancelable.h"
+#include "internal/platform/logging.h"
 #include "internal/platform/medium_environment.h"
 #include "internal/platform/runnable.h"
-#include "internal/test/fake_clock.h"
 
 namespace nearby {
 namespace g3 {
@@ -66,20 +65,13 @@ class ScheduledCancelable : public api::Cancelable {
 }  // namespace
 
 ScheduledExecutor::ScheduledExecutor() {
-  std::optional<FakeClock*> fake_clock =
-      MediumEnvironment::Instance().GetSimulatedClock();
-  if (fake_clock.has_value()) {
-    name_ = absl::StrFormat("G3 scheduled executor %p", this);
-    (*fake_clock)->AddObserver(name_, [this]() { RunReadyTasks(); });
-  }
+  name_ = absl::StrFormat("G3 scheduled executor %p", this);
+  MediumEnvironment::Instance().AddSimulatedClockObserver(
+      name_, [this]() { RunReadyTasks(); });
 }
 
 ScheduledExecutor::~ScheduledExecutor() {
-  std::optional<FakeClock*> fake_clock =
-      MediumEnvironment::Instance().GetSimulatedClock();
-  if (fake_clock.has_value()) {
-    (*fake_clock)->RemoveObserver(name_);
-  }
+  MediumEnvironment::Instance().RemoveSimulatedClockObserver(name_);
   executor_.Shutdown();
 }
 
@@ -96,10 +88,10 @@ std::shared_ptr<api::Cancelable> ScheduledExecutor::Schedule(
       runnable();
     }
   };
-  std::optional<FakeClock*> fake_clock =
-      MediumEnvironment::Instance().GetSimulatedClock();
-  if (fake_clock.has_value()) {
-    absl::Time trigger_time = (*fake_clock)->Now() + delay;
+  if (MediumEnvironment::Instance()
+          .GetEnvironmentConfig()
+          .use_simulated_clock) {
+    absl::Time trigger_time = MediumEnvironment::Instance().Now() + delay;
     absl::MutexLock lock(mutex_);
     tasks_.insert(std::pair<absl::Time, std::unique_ptr<Runnable>>(
         trigger_time, std::make_unique<Runnable>(std::move(task))));
@@ -110,15 +102,12 @@ std::shared_ptr<api::Cancelable> ScheduledExecutor::Schedule(
 }
 
 void ScheduledExecutor::RunReadyTasks() {
-  std::optional<FakeClock*> fake_clock =
-      MediumEnvironment::Instance().GetSimulatedClock();
   if (executor_.InShutdown()) {
     return;
   }
-  if (!fake_clock.has_value()) {
-    return;
-  }
-  absl::Time current_time = (*fake_clock)->Now();
+  CHECK(
+      MediumEnvironment::Instance().GetEnvironmentConfig().use_simulated_clock);
+  absl::Time current_time = MediumEnvironment::Instance().Now();
   absl::MutexLock lock(mutex_);
   for (auto it = tasks_.begin(); it != tasks_.end();) {
     if (it->first <= current_time) {

internal/platform/implementation/g3/system_clock.cc

@@ -15,26 +15,21 @@
 #include "internal/platform/implementation/system_clock.h"
 
 #include "absl/time/clock.h"
+#include "absl/time/time.h"
 #include "internal/platform/exception.h"
 #include "internal/platform/medium_environment.h"
-#include "internal/test/fake_clock.h"
 
 namespace nearby {
 
 absl::Time SystemClock::ElapsedRealtime() {
-  absl::optional<FakeClock*> fake_clock =
-      MediumEnvironment::Instance().GetSimulatedClock();
-  if (fake_clock.has_value()) {
-    return (*fake_clock)->Now();
-  }
-  return absl::Now();
+  return MediumEnvironment::Instance().Now();
 }
 
 Exception SystemClock::Sleep(absl::Duration duration) {
-  absl::optional<FakeClock*> fake_clock =
-      MediumEnvironment::Instance().GetSimulatedClock();
-  if (fake_clock.has_value()) {
-    (*fake_clock)->FastForward(duration);
+  if (MediumEnvironment::Instance()
+          .GetEnvironmentConfig()
+          .use_simulated_clock) {
+    MediumEnvironment::Instance().FastForward(duration);
   } else {
     absl::SleepFor(duration);
   }

internal/platform/medium_environment.cc

@@ -16,6 +16,7 @@
 
 #include <atomic>
 #include <cstdint>
+#include <functional>
 #include <memory>
 #include <optional>
 #include <string>
@@ -27,6 +28,7 @@
 #include "absl/status/status.h"
 #include "absl/strings/str_format.h"
 #include "absl/strings/string_view.h"
+#include "absl/time/clock.h"
 #include "absl/time/time.h"
 #include "internal/platform/borrowable.h"
 #include "internal/platform/byte_array.h"
@@ -46,6 +48,7 @@
 #include "internal/platform/nsd_service_info.h"
 #include "internal/platform/prng.h"
 #include "internal/platform/runnable.h"
+#include "internal/platform/service_address.h"
 #include "internal/platform/uuid.h"
 #include "internal/platform/wifi_credential.h"
 #include "internal/test/fake_clock.h"
@@ -69,10 +72,14 @@ MediumEnvironment& MediumEnvironment::Instance() {
 void MediumEnvironment::Start(EnvironmentConfig config) {
   if (!enabled_.exchange(true)) {
     LOG(INFO) << "MediumEnvironment::Start()";
-    config_ = std::move(config);
-    if (config_.use_simulated_clock) {
+    {
       MutexLock lock(&mutex_);
-      simulated_clock_ = std::make_unique<FakeClock>();
+      config_ = std::move(config);
+      if (config_.use_simulated_clock) {
+        simulated_clock_ = std::make_shared<FakeClock>();
+      } else {
+        simulated_clock_.reset();
+      }
     }
     Reset();
   }
@@ -82,8 +89,8 @@ void MediumEnvironment::Stop() {
   if (enabled_.exchange(false)) {
     LOG(INFO) << "MediumEnvironment::Stop()";
     Sync(false);
+    MutexLock lock(&mutex_);
     if (config_.use_simulated_clock) {
-      MutexLock lock(&mutex_);
       simulated_clock_.reset();
     }
     config_ = {};
@@ -132,7 +139,8 @@ void MediumEnvironment::Sync(bool enable_notifications) {
   LOG(INFO) << "MediumEnvironment::Sync(): done [count=" << count << "]";
 }
 
-const EnvironmentConfig& MediumEnvironment::GetEnvironmentConfig() {
+EnvironmentConfig MediumEnvironment::GetEnvironmentConfig() {
+  MutexLock lock(&mutex_);
   return config_;
 }
 
@@ -1150,12 +1158,43 @@ void MediumEnvironment::SetFeatureFlags(const FeatureFlags::Flags& flags) {
   FeatureFlags::GetMutableInstanceForTesting().SetFlags(flags);
 }
 
-std::optional<FakeClock*> MediumEnvironment::GetSimulatedClock() {
+absl::Time MediumEnvironment::Now() {
   MutexLock lock(&mutex_);
   if (simulated_clock_) {
-    return std::optional<FakeClock*>(simulated_clock_.get());
+    return simulated_clock_->Now();
+  }
+  return absl::Now();
+}
+
+// If simulated_clock_ is valid, it will be advanced by the given duration.
+// If simulated_clock_ is not valid, this method will do nothing.
+void MediumEnvironment::FastForward(absl::Duration duration) {
+  std::shared_ptr<FakeClock> sim_clock;
+  {
+    MutexLock lock(&mutex_);
+    if (simulated_clock_) {
+      sim_clock = simulated_clock_;
+    }
+  }
+  if (sim_clock) {
+    // Mutex is unlocked before calling FastForward to prevent deadlocks.
+    sim_clock->FastForward(duration);
+  }
+}
+
+void MediumEnvironment::AddSimulatedClockObserver(
+    const std::string& name, std::function<void()> observer) {
+  MutexLock lock(&mutex_);
+  if (simulated_clock_) {
+    simulated_clock_->AddObserver(name, std::move(observer));
+  }
+}
+
+void MediumEnvironment::RemoveSimulatedClockObserver(const std::string& name) {
+  MutexLock lock(&mutex_);
+  if (simulated_clock_) {
+    simulated_clock_->RemoveObserver(name);
   }
-  return std::nullopt;
 }
 
 void MediumEnvironment::RegisterGattServer(

internal/platform/medium_environment.h

@@ -17,6 +17,7 @@
 
 #include <atomic>
 #include <cstdint>
+#include <functional>
 #include <memory>
 #include <optional>
 #include <string>
@@ -162,7 +163,7 @@ class MediumEnvironment {
   // Returns a Bluetooth Device object matching given mac address to nullptr.
   api::BluetoothDevice* FindBluetoothDevice(MacAddress mac_address);
 
-  const EnvironmentConfig& GetEnvironmentConfig();
+  EnvironmentConfig GetEnvironmentConfig();
 #ifndef NO_WEBRTC
   // Registers |message_callback| to receive messages sent to device with id
   // |self_id|, and |complete_callback| to notify when signaling is complete.
@@ -334,7 +335,11 @@ class MediumEnvironment {
 
   void SetFeatureFlags(const FeatureFlags::Flags& flags);
 
-  std::optional<FakeClock*> GetSimulatedClock();
+  absl::Time Now();
+  void FastForward(absl::Duration duration);
+  void AddSimulatedClockObserver(const std::string& name,
+                                 std::function<void()> observer);
+  void RemoveSimulatedClockObserver(const std::string& name);
 
   api::ble::BleMedium* FindBleMedium(api::ble::BlePeripheral::UniqueId id);
 
@@ -516,7 +521,7 @@ class MediumEnvironment {
 
   bool use_valid_peer_connection_ = true;
   absl::Duration peer_connection_latency_ = absl::ZeroDuration();
-  std::unique_ptr<FakeClock> simulated_clock_ ABSL_GUARDED_BY(mutex_);
+  std::shared_ptr<FakeClock> simulated_clock_ ABSL_GUARDED_BY(mutex_);
   ObserverList<api::BluetoothClassicMedium::Observer> observers_;
   bool ble_extended_advertisements_available_ = false;
 };

internal/platform/scheduled_executor_test.cc

@@ -25,13 +25,12 @@
 #include "internal/platform/cancelable.h"
 #include "internal/platform/count_down_latch.h"
 #include "internal/platform/medium_environment.h"
-#include "internal/test/fake_clock.h"
 
 namespace nearby {
 
 // kShortDelay must be significant enough to guarantee that OS under heavy load
 // should be able to execute the non-blocking test paths within this time.
-absl::Duration kShortDelay = absl::Milliseconds(100);
+absl::Duration kShortDelay = absl::Milliseconds(200);
 
 // kLongDelay must be long enough to make sure that under OS under heavy load
 // will let kShortDelay fire and jobs scheduled before the kLongDelay fires.
@@ -212,8 +211,6 @@ TEST(ScheduledExecutorTest, ExecuteDuringShutdownFails) {
 
 TEST(ScheduledExecutorTest, SimulatedClockCanSchedule) {
   MediumEnvironment::Instance().Start({.use_simulated_clock = true});
-  FakeClock* fake_clock =
-      MediumEnvironment::Instance().GetSimulatedClock().value();
   ScheduledExecutor executor;
   std::atomic_int value = 0;
   CountDownLatch first_task_latch(1);
@@ -235,24 +232,23 @@ TEST(ScheduledExecutorTest, SimulatedClockCanSchedule) {
       },
       kShortDelay);
   EXPECT_EQ(value, 0);
-  fake_clock->FastForward(kShortDelay - absl::Milliseconds(1));
+  MediumEnvironment::Instance().FastForward(kShortDelay -
+                                            absl::Milliseconds(1));
   EXPECT_EQ(value, 0);
-  fake_clock->FastForward(absl::Milliseconds(1));
+  MediumEnvironment::Instance().FastForward(absl::Milliseconds(1));
   second_task_latch.Await();
   EXPECT_EQ(value, 1);
-  fake_clock->FastForward(kLongDelay - kShortDelay);
+  MediumEnvironment::Instance().FastForward(kLongDelay - kShortDelay);
   first_task_latch.Await();
   EXPECT_EQ(value, 5);
   // Very long sleep to make sure that the sleep is truly simulated.
-  fake_clock->FastForward(absl::Minutes(30));
+  MediumEnvironment::Instance().FastForward(absl::Minutes(30));
   MediumEnvironment::Instance().Stop();
 }
 
 TEST(ScheduledExecutorTest,
      DestroyExecutorWithSimulatedClockIgnoresPendingTasks) {
   MediumEnvironment::Instance().Start({.use_simulated_clock = true});
-  FakeClock* fake_clock =
-      MediumEnvironment::Instance().GetSimulatedClock().value();
   {
     ScheduledExecutor executor;
     executor.Schedule(
@@ -262,7 +258,7 @@ TEST(ScheduledExecutorTest,
         },
         kShortDelay);
   }
-  fake_clock->FastForward(absl::Minutes(30));
+  MediumEnvironment::Instance().FastForward(absl::Minutes(30));
   MediumEnvironment::Instance().Stop();
 }
 
@@ -403,8 +399,6 @@ TEST(ScheduledExecutorTest, CanCancelOneOfTwoRepeatedTasks) {
 
 TEST(ScheduledExecutorTest, SimulatedClockCanScheduleRepeatedly) {
   MediumEnvironment::Instance().Start({.use_simulated_clock = true});
-  FakeClock* fake_clock =
-      MediumEnvironment::Instance().GetSimulatedClock().value();
   ScheduledExecutor executor;
   std::atomic_int value = 0;
   std::atomic_int i = 0;
@@ -419,11 +413,12 @@ TEST(ScheduledExecutorTest, SimulatedClockCanScheduleRepeatedly) {
 
   EXPECT_EQ(value, 0);
   // Advance to just before the first execution.
-  fake_clock->FastForward(kShortDelay - absl::Milliseconds(1));
+  MediumEnvironment::Instance().FastForward(kShortDelay -
+                                            absl::Milliseconds(1));
   EXPECT_EQ(value, 0);
 
   // Advance past the first execution.
-  fake_clock->FastForward(absl::Milliseconds(1));
+  MediumEnvironment::Instance().FastForward(absl::Milliseconds(1));
   latch[0].Await(absl::Seconds(1));
   EXPECT_EQ(value, 1);
 
@@ -431,11 +426,12 @@ TEST(ScheduledExecutorTest, SimulatedClockCanScheduleRepeatedly) {
   absl::SleepFor(kShortDelay);
 
   // Advance to just before the second execution.
-  fake_clock->FastForward(kShortDelay - absl::Milliseconds(1));
+  MediumEnvironment::Instance().FastForward(kShortDelay -
+                                            absl::Milliseconds(1));
   EXPECT_EQ(value, 1);
 
   // Advance past the second execution.
-  fake_clock->FastForward(absl::Milliseconds(1));
+  MediumEnvironment::Instance().FastForward(absl::Milliseconds(1));
   latch[1].Await(absl::Seconds(1));
   EXPECT_EQ(value, 2);
 
@@ -443,7 +439,7 @@ TEST(ScheduledExecutorTest, SimulatedClockCanScheduleRepeatedly) {
   cancelable.Cancel();
 
   // Advance a long time and make sure it doesn't run again.
-  fake_clock->FastForward(kLongDelay * 5);
+  MediumEnvironment::Instance().FastForward(kLongDelay * 5);
   EXPECT_EQ(value, 2);
 
   MediumEnvironment::Instance().Stop();

internal/test/fake_clock.cc

@@ -19,6 +19,10 @@
 #include <utility>
 #include <vector>
 
+#include "absl/strings/string_view.h"
+#include "absl/synchronization/mutex.h"
+#include "absl/time/time.h"
+
 namespace nearby {
 
 FakeClock::~FakeClock() {

internal/test/fake_clock.h

@@ -53,6 +53,7 @@ class FakeClock : public Clock {
   absl::flat_hash_map<std::string, std::function<void()>> observers_
       ABSL_GUARDED_BY(mutex_);
 };
+
 }  // namespace nearby
 
 #endif  // THIRD_PARTY_NEARBY_INTERNAL_TEST_FAKE_CLOCK_H_

internal/test/fake_clock_test.cc

@@ -14,9 +14,8 @@
 
 #include "internal/test/fake_clock.h"
 
-#include "gmock/gmock.h"
-#include "protobuf-matchers/protocol-buffer-matchers.h"
 #include "gtest/gtest.h"
+#include "absl/time/time.h"
 
 namespace nearby {
 namespace {