Fix flaky CertificateReportingService browser tests.

chrome/browser/safe_browsing/certificate_reporting_service_browsertest.cc

Index: chrome/browser/safe_browsing/certificate_reporting_service_browsertest.cc
diff --git a/chrome/browser/safe_browsing/certificate_reporting_service_browsertest.cc b/chrome/browser/safe_browsing/certificate_reporting_service_browsertest.cc
index 89efeca0311f3df3995ac4ff74dc9bebd0059ffb..eb5249ac471cb845652166b3305ab739b5275dce 100644
--- a/chrome/browser/safe_browsing/certificate_reporting_service_browsertest.cc
+++ b/chrome/browser/safe_browsing/certificate_reporting_service_browsertest.cc
@@ -36,6 +36,7 @@
 #include "url/scheme_host_port.h"
 
 using certificate_reporting_test_utils::CertificateReportingServiceTestHelper;
+using certificate_reporting_test_utils::CertificateReportingServiceObserver;
 using certificate_reporting_test_utils::ReportExpectation;
 
 namespace {
@@ -53,24 +54,16 @@ namespace safe_browsing {
 
 // These tests check the whole mechanism to send and queue invalid certificate
 // reports. Each test triggers reports by visiting broken SSL pages. The reports
-// succeed, fail or hang indefinitely. The test waits for the URL requests
-// corresponding to the reports to to be created via the URL request
-// interceptor. When reports are expected to succeed or fail, test teardown
-// checks that there are no in-flight or pending reports in the
-// CertificateReportingService queue. When a report is to be delayed, a single
-// in-flight report is expected in CertificateReportingService. Since the actual
-// URL requests for reports are sent from the IO thread, the tests wait for the
-// IO thread to finish before checking the expected report counts.
-//
-// Note that these browser tests differ from the unit tests in how they check
-// expected reports: Unit tests create a network delegate and observe the
-// destruction of the URL requests, whereas browser tests wait for the URL
-// requests to be created instead.
+// succeed, fail or hang indefinitely:
+// - If a report is expected to fail or succeed, the test waits for the
+//   corresponding URL request jobs to be destroyed.
+// - If a report is expected to hang, the test waits for the corresponding URL
+//   request job to be created. Only after resuming the hung request job the
+//   test waits for the request to be destroyed.
 class CertificateReportingServiceBrowserTest : public InProcessBrowserTest {
  public:
   CertificateReportingServiceBrowserTest()
-      : https_server_(net::EmbeddedTestServer::TYPE_HTTPS),
-        expect_delayed_report_on_teardown_(false) {}
+      : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {}
 
   void SetUpOnMainThread() override {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -80,19 +73,25 @@ class CertificateReportingServiceBrowserTest : public InProcessBrowserTest {
     https_server_.ServeFilesFromSourceDirectory("chrome/test/data");
     ASSERT_TRUE(https_server_.Start());
 
-    test_helper_.SetUpInterceptor();
+    test_helper()->SetUpInterceptor();
 
     CertificateReportingServiceFactory::GetInstance()
         ->SetReportEncryptionParamsForTesting(
-            test_helper_.server_public_key(),
-            test_helper_.server_public_key_version());
+            test_helper()->server_public_key(),
+            test_helper()->server_public_key_version());
+    CertificateReportingServiceFactory::GetInstance()
+        ->SetServiceResetCallbackForTesting(
+            base::Bind(&CertificateReportingServiceObserver::OnServiceReset,
+                       base::Unretained(&service_observer_)));
     InProcessBrowserTest::SetUpOnMainThread();
   }
 
   void TearDownOnMainThread() override {
-    CheckExpectedReportCounts(expect_delayed_report_on_teardown_);
+    test_helper()->ExpectNoRequests(service());
     content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE,
                                      base::Bind(&CleanUpOnIOThread));
+    EXPECT_GE(num_expected_failed_report_, 0)
+        << "Don't forget to set expected failed report count.";
     // Check the histogram as the last thing. This makes sure no in-flight
     // report is missed.
     if (num_expected_failed_report_ != 0) {
@@ -138,16 +137,10 @@ class CertificateReportingServiceBrowserTest : public InProcessBrowserTest {
     // Navigate away from the interstitial to trigger report upload.
     ui_test_utils::NavigateToURL(browser(), GURL("about:blank"));
     content::WaitForInterstitialDetach(contents);
-    WaitForIOThread();
   }
 
   void SendPendingReports() { service()->SendPending(); }
 
-  // Checks that there are no outstanding reports.
-  // If |expect_delayed_report_on_teardown| is true, expects a single delayed
-  // report.
-  void CheckNoReports() { CheckExpectedReportCounts(false); }
-
   // Changes opt-in status and waits for the cert reporting service to reset.
   // Can only be used after the service is initialized. When changing the
   // value at the beginning of a test,
@@ -159,119 +152,52 @@ class CertificateReportingServiceBrowserTest : public InProcessBrowserTest {
   // a task to the IO thread to reset the service. Waiting for the IO thread
   // ensures that the service is reset before returning from this method.
   void ChangeOptInAndWait(certificate_reporting_test_utils::OptIn opt_in) {
+    service_observer_.Clear();
     certificate_reporting_test_utils::SetCertReportingOptIn(browser(), opt_in);
-    WaitForIOThread();
+    service_observer_.WaitForReset();
   }
 
   // Same as ChangeOptInAndWait, but enables/disables SafeBrowsing instead.
   void ToggleSafeBrowsingAndWaitForServiceReset(bool safebrowsing_enabled) {
+    service_observer_.Clear();
     browser()->profile()->GetPrefs()->SetBoolean(prefs::kSafeBrowsingEnabled,
                                                  safebrowsing_enabled);
-    WaitForIOThread();
-  }
-
-  void ShutdownServiceAndWait() {
-    service()->Shutdown();
-    WaitForIOThread();
-  }
-
-  // Waits for a number of URL requests to be created for the reports in
-  // |expectation| and checks that the reports in |expectation| matches the
-  // reports observed by URL request interceptor.
-  void WaitForReports(const ReportExpectation& expectation) {
-    test_helper_.interceptor()->WaitForReports(expectation.num_reports());
-    std::set<std::string> expected_hostnames;
-    CheckReports(expectation.successful_reports,
-                 test_helper_.interceptor()->successful_reports());
-    CheckReports(expectation.failed_reports,
-                 test_helper_.interceptor()->failed_reports());
-    CheckReports(expectation.delayed_reports,
-                 test_helper_.interceptor()->delayed_reports());
-    test_helper_.interceptor()->ClearObservedReports();
-  }
-
-  // Resumes the delayed request and waits for the resume task to complete which
-  // in turn means the response starts.
-  void ResumeDelayedRequestAndWait() {
-    base::RunLoop run_loop;
-    test_helper_.ResumeDelayedRequest(run_loop.QuitClosure());
-    run_loop.Run();
-  }
-
-  // Tells the test to expect a delayed report during test teardown. If not set,
-  // the tests expect no in-flight reports during teardown.
-  void SetExpectDelayedReportOnTeardown() {
-    expect_delayed_report_on_teardown_ = true;
+    service_observer_.WaitForReset();
   }
 
-  void SetExpectedHistogramCountOnTeardown(
-      unsigned int num_expected_failed_report) {
+  void SetExpectedHistogramCountOnTeardown(int num_expected_failed_report) {
     num_expected_failed_report_ = num_expected_failed_report;
   }
 
- private:
   CertificateReportingService* service() const {
     return CertificateReportingServiceFactory::GetForBrowserContext(
         browser()->profile());
   }
 
-  // Waits for pending tasks on the IO thread to complete.
-  void WaitForIOThread() {
-    scoped_refptr<base::ThreadTestHelper> io_helper(new base::ThreadTestHelper(
-        content::BrowserThread::GetTaskRunnerForThread(
-            content::BrowserThread::IO)
-            .get()));
-    ASSERT_TRUE(io_helper->Run());
-  }
-
+ private:
   // Checks that the serialized reports in |received_reports| have the same
   // hostnames as |expected_hostnames|.
   void CheckReports(const std::set<std::string>& expected_hostnames,
-                    const std::set<std::string>& received_reports) {
+                    const std::set<std::string>& received_reports,
+                    const std::string type) {
     std::set<std::string> received_hostnames;
     for (const std::string& serialized_report : received_reports) {
       certificate_reporting::ErrorReport report;
       ASSERT_TRUE(report.InitializeFromString(serialized_report));
       received_hostnames.insert(report.hostname());
     }
-    EXPECT_EQ(expected_hostnames, received_hostnames);
-  }
-
-  // Checks that there are no remaining successful and failed reports observed
-  // by the interceptor. If |expect_delayed_report| is true, expects a single
-  // delayed report. Otherwise, expects no delayed reports.
-  void CheckExpectedReportCounts(bool expect_delayed_report) {
-    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-    // Wait for the IO thread to ensure that any report-sending tasks previously
-    // posted to the IO thread hav run (and thus been observed by the
-    // interceptor).
-    WaitForIOThread();
-    EXPECT_TRUE(test_helper_.interceptor()->successful_reports().empty());
-    EXPECT_TRUE(test_helper_.interceptor()->failed_reports().empty());
-
-    if (expect_delayed_report)
-      EXPECT_EQ(1u, test_helper_.interceptor()->delayed_reports().size());
-    else
-      EXPECT_TRUE(test_helper_.interceptor()->delayed_reports().empty());
-
-    if (service()->GetReporterForTesting()) {
-      // Reporter can be null if reporting is disabled.
-      size_t num_inflight_reports = expect_delayed_report ? 1u : 0u;
-      EXPECT_EQ(num_inflight_reports,
-                service()
-                    ->GetReporterForTesting()
-                    ->inflight_report_count_for_testing());
-    }
+    EXPECT_EQ(expected_hostnames, received_hostnames) << type
+                                                      << " comparison failed";
   }
 
   net::EmbeddedTestServer https_server_;
-  // If true, the test will expect to see a delayed report during test teardown.
-  bool expect_delayed_report_on_teardown_ = false;
 
-  unsigned int num_expected_failed_report_ = 0;
+  int num_expected_failed_report_ = -1;
 
   CertificateReportingServiceTestHelper test_helper_;
 
+  CertificateReportingServiceObserver service_observer_;
+
   base::HistogramTester histogram_tester_;
 
   DISALLOW_COPY_AND_ASSIGN(CertificateReportingServiceBrowserTest);
@@ -307,7 +233,8 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
 
   // Reporting is opted in, so the report should succeed.
   SendReport("report0");
-  WaitForReports(ReportExpectation::Successful({"report0"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Successful({"report0"}));
 }
 
 // Tests that report send attempts are not cancelled when extended reporting is
@@ -325,11 +252,13 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
 
   // Send a failed report.
   SendReport("report0");
-  WaitForReports(ReportExpectation::Failed({"report0"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Failed({"report0"}));
 
   // Send another failed report.
   SendReport("report1");
-  WaitForReports(ReportExpectation::Failed({"report1"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Failed({"report1"}));
 
   // Let all report uploads complete successfully now.
   test_helper()->SetFailureMode(certificate_reporting_test_utils::
@@ -337,12 +266,14 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
 
   // Send another report. This time the report should be successfully sent.
   SendReport("report2");
-  WaitForReports(ReportExpectation::Successful({"report2"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Successful({"report2"}));
 
   // Send all pending reports. The two previously failed reports should have
   // been queued, and now be sent successfully.
   SendPendingReports();
-  WaitForReports(ReportExpectation::Successful({"report0", "report1"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Successful({"report0", "report1"}));
 
   // Try sending pending reports again. Since there is no pending report,
   // nothing should be sent this time. If any report is sent, test teardown
@@ -364,7 +295,8 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
 
   // Send a failed report.
   SendReport("report0");
-  WaitForReports(ReportExpectation::Failed({"report0"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Failed({"report0"}));
 
   // Disable reporting. This should clear all pending reports.
   ChangeOptInAndWait(
@@ -388,7 +320,7 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
 
   // Send attempt should be cancelled since reporting is opted out.
   SendReport("no-report");
-  CheckNoReports();
+  test_helper()->ExpectNoRequests(service());
 
   // Enable reporting.
   ChangeOptInAndWait(
@@ -396,7 +328,8 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
 
   // A failed report should be observed.
   SendReport("report0");
-  WaitForReports(ReportExpectation::Failed({"report0"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Failed({"report0"}));
 
   // Disable reporting. This should reset the reporting service and
   // clear all pending reports.
@@ -405,7 +338,7 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
 
   // Report should be cancelled since reporting is opted out.
   SendReport("report1");
-  CheckNoReports();
+  test_helper()->ExpectNoRequests(service());
 
   // Send pending reports. Nothing should be sent since there aren't any
   // pending reports. If any report is sent, test teardown will catch it.
@@ -424,27 +357,30 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
   test_helper()->SetFailureMode(
       certificate_reporting_test_utils::ReportSendingResult::REPORTS_FAIL);
 
-  // Send a failed report.
+  // Send a delayed report.
   SendReport("report0");
-  WaitForReports(ReportExpectation::Failed({"report0"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Failed({"report0"}));
 
   // Disable SafeBrowsing. This should clear all pending reports.
   ToggleSafeBrowsingAndWaitForServiceReset(false);
 
   // Send pending reports. No reports should be observed.
   SendPendingReports();
-  CheckNoReports();
+  test_helper()->ExpectNoRequests(service());
 
   // Re-enable SafeBrowsing and trigger another report which will be queued.
   ToggleSafeBrowsingAndWaitForServiceReset(true);
   SendReport("report1");
-  WaitForReports(ReportExpectation::Failed({"report1"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Failed({"report1"}));
 
   // Queued report should now be successfully sent.
   test_helper()->SetFailureMode(certificate_reporting_test_utils::
                                     ReportSendingResult::REPORTS_SUCCESSFUL);
   SendPendingReports();
-  WaitForReports(ReportExpectation::Successful({"report1"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Successful({"report1"}));
 }
 
 // CertificateReportingService should ignore reports older than the report TTL.
@@ -469,12 +405,14 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
 
   // Send a failed report.
   SendReport("report0");
-  WaitForReports(ReportExpectation::Failed({"report0"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Failed({"report0"}));
 
   // Advance the clock a bit and trigger another failed report.
   clock->Advance(base::TimeDelta::FromHours(5));
   SendReport("report1");
-  WaitForReports(ReportExpectation::Failed({"report1"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Failed({"report1"}));
 
   // Advance the clock to 20 hours, putting it 25 hours ahead of the reference
   // time. This makes report0 older than 24 hours. report1 is now 20 hours.
@@ -483,11 +421,13 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
   // Send pending reports. report0 should be discarded since it's too old.
   // report1 should be queued again.
   SendPendingReports();
-  WaitForReports(ReportExpectation::Failed({"report1"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Failed({"report1"}));
 
   // Trigger another failed report.
   SendReport("report2");
-  WaitForReports(ReportExpectation::Failed({"report2"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Failed({"report2"}));
 
   // Advance the clock 5 hours. report1 will now be 25 hours old.
   clock->Advance(base::TimeDelta::FromHours(5));
@@ -495,7 +435,8 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
   // Send pending reports. report1 should be discarded since it's too old.
   // report2 should be queued again.
   SendPendingReports();
-  WaitForReports(ReportExpectation::Failed({"report2"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Failed({"report2"}));
 
   // Advance the clock 20 hours again so that report2 is 25 hours old and is
   // older than max age (24 hours).
@@ -531,7 +472,8 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
 
   // Trigger a failed report.
   SendReport("report0");
-  WaitForReports(ReportExpectation::Failed({"report0"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Failed({"report0"}));
 
   // Trigger three more reports within five hours of each other. After this:
   // report0 is 0 hours after reference time (15 hours old).
@@ -547,13 +489,15 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
   clock->Advance(base::TimeDelta::FromHours(5));
   SendReport("report3");
 
-  WaitForReports(ReportExpectation::Failed({"report1", "report2", "report3"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Failed({"report1", "report2", "report3"}));
 
   // Send pending reports. Four reports were generated above, but the service
   // only queues three reports, so report0 should be dropped since it's the
   // oldest.
   SendPendingReports();
-  WaitForReports(ReportExpectation::Failed({"report1", "report2", "report3"}));
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Failed({"report1", "report2", "report3"}));
 
   // Let all reports succeed.
   test_helper()->SetFailureMode(certificate_reporting_test_utils::
@@ -569,26 +513,8 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
   // Send pending reports. Only reports 2 and 3 should be sent, report 1
   // should be ignored because it's too old.
   SendPendingReports();
-  WaitForReports(ReportExpectation::Successful({"report2", "report3"}));
-}
-
-// Resume a delayed report after CertificateReportingService shuts down. Should
-// not crash.
-IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
-                       Delayed_NotResumed_ShouldNotCrash) {
-  SetExpectedHistogramCountOnTeardown(0);
-
-  certificate_reporting_test_utils::SetCertReportingOptIn(
-      browser(), certificate_reporting_test_utils::EXTENDED_REPORTING_OPT_IN);
-  // Let reports hang.
-  test_helper()->SetFailureMode(
-      certificate_reporting_test_utils::ReportSendingResult::REPORTS_DELAY);
-
-  // Navigate to and away from an interstitial to trigger a report. The report
-  // is triggered but hangs, so no error or success callbacks should be called.
-  SendReport("no-report");
-
-  SetExpectDelayedReportOnTeardown();
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Successful({"report2", "report3"}));
 }
 
 IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
@@ -597,18 +523,21 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
 
   certificate_reporting_test_utils::SetCertReportingOptIn(
       browser(), certificate_reporting_test_utils::EXTENDED_REPORTING_OPT_IN);
-  // Let all reports fail.
+  // Let all reports hang.
   test_helper()->SetFailureMode(
       certificate_reporting_test_utils::ReportSendingResult::REPORTS_DELAY);
 
   // Trigger a report that hangs.
   SendReport("report0");
-  WaitForReports(ReportExpectation::Delayed({"report0"}));
+  test_helper()->WaitForRequestsCreated(
+      ReportExpectation::Delayed({"report0"}));
 
   // Resume the report upload. The report upload should successfully complete.
   // The interceptor only observes request creations and not response
   // completions, so there is nothing to observe.
-  ResumeDelayedRequestAndWait();
+  test_helper()->ResumeDelayedRequest();
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Delayed({"report0"}));
 }
 
 // Same as above, but the service is shut down before resuming the delayed
@@ -619,21 +548,20 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest,
 
   certificate_reporting_test_utils::SetCertReportingOptIn(
       browser(), certificate_reporting_test_utils::EXTENDED_REPORTING_OPT_IN);
-  // Let all reports fail.
+  // Let all reports hang.
   test_helper()->SetFailureMode(
       certificate_reporting_test_utils::ReportSendingResult::REPORTS_DELAY);
 
   // Trigger a report that hangs.
   SendReport("report0");
-  WaitForReports(ReportExpectation::Delayed({"report0"}));
-
-  // Shutdown the service. Resuming the delayed request shouldn't crash.
-  ShutdownServiceAndWait();
-
-  // Resume the report upload. The report upload should successfully complete.
-  // The interceptor only observes request creations and not response
-  // completions, so there is nothing to observe.
-  ResumeDelayedRequestAndWait();
+  test_helper()->WaitForRequestsCreated(
+      ReportExpectation::Delayed({"report0"}));
+
+  // Shutdown the service and resume the report upload. Shouldn't crash.
+  service()->Shutdown();
+  test_helper()->ResumeDelayedRequest();
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Delayed({"report0"}));
 }
 
 // Trigger a delayed report, then disable Safebrowsing. Certificate reporting
@@ -643,34 +571,37 @@ IN_PROC_BROWSER_TEST_F(CertificateReportingServiceBrowserTest, Delayed_Reset) {
 
   certificate_reporting_test_utils::SetCertReportingOptIn(
       browser(), certificate_reporting_test_utils::EXTENDED_REPORTING_OPT_IN);
-  // Let all reports fail.
+  // Let all reports hang.
   test_helper()->SetFailureMode(
       certificate_reporting_test_utils::ReportSendingResult::REPORTS_DELAY);
 
   // Trigger a report that hangs.
   SendReport("report0");
-  WaitForReports(ReportExpectation::Delayed({"report0"}));
+  test_helper()->WaitForRequestsCreated(
+      ReportExpectation::Delayed({"report0"}));
 
   // Disable SafeBrowsing. This should clear all pending reports.
   ToggleSafeBrowsingAndWaitForServiceReset(false);
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Delayed({"report0"}));
 
   // Resume delayed report. No response should be observed since all pending
   // reports should be cleared.
-  ResumeDelayedRequestAndWait();
-  CheckNoReports();
+  test_helper()->ResumeDelayedRequest();
+  test_helper()->ExpectNoRequests(service());
 
   // Re-enable SafeBrowsing.
   ToggleSafeBrowsingAndWaitForServiceReset(true);
 
   // Trigger a report that hangs.
   SendReport("report1");
-  WaitForReports(ReportExpectation::Delayed({"report1"}));
+  test_helper()->WaitForRequestsCreated(
+      ReportExpectation::Delayed({"report1"}));
 
-  // Resume delayed report. By the time the runloop is finished, the response
-  // will be complete and CertificateReportingService will process the
-  // error/success callback for the report. There will be no inflight reports
-  // remaining.
-  ResumeDelayedRequestAndWait();
+  // Resume the delayed report and wait for it to complete.
+  test_helper()->ResumeDelayedRequest();
+  test_helper()->WaitForRequestsDestroyed(
+      ReportExpectation::Delayed({"report1"}));
 }
 
 }  // namespace safe_browsing