Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(299)

Unified Diff: http_fetcher_unittest.cc

Issue 5009009: AU: Fix potential issues with premature destruction of HTTP fetchers. (Closed) Base URL: ssh://git@gitrw.chromium.org:9222/update_engine.git@master
Patch Set: review comments Created 10 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « http_fetcher.h ('k') | libcurl_http_fetcher.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: http_fetcher_unittest.cc
diff --git a/http_fetcher_unittest.cc b/http_fetcher_unittest.cc
index 30cde956acc6465b067643c4eae8bd5c39bcfa34..87dda7831df45ea36131369d30e8813f803e5c41 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
g_main_loop_quit(loop_);
}
+ virtual void TransferTerminated(HttpFetcher* fetcher) {
+ EXPECT_EQ(fetcher, fetcher_.get());
+ EXPECT_FALSE(once_);
+ EXPECT_TRUE(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_;
};
« no previous file with comments | « http_fetcher.h ('k') | libcurl_http_fetcher.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698