Fix invalid WeakPtr usage in cert reporting test utils.

Fix invalid WeakPtr usage in cert reporting test utils.
CertReportJobInterceptor::MaybeInterceptRequest was posting IO thread-
affine WeakPtrs onto the UI thread, which is an invalid use of
WeakPtr.

This CL replaces the use of WeakPtr with Unretained(). WeakPtr seems
overkill for test code, as the test cases may block & join to ensure
proper destruction ordering.

R=vakh@chromium.org
CC=​​wez@chromium.org
TBR=vakh@chromium.org

Bug: 729716
Change-Id: I99a07e1098d3a8341dbc2043ca2b335b9d97ef60
Reviewed-on: https://chromium-review.googlesource.com/527732
Commit-Queue: Kevin Marshall <kmarshall@chromium.org>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487637}

chrome/browser/safe_browsing/certificate_reporting_service_test_utils.cc

@@ -71,6 +71,7 @@ namespace certificate_reporting_test_utils {
 
 RequestObserver::RequestObserver()
     : num_events_to_wait_for_(0u), num_received_events_(0u) {}
+
 RequestObserver::~RequestObserver() {}
 
 void RequestObserver::Wait(unsigned int num_events_to_wait_for) {
@@ -220,10 +221,11 @@ CertReportJobInterceptor::CertReportJobInterceptor(
     ReportSendingResult expected_report_result,
     const uint8_t* server_private_key)
     : expected_report_result_(expected_report_result),
-      server_private_key_(server_private_key),
-      weak_factory_(this) {}
+      server_private_key_(server_private_key) {}
 
-CertReportJobInterceptor::~CertReportJobInterceptor() {}
+CertReportJobInterceptor::~CertReportJobInterceptor() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+}
 
 net::URLRequestJob* CertReportJobInterceptor::MaybeInterceptRequest(
     net::URLRequest* request,
@@ -232,11 +234,7 @@ net::URLRequestJob* CertReportJobInterceptor::MaybeInterceptRequest(
 
   const std::string serialized_report =
       GetReportContents(request, server_private_key_);
-  content::BrowserThread::PostTask(
-      content::BrowserThread::UI, FROM_HERE,
-      base::BindOnce(&CertReportJobInterceptor::RequestCreated,
-                     weak_factory_.GetWeakPtr(), serialized_report,
-                     expected_report_result_));
+  RequestCreated(serialized_report, expected_report_result_);
 
   if (expected_report_result_ == REPORTS_FAIL) {
     return new DelayableCertReportURLRequestJob(
@@ -270,7 +268,7 @@ void CertReportJobInterceptor::SetFailureMode(
   content::BrowserThread::PostTask(
       content::BrowserThread::IO, FROM_HERE,
       base::BindOnce(&CertReportJobInterceptor::SetFailureModeOnIOThread,
-                     weak_factory_.GetWeakPtr(), expected_report_result));
+                     base::Unretained(this), expected_report_result));
 }
 
 void CertReportJobInterceptor::Resume() {
@@ -307,8 +305,11 @@ void CertReportJobInterceptor::ResumeOnIOThread() {
 void CertReportJobInterceptor::RequestCreated(
     const std::string& serialized_report,
     ReportSendingResult expected_report_result) const {
-  request_created_observer_.OnRequest(serialized_report,
-                                      expected_report_result);
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::BindOnce(&RequestObserver::OnRequest,
+                     base::Unretained(&request_created_observer_),
+                     serialized_report, expected_report_result));
 }
 
 void CertReportJobInterceptor::RequestDestructed(

chrome/browser/safe_browsing/certificate_reporting_service_test_utils.h

@@ -137,6 +137,8 @@ class DelayableCertReportURLRequestJob : public net::URLRequestJob {
 
 // A job interceptor that returns a failed, succesful or delayed request job.
 // Used to simulate report uploads that fail, succeed or hang.
+// The caller is responsible for guaranteeing that |this| is kept alive for
+// all posted tasks and URLRequestJob objects.
 class CertReportJobInterceptor : public net::URLRequestInterceptor {
  public:
   CertReportJobInterceptor(ReportSendingResult expected_report_result,
@@ -177,9 +179,7 @@ class CertReportJobInterceptor : public net::URLRequestInterceptor {
   mutable RequestObserver request_created_observer_;
   mutable RequestObserver request_destroyed_observer_;
 
-  mutable base::WeakPtr<DelayableCertReportURLRequestJob> delayed_request_ =
-      nullptr;
-  mutable base::WeakPtrFactory<CertReportJobInterceptor> weak_factory_;
+  mutable base::WeakPtr<DelayableCertReportURLRequestJob> delayed_request_;
 
   DISALLOW_COPY_AND_ASSIGN(CertReportJobInterceptor);
 };