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

components/domain_reliability/uploader_unittest.cc

Index: components/domain_reliability/uploader_unittest.cc
diff --git a/components/domain_reliability/uploader_unittest.cc b/components/domain_reliability/uploader_unittest.cc
index 4aeb6ddc024ff3c280fb617614f901cc7c39c5d6..51277fc84becffd9f821a2fa3fd2d0c5b563f737 100644
--- a/components/domain_reliability/uploader_unittest.cc
+++ b/components/domain_reliability/uploader_unittest.cc
@@ -111,7 +111,7 @@ class UploadMockURLRequestJob : public net::URLRequestJob {
 
 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());
@@ -127,6 +127,8 @@ class UploadInterceptor : public net::URLRequestInterceptor {
     last_upload_depth_ =
         DomainReliabilityUploader::GetURLRequestUploadDepth(*request);
 
+    ++request_count_;
+
     return new UploadMockURLRequestJob(request, delegate, result);
   }
 
@@ -155,10 +157,12 @@ class UploadInterceptor : public net::URLRequestInterceptor {
     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_;
 };
 
@@ -214,6 +218,7 @@ class DomainReliabilityUploaderTest : public testing::Test {
 };
 
 TEST_F(DomainReliabilityUploaderTest, Null) {
+  uploader()->Shutdown();
 }
 
 TEST_F(DomainReliabilityUploaderTest, SuccessfulUpload) {
@@ -224,6 +229,8 @@ TEST_F(DomainReliabilityUploaderTest, SuccessfulUpload) {
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, c.called_count());
   EXPECT_TRUE(c.last_result().is_success());
+
+  uploader()->Shutdown();
 }
 
 TEST_F(DomainReliabilityUploaderTest, NetworkErrorUpload) {
@@ -234,6 +241,8 @@ TEST_F(DomainReliabilityUploaderTest, NetworkErrorUpload) {
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, c.called_count());
   EXPECT_TRUE(c.last_result().is_failure());
+
+  uploader()->Shutdown();
 }
 
 TEST_F(DomainReliabilityUploaderTest, ServerErrorUpload) {
@@ -244,6 +253,8 @@ TEST_F(DomainReliabilityUploaderTest, ServerErrorUpload) {
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, c.called_count());
   EXPECT_TRUE(c.last_result().is_failure());
+
+  uploader()->Shutdown();
 }
 
 TEST_F(DomainReliabilityUploaderTest, RetryAfterUpload) {
@@ -256,6 +267,8 @@ TEST_F(DomainReliabilityUploaderTest, RetryAfterUpload) {
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, c.called_count());
   EXPECT_TRUE(c.last_result().is_retry_after());
+
+  uploader()->Shutdown();
 }
 
 TEST_F(DomainReliabilityUploaderTest, UploadDepth1) {
@@ -267,6 +280,8 @@ TEST_F(DomainReliabilityUploaderTest, UploadDepth1) {
   EXPECT_EQ(1u, c.called_count());
 
   EXPECT_EQ(1, interceptor()->last_upload_depth());
+
+  uploader()->Shutdown();
 }
 
 TEST_F(DomainReliabilityUploaderTest, UploadDepth2) {
@@ -278,6 +293,19 @@ TEST_F(DomainReliabilityUploaderTest, UploadDepth2) {
   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