AU: Fix potential issues with premature destruction of HTTP fetchers.

http_fetcher_unittest.cc

Index: http_fetcher_unittest.cc
diff --git a/http_fetcher_unittest.cc b/http_fetcher_unittest.cc
index 30cde956acc6465b067643c4eae8bd5c39bcfa34..67d7c6ed5b0afd86b7debafdc4f6d9c85d6080eb 100644
--- a/http_fetcher_unittest.cc
+++ b/http_fetcher_unittest.cc
@@ -196,6 +196,9 @@ class HttpFetcherTestDelegate : public HttpFetcherDelegate {
     EXPECT_EQ(200, fetcher->http_response_code());
     g_main_loop_quit(loop_);
   }
+  virtual void TransferTerminated(HttpFetcher* fetcher) {
+    ADD_FAILURE();
+  }
   GMainLoop* loop_;
 };
 
@@ -264,6 +267,9 @@ class PausingHttpFetcherTestDelegate : public HttpFetcherDelegate {
   virtual void TransferComplete(HttpFetcher* fetcher, bool successful) {
     g_main_loop_quit(loop_);
   }
+  virtual void TransferTerminated(HttpFetcher* fetcher) {
+    ADD_FAILURE();
+  }
   void Unpause() {
     CHECK(paused_);
     paused_ = false;
@@ -314,9 +320,17 @@ class AbortingHttpFetcherTestDelegate : public HttpFetcherDelegate {
   virtual void ReceivedBytes(HttpFetcher* fetcher,
                              const char* bytes, int length) {}
   virtual void TransferComplete(HttpFetcher* fetcher, bool successful) {
-    CHECK(false);  // We should never get here
+    ADD_FAILURE(); // We should never get here

[adlr, 2010/11/17 21:33:50]

put space back before //

[petkov, 2010/11/17 21:47:54]

Done.

     g_main_loop_quit(loop_);
   }
+  virtual void TransferTerminated(HttpFetcher* fetcher) {
+    CHECK(fetcher == fetcher_.get());
+    CHECK(!once_);

[adlr, 2010/11/17 21:33:50]

use EXPECT rather than CHECK? Unless that doesn't work here. EXPECT seems to
work more places than ASSERT

[petkov, 2010/11/17 21:47:54]

Done.

+    CHECK(callback_once_);
+    callback_once_ = false;
+    // |fetcher| can be destroyed during this callback.
+    fetcher_.reset(NULL);
+ }
   void TerminateTransfer() {
     CHECK(once_);
     once_ = false;
@@ -326,7 +340,8 @@ class AbortingHttpFetcherTestDelegate : public HttpFetcherDelegate {
     g_main_loop_quit(loop_);
   }
   bool once_;
-  HttpFetcher* fetcher_;
+  bool callback_once_;
+  scoped_ptr<HttpFetcher> fetcher_;
   GMainLoop* loop_;
 };
 
@@ -347,11 +362,11 @@ TYPED_TEST(HttpFetcherTest, AbortTest) {
   GMainLoop* loop = g_main_loop_new(g_main_context_default(), FALSE);
   {
     AbortingHttpFetcherTestDelegate delegate;
-    scoped_ptr<HttpFetcher> fetcher(this->NewLargeFetcher());
+    delegate.fetcher_.reset(this->NewLargeFetcher());
     delegate.once_ = true;
+    delegate.callback_once_ = true;
     delegate.loop_ = loop;
-    delegate.fetcher_ = fetcher.get();
-    fetcher->set_delegate(&delegate);
+    delegate.fetcher_->set_delegate(&delegate);
 
     typename TestFixture::HttpServer server;
     this->IgnoreServerAborting(&server);
@@ -361,9 +376,11 @@ TYPED_TEST(HttpFetcherTest, AbortTest) {
     g_source_set_callback(timeout_source_, AbortingTimeoutCallback, &delegate,
                           NULL);
     g_source_attach(timeout_source_, NULL);
-    fetcher->BeginTransfer(this->BigUrl());
+    delegate.fetcher_->BeginTransfer(this->BigUrl());
 
     g_main_loop_run(loop);
+    CHECK(!delegate.once_);
+    CHECK(!delegate.callback_once_);
     g_source_destroy(timeout_source_);
   }
   g_main_loop_unref(loop);
@@ -381,6 +398,9 @@ class FlakyHttpFetcherTestDelegate : public HttpFetcherDelegate {
     EXPECT_EQ(206, fetcher->http_response_code());
     g_main_loop_quit(loop_);
   }
+  virtual void TransferTerminated(HttpFetcher* fetcher) {
+    ADD_FAILURE();
+  }
   string data;
   GMainLoop* loop_;
 };
@@ -435,6 +455,9 @@ class FailureHttpFetcherTestDelegate : public HttpFetcherDelegate {
     EXPECT_EQ(0, fetcher->http_response_code());
     g_main_loop_quit(loop_);
   }
+  virtual void TransferTerminated(HttpFetcher* fetcher) {
+    ADD_FAILURE();
+  }
   GMainLoop* loop_;
   PythonHttpServer* server_;
 };
@@ -509,6 +532,9 @@ class RedirectHttpFetcherTestDelegate : public HttpFetcherDelegate {
     }
     g_main_loop_quit(loop_);
   }
+  virtual void TransferTerminated(HttpFetcher* fetcher) {
+    ADD_FAILURE();
+  }
   bool expected_successful_;
   string data;
   GMainLoop* loop_;
@@ -588,14 +614,22 @@ class MultiHttpFetcherTestDelegate : public HttpFetcherDelegate {
       : expected_response_code_(expected_response_code) {}
   virtual void ReceivedBytes(HttpFetcher* fetcher,
                              const char* bytes, int length) {
+    EXPECT_EQ(fetcher, fetcher_.get());
     data.append(bytes, length);
   }
   virtual void TransferComplete(HttpFetcher* fetcher, bool successful) {
+    EXPECT_EQ(fetcher, fetcher_.get());
     EXPECT_EQ(expected_response_code_ != 0, successful);
     if (expected_response_code_ != 0)
       EXPECT_EQ(expected_response_code_, fetcher->http_response_code());
+    // Destroy the fetcher (because we're allowed to).
+    fetcher_.reset(NULL);
     g_main_loop_quit(loop_);
   }
+  virtual void TransferTerminated(HttpFetcher* fetcher) {
+    ADD_FAILURE();
+  }
+  scoped_ptr<HttpFetcher> fetcher_;
   int expected_response_code_;
   string data;
   GMainLoop* loop_;
@@ -611,16 +645,16 @@ void MultiTest(HttpFetcher* fetcher_in,
   {
     MultiHttpFetcherTestDelegate delegate(expected_response_code);
     delegate.loop_ = loop;
-    scoped_ptr<HttpFetcher> fetcher(fetcher_in);
+    delegate.fetcher_.reset(fetcher_in);
     MultiHttpFetcher<LibcurlHttpFetcher>* multi_fetcher =
-        dynamic_cast<MultiHttpFetcher<LibcurlHttpFetcher>*>(fetcher.get());
+        dynamic_cast<MultiHttpFetcher<LibcurlHttpFetcher>*>(fetcher_in);
     ASSERT_TRUE(multi_fetcher);
     multi_fetcher->set_ranges(ranges);
     multi_fetcher->SetConnectionAsExpensive(false);
     multi_fetcher->SetBuildType(false);
-    fetcher->set_delegate(&delegate);
+    multi_fetcher->set_delegate(&delegate);
 
-    StartTransferArgs start_xfer_args = {fetcher.get(), url};
+    StartTransferArgs start_xfer_args = {multi_fetcher, url};
 
     g_timeout_add(0, StartTransfer, &start_xfer_args);
     g_main_loop_run(loop);
@@ -633,7 +667,7 @@ void MultiTest(HttpFetcher* fetcher_in,
 }
 }  // namespace {}
 
-TYPED_TEST(HttpFetcherTest, MultiHttpFetcherSimplTest) {
+TYPED_TEST(HttpFetcherTest, MultiHttpFetcherSimpleTest) {
   if (!this->IsMulti())
     return;
   typename TestFixture::HttpServer server;
@@ -713,6 +747,9 @@ class BlockedTransferTestDelegate : public HttpFetcherDelegate {
     EXPECT_FALSE(successful);
     g_main_loop_quit(loop_);
   }
+  virtual void TransferTerminated(HttpFetcher* fetcher) {
+    ADD_FAILURE();
+  }
   GMainLoop* loop_;
 };