update_engine: ReceivedBytes returns boolean on transfer completion/termination
Currently, there are situations that a HttpFetcherDelegate.ReceivedBytes can
cause an eventual transfer completion or termination. This can eventually cause
the object (holding an instance) of HttpFetcherDelegate to be deallocated. So
there should not be any access to any member variable if the object is
deallocated. In this CL we add a return value to ReceivedBytes to indicate
explicitly that this situation happened so we can be careful about acceessing
member variables after a call to this.
BUG=chromium:868520
TEST=unittests
Change-Id: I18db33910f6171c4e426d9bbe604fa1ae07a56dc
Reviewed-on: https://chromium-review.googlesource.com/1158124
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
diff --git a/common/http_fetcher_unittest.cc b/common/http_fetcher_unittest.cc
index 73110e9..a0b0522 100644
--- a/common/http_fetcher_unittest.cc
+++ b/common/http_fetcher_unittest.cc
@@ -412,12 +412,13 @@
public:
HttpFetcherTestDelegate() = default;
- void ReceivedBytes(HttpFetcher* /* fetcher */,
+ bool ReceivedBytes(HttpFetcher* /* fetcher */,
const void* bytes,
size_t length) override {
data.append(reinterpret_cast<const char*>(bytes), length);
// Update counters
times_received_bytes_called_++;
+ return true;
}
void TransferComplete(HttpFetcher* fetcher, bool successful) override {
@@ -560,11 +561,13 @@
namespace {
class PausingHttpFetcherTestDelegate : public HttpFetcherDelegate {
public:
- void ReceivedBytes(HttpFetcher* fetcher,
- const void* /* bytes */, size_t /* length */) override {
+ bool ReceivedBytes(HttpFetcher* fetcher,
+ const void* /* bytes */,
+ size_t /* length */) override {
CHECK(!paused_);
paused_ = true;
fetcher->Pause();
+ return true;
}
void TransferComplete(HttpFetcher* fetcher, bool successful) override {
MessageLoop::current()->BreakLoop();
@@ -641,8 +644,11 @@
namespace {
class AbortingHttpFetcherTestDelegate : public HttpFetcherDelegate {
public:
- void ReceivedBytes(HttpFetcher* fetcher,
- const void* bytes, size_t length) override {}
+ bool ReceivedBytes(HttpFetcher* fetcher,
+ const void* bytes,
+ size_t length) override {
+ return true;
+ }
void TransferComplete(HttpFetcher* fetcher, bool successful) override {
ADD_FAILURE(); // We should never get here
MessageLoop::current()->BreakLoop();
@@ -736,9 +742,11 @@
namespace {
class FlakyHttpFetcherTestDelegate : public HttpFetcherDelegate {
public:
- void ReceivedBytes(HttpFetcher* fetcher,
- const void* bytes, size_t length) override {
+ bool ReceivedBytes(HttpFetcher* fetcher,
+ const void* bytes,
+ size_t length) override {
data.append(reinterpret_cast<const char*>(bytes), length);
+ return true;
}
void TransferComplete(HttpFetcher* fetcher, bool successful) override {
EXPECT_TRUE(successful);
@@ -800,13 +808,15 @@
}
}
- void ReceivedBytes(HttpFetcher* fetcher,
- const void* bytes, size_t length) override {
+ bool ReceivedBytes(HttpFetcher* fetcher,
+ const void* bytes,
+ size_t length) override {
if (server_) {
LOG(INFO) << "Stopping server in ReceivedBytes";
server_.reset();
LOG(INFO) << "server stopped";
}
+ return true;
}
void TransferComplete(HttpFetcher* fetcher, bool successful) override {
EXPECT_FALSE(successful);
@@ -974,9 +984,11 @@
public:
explicit RedirectHttpFetcherTestDelegate(bool expected_successful)
: expected_successful_(expected_successful) {}
- void ReceivedBytes(HttpFetcher* fetcher,
- const void* bytes, size_t length) override {
+ bool ReceivedBytes(HttpFetcher* fetcher,
+ const void* bytes,
+ size_t length) override {
data.append(reinterpret_cast<const char*>(bytes), length);
+ return true;
}
void TransferComplete(HttpFetcher* fetcher, bool successful) override {
EXPECT_EQ(expected_successful_, successful);
@@ -1073,10 +1085,12 @@
explicit MultiHttpFetcherTestDelegate(int expected_response_code)
: expected_response_code_(expected_response_code) {}
- void ReceivedBytes(HttpFetcher* fetcher,
- const void* bytes, size_t length) override {
+ bool ReceivedBytes(HttpFetcher* fetcher,
+ const void* bytes,
+ size_t length) override {
EXPECT_EQ(fetcher, fetcher_.get());
data.append(reinterpret_cast<const char*>(bytes), length);
+ return true;
}
void TransferComplete(HttpFetcher* fetcher, bool successful) override {
@@ -1272,7 +1286,7 @@
explicit MultiHttpFetcherTerminateTestDelegate(size_t terminate_trigger_bytes)
: terminate_trigger_bytes_(terminate_trigger_bytes) {}
- void ReceivedBytes(HttpFetcher* fetcher,
+ bool ReceivedBytes(HttpFetcher* fetcher,
const void* bytes,
size_t length) override {
LOG(INFO) << "ReceivedBytes, " << length << " bytes.";
@@ -1285,6 +1299,7 @@
base::Unretained(fetcher_.get())));
}
bytes_downloaded_ += length;
+ return true;
}
void TransferComplete(HttpFetcher* fetcher, bool successful) override {
@@ -1338,9 +1353,11 @@
namespace {
class BlockedTransferTestDelegate : public HttpFetcherDelegate {
public:
- void ReceivedBytes(HttpFetcher* fetcher,
- const void* bytes, size_t length) override {
+ bool ReceivedBytes(HttpFetcher* fetcher,
+ const void* bytes,
+ size_t length) override {
ADD_FAILURE();
+ return true;
}
void TransferComplete(HttpFetcher* fetcher, bool successful) override {
EXPECT_FALSE(successful);