[Nearby] Remove public certificates when they expire

Use an expiration scheduler to notify the certificate manager when a
public certificate has expired. Then, remove all expired public
certificates from storage.

Fixed: b/166117022
Change-Id: I28b019d805518e9f9520ccd427840f75bd5bee1a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2377070
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805294}

chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager_impl.h

@@ -104,35 +104,61 @@ class NearbyShareCertificateManagerImpl : public NearbyShareCertificateManager {
   void OnStart() override;
   void OnStop() override;
 
-  // Removes expired privates certificates, ensures that at least
-  // kNearbyShareNumPrivateCertificates are present for each visibility with
-  // contiguous validity periods, and uploads any changes to the Nearby Share
-  // server.
-  base::Time NextPrivateCertificateExpirationTime();
+  // Used by the private certificate expiration scheduler to determine the next
+  // private certificate expiration time. Returns base::Time::Min() if
+  // certificates are missing. This function never returns base::nullopt.
+  base::Optional<base::Time> NextPrivateCertificateExpirationTime();
+
+  // Used by the public certificate expiration scheduler to determine the next
+  // public certificate expiration time. Returns base::nullopt if no public
+  // certificates are present, and no expiration event is scheduled.
+  base::Optional<base::Time> NextPublicCertificateExpirationTime();
+
+  // Invoked by the private certificate expiration scheduler when an expired
+  // private certificate needs to be removed or if no private certificates exist
+  // yet. New certificate(s) will be created, and an upload to the Nearby Share
+  // server will be requested.
   void OnPrivateCertificateExpiration();
+
   void FinishPrivateCertificateRefresh(
       std::vector<NearbySharePrivateCertificate> new_certs,
       base::flat_map<NearbyShareVisibility, size_t> num_valid_certs,
       base::flat_map<NearbyShareVisibility, base::Time> latest_not_after,
       scoped_refptr<device::BluetoothAdapter> bluetooth_adapter);
+
+  // Invoked by the certificate upload scheduler when private certificates need
+  // to be converted to public certificates and uploaded to the Nearby Share
+  // server.
   void OnLocalDeviceCertificateUploadRequest();
+
   void OnLocalDeviceCertificateUploadFinished(bool success);
 
+  // Invoked by the public certificate expiration scheduler when an expired
+  // public certificate needs to be removed from storage.
+  void OnPublicCertificateExpiration();
+
+  void OnExpiredPublicCertificatesRemoved(bool success);
+
+  // Invoked by the certificate download scheduler when the public certificates
+  // from trusted contacts need to be downloaded from Nearby Share server via
+  // the ListPublicCertificates RPC.
   void OnDownloadPublicCertificatesRequest(
       base::Optional<std::string> page_token,
       size_t page_number,
       size_t certificate_count);
-  void OnRpcSuccess(
+
+  void OnListPublicCertificatesSuccess(
       size_t page_number,
       size_t certificate_count,
       const nearbyshare::proto::ListPublicCertificatesResponse& response);
-  void OnRpcFailure(size_t page_number,
-                    size_t certificate_count,
-                    NearbyShareHttpError error);
-  void OnPublicCertificatesAdded(base::Optional<std::string> page_token,
-                                 size_t page_number,
-                                 size_t certificate_count,
-                                 bool success);
+  void OnListPublicCertificatesFailure(size_t page_number,
+                                       size_t certificate_count,
+                                       NearbyShareHttpError error);
+  void OnPublicCertificatesAddedToStorage(
+      base::Optional<std::string> page_token,
+      size_t page_number,
+      size_t certificate_count,
+      bool success);
   void FinishDownloadPublicCertificates(bool success,
                                         NearbyShareHttpResult http_result,
                                         size_t page_number,
@@ -142,12 +168,14 @@ class NearbyShareCertificateManagerImpl : public NearbyShareCertificateManager {
   PrefService* pref_service_ = nullptr;
   NearbyShareClientFactory* client_factory_ = nullptr;
   base::Clock* clock_ = nullptr;
+  std::unique_ptr<NearbyShareCertificateStorage> certificate_storage_;
   std::unique_ptr<NearbyShareScheduler>
       private_certificate_expiration_scheduler_;
+  std::unique_ptr<NearbyShareScheduler>
+      public_certificate_expiration_scheduler_;
   std::unique_ptr<NearbyShareScheduler>
       upload_local_device_certificates_scheduler_;
   std::unique_ptr<NearbyShareScheduler> download_public_certificates_scheduler_;
-  std::unique_ptr<NearbyShareCertificateStorage> cert_store_;
   std::unique_ptr<NearbyShareClient> client_;
   base::WeakPtrFactory<NearbyShareCertificateManagerImpl> weak_ptr_factory_{
       this};

chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager_impl_unittest.cc

@@ -63,12 +63,20 @@ class NearbyShareCertificateManagerImplTest
         &clock_);
     cert_man_->AddObserver(this);
 
+    cert_store_ = cert_store_factory_.instances().back();
+
     private_cert_exp_scheduler_ =
         scheduler_factory_.pref_name_to_expiration_instance()
             .find(
                 prefs::
                     kNearbySharingSchedulerPrivateCertificateExpirationPrefName)
             ->second.fake_scheduler;
+    public_cert_exp_scheduler_ =
+        scheduler_factory_.pref_name_to_expiration_instance()
+            .find(
+                prefs::
+                    kNearbySharingSchedulerPublicCertificateExpirationPrefName)
+            ->second.fake_scheduler;
     upload_scheduler_ =
         scheduler_factory_.pref_name_to_on_demand_instance()
             .find(
@@ -80,7 +88,6 @@ class NearbyShareCertificateManagerImplTest
             .find(prefs::
                       kNearbySharingSchedulerDownloadPublicCertificatesPrefName)
             ->second.fake_scheduler;
-    cert_store_ = cert_store_factory_.instances().back();
 
     PopulatePrivateCertificates();
     PopulatePublicCertificates();
@@ -230,6 +237,8 @@ class NearbyShareCertificateManagerImplTest
 
     size_t initial_num_notifications =
         num_public_certs_downloaded_notifications_;
+    size_t initial_num_public_cert_exp_reschedules =
+        public_cert_exp_scheduler_->num_reschedule_calls();
     std::string page_token;
     for (size_t page_number = 0; page_number < num_pages; ++page_number) {
       bool last_page = page_number == num_pages - 1;
@@ -258,10 +267,13 @@ class NearbyShareCertificateManagerImplTest
     }
     ASSERT_EQ(download_scheduler_->handled_results().size(),
               prev_num_results + 1);
+
     bool success = !rpc_fail && !store_fail;
     EXPECT_EQ(download_scheduler_->handled_results().back(), success);
     EXPECT_EQ(initial_num_notifications + (success ? 1u : 0u),
               num_public_certs_downloaded_notifications_);
+    EXPECT_EQ(initial_num_public_cert_exp_reschedules + (success ? 1u : 0u),
+              public_cert_exp_scheduler_->num_reschedule_calls());
   }
 
   void CheckRpcRequest(
@@ -334,6 +346,7 @@ class NearbyShareCertificateManagerImplTest
 
   FakeNearbyShareCertificateStorage* cert_store_;
   FakeNearbyShareScheduler* private_cert_exp_scheduler_;
+  FakeNearbyShareScheduler* public_cert_exp_scheduler_;
   FakeNearbyShareScheduler* upload_scheduler_;
   FakeNearbyShareScheduler* download_scheduler_;
   std::string bluetooth_mac_address_ = kTestUnparsedBluetoothMacAddress;
@@ -592,3 +605,45 @@ TEST_F(NearbyShareCertificateManagerImplTest,
 
   VerifyPrivateCertificates(/*expected_metadata=*/metadata);
 }
+
+TEST_F(NearbyShareCertificateManagerImplTest,
+       RemoveExpiredPublicCertificates_Success) {
+  clock_.SetNow(t0);
+  cert_man_->Start();
+
+  // The public certificate expiration scheduler notifies the certificate
+  // manager that a public certificate has expired.
+  EXPECT_EQ(0u, cert_store_->remove_expired_public_certificates_calls().size());
+  public_cert_exp_scheduler_->InvokeRequestCallback();
+  EXPECT_EQ(1u, cert_store_->remove_expired_public_certificates_calls().size());
+  EXPECT_EQ(t0,
+            cert_store_->remove_expired_public_certificates_calls().back().now);
+
+  EXPECT_EQ(0u, public_cert_exp_scheduler_->handled_results().size());
+  std::move(
+      cert_store_->remove_expired_public_certificates_calls().back().callback)
+      .Run(/*success=*/true);
+  EXPECT_EQ(1u, public_cert_exp_scheduler_->handled_results().size());
+  EXPECT_TRUE(public_cert_exp_scheduler_->handled_results().back());
+}
+
+TEST_F(NearbyShareCertificateManagerImplTest,
+       RemoveExpiredPublicCertificates_Failue) {
+  clock_.SetNow(t0);
+  cert_man_->Start();
+
+  // The public certificate expiration scheduler notifies the certificate
+  // manager that a public certificate has expired.
+  EXPECT_EQ(0u, cert_store_->remove_expired_public_certificates_calls().size());
+  public_cert_exp_scheduler_->InvokeRequestCallback();
+  EXPECT_EQ(1u, cert_store_->remove_expired_public_certificates_calls().size());
+  EXPECT_EQ(t0,
+            cert_store_->remove_expired_public_certificates_calls().back().now);
+
+  EXPECT_EQ(0u, public_cert_exp_scheduler_->handled_results().size());
+  std::move(
+      cert_store_->remove_expired_public_certificates_calls().back().callback)
+      .Run(/*success=*/false);
+  EXPECT_EQ(1u, public_cert_exp_scheduler_->handled_results().size());
+  EXPECT_FALSE(public_cert_exp_scheduler_->handled_results().back());
+}

chrome/browser/nearby_sharing/certificates/nearby_share_certificate_storage_impl.cc

@@ -18,6 +18,7 @@
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "base/util/values/values_util.h"
 #include "base/values.h"
+#include "chrome/browser/nearby_sharing/certificates/common.h"
 #include "chrome/browser/nearby_sharing/certificates/constants.h"
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_private_certificate.h"
 #include "chrome/browser/nearby_sharing/common/nearby_share_prefs.h"
@@ -575,8 +576,16 @@ void NearbyShareCertificateStorageImpl::RemoveExpiredPublicCertificates(
 
   auto ids_to_remove = std::make_unique<std::vector<std::string>>();
   for (const auto& pair : public_certificate_expirations_) {
-    if (pair.second > now)
+    // Because the list is sorted by expiration time, break as soon as we
+    // encounter an unexpired certificate. Apply a tolerance when evaluating
+    // whether the certificate is expired to account for clock skew between
+    // devices. This conforms this the GmsCore implementation.
+    if (!IsNearbyShareCertificateExpired(
+            now,
+            /*not_after=*/pair.second,
+            /*use_public_certificate_tolerance=*/true)) {
       break;
+    }
 
     ids_to_remove->emplace_back(pair.first);
   }

chrome/browser/nearby_sharing/certificates/nearby_share_certificate_storage_impl_unittest.cc

@@ -438,7 +438,12 @@ TEST_F(NearbyShareCertificateStorageImplTest, RemoveExpiredPublicCertificates) {
     expiration_times.emplace_back(TimestampToTime(pair.second.end_time()));
   }
   std::sort(expiration_times.begin(), expiration_times.end());
-  base::Time now = expiration_times[1];
+
+  // The current time exceeds the expiration times of the first two certificates
+  // even accounting for the expiration time tolerance applied to public
+  // certificates to account for clock skew.
+  base::Time now = expiration_times[1] +
+                   kNearbySharePublicCertificateValidityBoundOffsetTolerance;
 
   bool succeeded = false;
   cert_store_->RemoveExpiredPublicCertificates(
@@ -450,7 +455,8 @@ TEST_F(NearbyShareCertificateStorageImplTest, RemoveExpiredPublicCertificates) {
   ASSERT_TRUE(succeeded);
   ASSERT_EQ(1u, db_entries_.size());
   for (const auto& pair : db_entries_) {
-    EXPECT_LE(now, TimestampToTime(pair.second.end_time()));
+    EXPECT_LE(now - kNearbySharePublicCertificateValidityBoundOffsetTolerance,
+              TimestampToTime(pair.second.end_time()));
   }
 }
 

chrome/browser/nearby_sharing/common/nearby_share_prefs.cc

@@ -42,6 +42,8 @@ const char kNearbySharingSchedulerDownloadPublicCertificatesPrefName[] =
     "nearby_sharing.scheduler.download_public_certificates";
 const char kNearbySharingSchedulerPrivateCertificateExpirationPrefName[] =
     "nearby_sharing.scheduler.private_certificate_expiration";
+const char kNearbySharingSchedulerPublicCertificateExpirationPrefName[] =
+    "nearby_sharing.scheduler.public_certificate_expiration";
 const char kNearbySharingSchedulerUploadDeviceNamePrefName[] =
     "nearby_sharing.scheduler.upload_device_name";
 const char kNearbySharingSchedulerUploadLocalDeviceCertificatesPrefName[] =
@@ -88,6 +90,8 @@ void RegisterNearbySharingPrefs(PrefRegistrySimple* registry) {
       prefs::kNearbySharingSchedulerDownloadPublicCertificatesPrefName);
   registry->RegisterDictionaryPref(
       prefs::kNearbySharingSchedulerPrivateCertificateExpirationPrefName);
+  registry->RegisterDictionaryPref(
+      prefs::kNearbySharingSchedulerPublicCertificateExpirationPrefName);
   registry->RegisterDictionaryPref(
       prefs::kNearbySharingSchedulerUploadDeviceNamePrefName);
   registry->RegisterDictionaryPref(

chrome/browser/nearby_sharing/common/nearby_share_prefs.h

@@ -26,6 +26,7 @@ extern const char kNearbySharingSchedulerContactUploadPrefName[];
 extern const char kNearbySharingSchedulerDownloadDeviceDataPrefName[];
 extern const char kNearbySharingSchedulerDownloadPublicCertificatesPrefName[];
 extern const char kNearbySharingSchedulerPrivateCertificateExpirationPrefName[];
+extern const char kNearbySharingSchedulerPublicCertificateExpirationPrefName[];
 extern const char kNearbySharingSchedulerUploadDeviceNamePrefName[];
 extern const char
     kNearbySharingSchedulerUploadLocalDeviceCertificatesPrefName[];

chrome/browser/nearby_sharing/scheduling/fake_nearby_share_scheduler.cc

@@ -19,6 +19,10 @@ void FakeNearbyShareScheduler::HandleResult(bool success) {
   handled_results_.push_back(success);
 }
 
+void FakeNearbyShareScheduler::Reschedule() {
+  ++num_reschedule_calls_;
+}
+
 base::Optional<base::Time> FakeNearbyShareScheduler::GetLastSuccessTime()
     const {
   return last_success_time_;

chrome/browser/nearby_sharing/scheduling/fake_nearby_share_scheduler.h

@@ -23,6 +23,7 @@ class FakeNearbyShareScheduler : public NearbyShareScheduler {
   // NearbyShareScheduler:
   void MakeImmediateRequest() override;
   void HandleResult(bool success) override;
+  void Reschedule() override;
   base::Optional<base::Time> GetLastSuccessTime() const override;
   base::Optional<base::TimeDelta> GetTimeUntilNextRequest() const override;
   bool IsWaitingForResult() const override;
@@ -36,6 +37,7 @@ class FakeNearbyShareScheduler : public NearbyShareScheduler {
   void InvokeRequestCallback();
 
   size_t num_immediate_requests() const { return num_immediate_requests_; }
+  size_t num_reschedule_calls() const { return num_reschedule_calls_; }
   const std::vector<bool>& handled_results() const { return handled_results_; }
 
  private:
@@ -45,6 +47,7 @@ class FakeNearbyShareScheduler : public NearbyShareScheduler {
 
   bool can_invoke_request_callback_ = false;
   size_t num_immediate_requests_ = 0;
+  size_t num_reschedule_calls_ = 0;
   std::vector<bool> handled_results_;
   base::Optional<base::Time> last_success_time_;
   base::Optional<base::TimeDelta> time_until_next_request_;

chrome/browser/nearby_sharing/scheduling/fake_nearby_share_scheduler_factory.cc

@@ -21,8 +21,8 @@ FakeNearbyShareSchedulerFactory::~FakeNearbyShareSchedulerFactory() = default;
 
 std::unique_ptr<NearbyShareScheduler>
 FakeNearbyShareSchedulerFactory::CreateExpirationSchedulerInstance(
-    NearbyShareExpirationScheduler::ExpirationTimeCallback
-        expiration_time_callback,
+    NearbyShareExpirationScheduler::ExpirationTimeFunctor
+        expiration_time_functor,
     bool retry_failures,
     bool require_connectivity,
     const std::string& pref_name,
@@ -30,7 +30,7 @@ FakeNearbyShareSchedulerFactory::CreateExpirationSchedulerInstance(
     NearbyShareScheduler::OnRequestCallback on_request_callback,
     const base::Clock* clock) {
   ExpirationInstance instance;
-  instance.expiration_time_callback = std::move(expiration_time_callback);
+  instance.expiration_time_functor = std::move(expiration_time_functor);
   instance.retry_failures = retry_failures;
   instance.require_connectivity = require_connectivity;
   instance.pref_service = pref_service;

chrome/browser/nearby_sharing/scheduling/fake_nearby_share_scheduler_factory.h

@@ -31,8 +31,8 @@ class FakeNearbyShareSchedulerFactory : public NearbyShareSchedulerFactory {
     ~ExpirationInstance();
 
     FakeNearbyShareScheduler* fake_scheduler = nullptr;
-    NearbyShareExpirationScheduler::ExpirationTimeCallback
-        expiration_time_callback;
+    NearbyShareExpirationScheduler::ExpirationTimeFunctor
+        expiration_time_functor;
     bool retry_failures;
     bool require_connectivity;
     PrefService* pref_service = nullptr;
@@ -77,8 +77,8 @@ class FakeNearbyShareSchedulerFactory : public NearbyShareSchedulerFactory {
  private:
   // NearbyShareSchedulerFactory:
   std::unique_ptr<NearbyShareScheduler> CreateExpirationSchedulerInstance(
-      NearbyShareExpirationScheduler::ExpirationTimeCallback
-          expiration_time_callback,
+      NearbyShareExpirationScheduler::ExpirationTimeFunctor
+          expiration_time_functor,
       bool retry_failures,
       bool require_connectivity,
       const std::string& pref_name,

chrome/browser/nearby_sharing/scheduling/nearby_share_expiration_scheduler.cc

@@ -8,7 +8,7 @@
 #include <utility>
 
 NearbyShareExpirationScheduler::NearbyShareExpirationScheduler(
-    ExpirationTimeCallback expiration_time_callback,
+    ExpirationTimeFunctor expiration_time_functor,
     bool retry_failures,
     bool require_connectivity,
     const std::string& pref_name,
@@ -21,13 +21,16 @@ NearbyShareExpirationScheduler::NearbyShareExpirationScheduler(
                                pref_service,
                                std::move(on_request_callback),
                                clock),
-      expiration_time_callback_(std::move(expiration_time_callback)) {}
+      expiration_time_functor_(std::move(expiration_time_functor)) {}
 
 NearbyShareExpirationScheduler::~NearbyShareExpirationScheduler() = default;
 
 base::Optional<base::TimeDelta>
 NearbyShareExpirationScheduler::TimeUntilRecurringRequest(
     base::Time now) const {
-  return std::max(base::TimeDelta::FromSeconds(0),
-                  expiration_time_callback_.Run() - now);
+  base::Optional<base::Time> expiration_time = expiration_time_functor_.Run();
+  if (!expiration_time)
+    return base::nullopt;
+
+  return std::max(base::TimeDelta::FromSeconds(0), *expiration_time - now);
 }

chrome/browser/nearby_sharing/scheduling/nearby_share_expiration_scheduler.h

@@ -16,19 +16,19 @@
 // expiration time provided by the owner.
 class NearbyShareExpirationScheduler : public NearbyShareSchedulerBase {
  public:
-  using ExpirationTimeCallback = base::RepeatingCallback<base::Time()>;
+  using ExpirationTimeFunctor =
+      base::RepeatingCallback<base::Optional<base::Time>()>;
 
-  // |expiration_time_callback|: A function provided by the owner that returns
+  // |expiration_time_functor|: A function provided by the owner that returns
   //     the next expiration time.
   // See NearbyShareSchedulerBase for a description of other inputs.
-  NearbyShareExpirationScheduler(
-      ExpirationTimeCallback expiration_time_callback,
-      bool retry_failures,
-      bool require_connectivity,
-      const std::string& pref_name,
-      PrefService* pref_service,
-      OnRequestCallback on_request_callback,
-      const base::Clock* clock);
+  NearbyShareExpirationScheduler(ExpirationTimeFunctor expiration_time_functor,
+                                 bool retry_failures,
+                                 bool require_connectivity,
+                                 const std::string& pref_name,
+                                 PrefService* pref_service,
+                                 OnRequestCallback on_request_callback,
+                                 const base::Clock* clock);
 
   ~NearbyShareExpirationScheduler() override;
 
@@ -36,7 +36,7 @@ class NearbyShareExpirationScheduler : public NearbyShareSchedulerBase {
   base::Optional<base::TimeDelta> TimeUntilRecurringRequest(
       base::Time now) const override;
 
-  ExpirationTimeCallback expiration_time_callback_;
+  ExpirationTimeFunctor expiration_time_functor_;
 };
 
 #endif  // CHROME_BROWSER_NEARBY_SHARING_SCHEDULING_NEARBY_SHARE_EXPIRATION_SCHEDULER_H_

chrome/browser/nearby_sharing/scheduling/nearby_share_expiration_scheduler_unittest.cc

@@ -5,6 +5,7 @@
 #include <memory>
 
 #include "base/bind.h"
+#include "base/optional.h"
 #include "base/test/task_environment.h"
 #include "base/time/clock.h"
 #include "base/time/time.h"
@@ -39,13 +40,15 @@ class NearbyShareExpirationSchedulerTest : public ::testing::Test {
 
     scheduler_ = std::make_unique<NearbyShareExpirationScheduler>(
         base::BindRepeating(
-            &NearbyShareExpirationSchedulerTest::TestExpirationTimeCallback,
+            &NearbyShareExpirationSchedulerTest::TestExpirationTimeFunctor,
             base::Unretained(this)),
         /*retry_failures=*/true, /*require_connectivity=*/true, kTestPrefName,
         &pref_service_, base::DoNothing(), task_environment_.GetMockClock());
   }
 
-  base::Time TestExpirationTimeCallback() { return expiration_time_; }
+  base::Optional<base::Time> TestExpirationTimeFunctor() {
+    return expiration_time_;
+  }
 
   base::Time Now() const { return task_environment_.GetMockClock()->Now(); }
 
@@ -54,13 +57,12 @@ class NearbyShareExpirationSchedulerTest : public ::testing::Test {
     task_environment_.FastForwardBy(delta);
   }
 
-  base::Time expiration_time() const { return expiration_time_; }
+  base::Optional<base::Time> expiration_time_;
   NearbyShareScheduler* scheduler() { return scheduler_.get(); }
 
  private:
   base::test::SingleThreadTaskEnvironment task_environment_{
       base::test::TaskEnvironment::TimeSource::MOCK_TIME};
-  base::Time expiration_time_;
   TestingPrefServiceSimple pref_service_;
   std::unique_ptr<NearbyShareScheduler> scheduler_;
 };
@@ -69,8 +71,31 @@ TEST_F(NearbyShareExpirationSchedulerTest, ExpirationRequest) {
   scheduler()->Start();
 
   // Let 5 minutes elapse since the start time just to make sure the time to the
-  // next request only depend on the expiration time and the current time.
+  // next request only depends on the expiration time and the current time.
   FastForward(base::TimeDelta::FromMinutes(5));
 
-  EXPECT_EQ(expiration_time() - Now(), scheduler()->GetTimeUntilNextRequest());
+  EXPECT_EQ(*expiration_time_ - Now(), scheduler()->GetTimeUntilNextRequest());
+}
+
+TEST_F(NearbyShareExpirationSchedulerTest, Reschedule) {
+  scheduler()->Start();
+  FastForward(base::TimeDelta::FromMinutes(5));
+
+  base::TimeDelta initial_expected_time_until_next_request =
+      *expiration_time_ - Now();
+  EXPECT_EQ(initial_expected_time_until_next_request,
+            scheduler()->GetTimeUntilNextRequest());
+
+  // The expiration time suddenly changes.
+  expiration_time_ = *expiration_time_ + base::TimeDelta::FromDays(2);
+  scheduler()->Reschedule();
+  EXPECT_EQ(
+      initial_expected_time_until_next_request + base::TimeDelta::FromDays(2),
+      scheduler()->GetTimeUntilNextRequest());
+}
+
+TEST_F(NearbyShareExpirationSchedulerTest, NullExpirationTime) {
+  expiration_time_.reset();
+  scheduler()->Start();
+  EXPECT_EQ(base::nullopt, scheduler()->GetTimeUntilNextRequest());
 }

chrome/browser/nearby_sharing/scheduling/nearby_share_scheduler.h

@@ -30,10 +30,16 @@ class NearbyShareScheduler {
   // Makes a request that runs as soon as possible.
   virtual void MakeImmediateRequest() = 0;
 
-  // Processes the result of the previous request. Method to be called by
-  // the owner with the request is finished.
+  // Processes the result of the previous request. Method to be called by the
+  // owner when the request is finished. The timer for the next request is
+  // automatically scheduled.
   virtual void HandleResult(bool success) = 0;
 
+  // Recomputes the time until the next request, using GetTimeUntilNextRequest()
+  // as the source of truth. This method is essentially idempotent. NOTE: This
+  // method should rarely need to be called.
+  virtual void Reschedule() = 0;
+
   // Returns the time of the last known successful request. If no request has
   // succeeded, base::nullopt is returned.
   virtual base::Optional<base::Time> GetLastSuccessTime() const = 0;
@@ -42,10 +48,8 @@ class NearbyShareScheduler {
   // there is no request scheduled.
   virtual base::Optional<base::TimeDelta> GetTimeUntilNextRequest() const = 0;
 
-  // Returns true after the relevant delegate has been alerted of a request
-  // |type| but before the delegate has returned the result to the scheduler.
-  // In other words, between ...Delegate::On...Requested() and
-  // HandleResult().
+  // Returns true after the |callback_| has been alerted of a request but before
+  // HandleResult() is invoked.
   virtual bool IsWaitingForResult() const = 0;
 
   // The number of times the current request type has failed.

chrome/browser/nearby_sharing/scheduling/nearby_share_scheduler_base.cc

@@ -60,7 +60,7 @@ NearbyShareSchedulerBase::~NearbyShareSchedulerBase() {
 void NearbyShareSchedulerBase::MakeImmediateRequest() {
   timer_.Stop();
   SetHasPendingImmediateRequest(true);
-  SetTimer();
+  Reschedule();
 }
 
 void NearbyShareSchedulerBase::HandleResult(bool success) {
@@ -75,7 +75,22 @@ void NearbyShareSchedulerBase::HandleResult(bool success) {
   }
 
   SetIsWaitingForResult(false);
-  SetTimer();
+  Reschedule();
+}
+
+void NearbyShareSchedulerBase::Reschedule() {
+  if (!is_running())
+    return;
+
+  timer_.Stop();
+
+  base::Optional<base::TimeDelta> delay = GetTimeUntilNextRequest();
+  if (!delay)
+    return;
+
+  timer_.Start(FROM_HERE, *delay,
+               base::BindOnce(&NearbyShareSchedulerBase::OnTimerFired,
+                              base::Unretained(this)));
 }
 
 base::Optional<base::Time> NearbyShareSchedulerBase::GetLastSuccessTime()
@@ -123,7 +138,7 @@ size_t NearbyShareSchedulerBase::GetNumConsecutiveFailures() const {
 }
 
 void NearbyShareSchedulerBase::OnStart() {
-  SetTimer();
+  Reschedule();
 }
 
 void NearbyShareSchedulerBase::OnStop() {
@@ -135,7 +150,7 @@ void NearbyShareSchedulerBase::OnConnectionChanged(
   if (content::GetNetworkConnectionTracker()->IsOffline())
     return;
 
-  SetTimer();
+  Reschedule();
 }
 
 base::Optional<base::Time> NearbyShareSchedulerBase::GetLastAttemptTime()
@@ -215,21 +230,6 @@ base::Optional<base::TimeDelta> NearbyShareSchedulerBase::TimeUntilRetry(
   return std::max(kZeroTimeDelta, delay - time_elapsed_since_last_attempt);
 }
 
-void NearbyShareSchedulerBase::SetTimer() {
-  if (!is_running())
-    return;
-
-  timer_.Stop();
-
-  base::Optional<base::TimeDelta> delay = GetTimeUntilNextRequest();
-  if (!delay)
-    return;
-
-  timer_.Start(FROM_HERE, *delay,
-               base::BindOnce(&NearbyShareSchedulerBase::OnTimerFired,
-                              base::Unretained(this)));
-}
-
 void NearbyShareSchedulerBase::OnTimerFired() {
   DCHECK(is_running());
   if (require_connectivity_ &&

chrome/browser/nearby_sharing/scheduling/nearby_share_scheduler_base.h

@@ -65,6 +65,7 @@ class NearbyShareSchedulerBase
   // NearbyShareScheduler:
   void MakeImmediateRequest() override;
   void HandleResult(bool success) override;
+  void Reschedule() override;
   base::Optional<base::Time> GetLastSuccessTime() const override;
   base::Optional<base::TimeDelta> GetTimeUntilNextRequest() const override;
   bool IsWaitingForResult() const override;
@@ -95,10 +96,6 @@ class NearbyShareSchedulerBase
   // enabled for the scheduler.
   base::Optional<base::TimeDelta> TimeUntilRetry(base::Time now) const;
 
-  // Schedules/Reschedules request, using GetTimeUntilNextRequest() as the
-  // source of truth. This method is essentially idempotent.
-  void SetTimer();
-
   // Notifies the owner that a request is ready. Early returns if not online and
   // the scheduler requires connectivity; the attempt is rescheduled when
   // connectivity is restored.

chrome/browser/nearby_sharing/scheduling/nearby_share_scheduler_factory.cc

@@ -16,8 +16,8 @@ NearbyShareSchedulerFactory* NearbyShareSchedulerFactory::test_factory_ =
 // static
 std::unique_ptr<NearbyShareScheduler>
 NearbyShareSchedulerFactory::CreateExpirationScheduler(
-    NearbyShareExpirationScheduler::ExpirationTimeCallback
-        expiration_time_callback,
+    NearbyShareExpirationScheduler::ExpirationTimeFunctor
+        expiration_time_functor,
     bool retry_failures,
     bool require_connectivity,
     const std::string& pref_name,
@@ -26,13 +26,13 @@ NearbyShareSchedulerFactory::CreateExpirationScheduler(
     const base::Clock* clock) {
   if (test_factory_) {
     return test_factory_->CreateExpirationSchedulerInstance(
-        std::move(expiration_time_callback), retry_failures,
+        std::move(expiration_time_functor), retry_failures,
         require_connectivity, pref_name, pref_service,
         std::move(on_request_callback), clock);
   }
 
   return std::make_unique<NearbyShareExpirationScheduler>(
-      std::move(expiration_time_callback), retry_failures, require_connectivity,
+      std::move(expiration_time_functor), retry_failures, require_connectivity,
       pref_name, pref_service, std::move(on_request_callback), clock);
 }
 

chrome/browser/nearby_sharing/scheduling/nearby_share_scheduler_factory.h

@@ -21,8 +21,8 @@ class PrefService;
 class NearbyShareSchedulerFactory {
  public:
   static std::unique_ptr<NearbyShareScheduler> CreateExpirationScheduler(
-      NearbyShareExpirationScheduler::ExpirationTimeCallback
-          expiration_time_callback,
+      NearbyShareExpirationScheduler::ExpirationTimeFunctor
+          expiration_time_functor,
       bool retry_failures,
       bool require_connectivity,
       const std::string& pref_name,
@@ -54,8 +54,8 @@ class NearbyShareSchedulerFactory {
 
   virtual std::unique_ptr<NearbyShareScheduler>
   CreateExpirationSchedulerInstance(
-      NearbyShareExpirationScheduler::ExpirationTimeCallback
-          expiration_time_callback,
+      NearbyShareExpirationScheduler::ExpirationTimeFunctor
+          expiration_time_functor,
       bool retry_failures,
       bool require_connectivity,
       const std::string& pref_name,