Domain Reliability: Don't crash on shutdown with uploads pending

components/domain_reliability/uploader_unittest.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/domain_reliability/uploader.h"
 
 #include <stddef.h>
 
 #include <memory>
 #include <string>
@@ skipping 93 matching lines @@
   }
 
   net::UploadDataStream* upload_stream_;
   scoped_refptr<net::IOBufferWithSize> upload_buffer_;
   std::string upload_data_;
   MockUploadResult result_;
 };
 
 class UploadInterceptor : public net::URLRequestInterceptor {
  public:
-  UploadInterceptor() : last_upload_depth_(-1) {}
+  UploadInterceptor() : request_count_(0), last_upload_depth_(-1) {}
 
   ~UploadInterceptor() override {
     EXPECT_TRUE(results_.empty());
   }
 
   net::URLRequestJob* MaybeInterceptRequest(
       net::URLRequest* request,
       net::NetworkDelegate* delegate) const override {
     EXPECT_FALSE(results_.empty());
     MockUploadResult result = results_.front();
     results_.pop_front();
 
     last_upload_depth_ =
         DomainReliabilityUploader::GetURLRequestUploadDepth(*request);
 
+    ++request_count_;
+
     return new UploadMockURLRequestJob(request, delegate, result);
   }
 
   void ExpectRequestAndReturnError(int net_error) {
     MockUploadResult result;
     result.net_error = net_error;
     result.response_code = -1;
     results_.push_back(result);
   }
 
   void ExpectRequestAndReturnResponseCode(int response_code) {
     MockUploadResult result;
     result.net_error = net::OK;
     result.response_code = response_code;
     results_.push_back(result);
   }
 
   void ExpectRequestAndReturnResponseCodeAndHeaders(
       int response_code,
       const char* headers) {
     MockUploadResult result;
     result.net_error = net::OK;
     result.response_code = response_code;
     result.response_headers = new net::HttpResponseHeaders(
         net::HttpUtil::AssembleRawHeaders(headers, strlen(headers)));
     results_.push_back(result);
   }
 
+  int request_count() const { return request_count_; }
   int last_upload_depth() const { return last_upload_depth_; }
 
  private:
   mutable std::list<MockUploadResult> results_;
+  mutable int request_count_;
   mutable int last_upload_depth_;
 };
 
 class TestUploadCallback {
  public:
   TestUploadCallback() : called_count_(0u) {}
 
   DomainReliabilityUploader::UploadCallback callback() {
     return base::Bind(&TestUploadCallback::OnCalled, base::Unretained(this));
   }
@@ skipping 35 matching lines @@
 
  private:
   base::MessageLoopForIO message_loop_;
   scoped_refptr<net::TestURLRequestContextGetter> url_request_context_getter_;
   UploadInterceptor* interceptor_;
   MockTime time_;
   std::unique_ptr<DomainReliabilityUploader> uploader_;
 };
 
 TEST_F(DomainReliabilityUploaderTest, Null) {
+  uploader()->Shutdown();
 }
 
 TEST_F(DomainReliabilityUploaderTest, SuccessfulUpload) {
   interceptor()->ExpectRequestAndReturnResponseCode(200);
 
   TestUploadCallback c;
   uploader()->UploadReport("{}", 0, GURL(kUploadURL), c.callback());
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, c.called_count());
   EXPECT_TRUE(c.last_result().is_success());
+
+  uploader()->Shutdown();
 }
 
 TEST_F(DomainReliabilityUploaderTest, NetworkErrorUpload) {
   interceptor()->ExpectRequestAndReturnError(net::ERR_CONNECTION_REFUSED);
 
   TestUploadCallback c;
   uploader()->UploadReport("{}", 0, GURL(kUploadURL), c.callback());
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, c.called_count());
   EXPECT_TRUE(c.last_result().is_failure());
+
+  uploader()->Shutdown();
 }
 
 TEST_F(DomainReliabilityUploaderTest, ServerErrorUpload) {
   interceptor()->ExpectRequestAndReturnResponseCode(500);
 
   TestUploadCallback c;
   uploader()->UploadReport("{}", 0, GURL(kUploadURL), c.callback());
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, c.called_count());
   EXPECT_TRUE(c.last_result().is_failure());
+
+  uploader()->Shutdown();
 }
 
 TEST_F(DomainReliabilityUploaderTest, RetryAfterUpload) {
   interceptor()->ExpectRequestAndReturnResponseCodeAndHeaders(
       503,
       "HTTP/1.1 503 Ugh\nRetry-After: 3600\n\n");
 
   TestUploadCallback c;
   uploader()->UploadReport("{}", 0, GURL(kUploadURL), c.callback());
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, c.called_count());
   EXPECT_TRUE(c.last_result().is_retry_after());
+
+  uploader()->Shutdown();
 }
 
 TEST_F(DomainReliabilityUploaderTest, UploadDepth1) {
   interceptor()->ExpectRequestAndReturnResponseCode(200);
 
   TestUploadCallback c;
   uploader()->UploadReport("{}", 0, GURL(kUploadURL), c.callback());
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, c.called_count());
 
   EXPECT_EQ(1, interceptor()->last_upload_depth());
+
+  uploader()->Shutdown();
 }
 
 TEST_F(DomainReliabilityUploaderTest, UploadDepth2) {
   interceptor()->ExpectRequestAndReturnResponseCode(200);
 
   TestUploadCallback c;
   uploader()->UploadReport("{}", 1, GURL(kUploadURL), c.callback());
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, c.called_count());
 
   EXPECT_EQ(2, interceptor()->last_upload_depth());
+
+  uploader()->Shutdown();
+}
+
+TEST_F(DomainReliabilityUploaderTest, NoUploadAfterShutdown) {

[mmenke, 2016/12/14 19:12:17]

Also have a test that checks that requests are cancelled at shutdown? (Create a
hanging request wait for request_count_ to reach 1, call Shutdown(), and then
call url_request_context_getter_->GetContext()->AssertNoURLRequests());

Since this test uses its own URLRequestJob subclass, count even add a way to
check that the created job was torn down instead.  Fine with either approach.

[Julia Tuttle, 2016/12/15 21:53:00]

Done.

+  uploader()->Shutdown();
+
+  TestUploadCallback c;
+  uploader()->UploadReport("{}", 1, GURL(kUploadURL), c.callback());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1u, c.called_count());
+
+  EXPECT_EQ(0, interceptor()->request_count());
 }
 
 }  // namespace
 }  // namespace domain_reliability