Delete DownloadActionDelegate::SetDownloadStatus() method.
This method is only used by the caller to detect a programming error in
the DownloadAction (calling BytesReceived when not downloading) and log
a message. This patch removes the method from the delegate interface
and makes sure it doesn't issue a call to BytesReceived when not
activelly downloading.
Bug: 25773375
TEST=FEATURES=test emerge-link update_engine
Change-Id: I8ff5c53f1fd40c0777b3f6523703a8bee71c019d
diff --git a/payload_consumer/download_action.cc b/payload_consumer/download_action.cc
index 0136f49..58e29b2 100644
--- a/payload_consumer/download_action.cc
+++ b/payload_consumer/download_action.cc
@@ -186,9 +186,7 @@
&install_plan_));
writer_ = delta_performer_.get();
}
- if (delegate_) {
- delegate_->SetDownloadStatus(true); // Set to active.
- }
+ download_active_= true;
if (system_state_ != nullptr) {
const PayloadStateInterface* payload_state = system_state_->payload_state();
@@ -236,9 +234,7 @@
writer_->Close();
writer_ = nullptr;
}
- if (delegate_) {
- delegate_->SetDownloadStatus(false); // Set to inactive.
- }
+ download_active_= false;
CloseP2PSharingFd(false); // Keep p2p file.
// Terminates the transfer. The action is terminated, if necessary, when the
// TransferTerminated callback is received.
@@ -258,7 +254,7 @@
}
bytes_received_ += length;
- if (delegate_)
+ if (delegate_ && download_active_)
delegate_->BytesReceived(bytes_received_, install_plan_.payload_size);
if (writer_ && !writer_->Write(bytes, length, &code_)) {
LOG(ERROR) << "Error " << code_ << " in DeltaPerformer's Write method when "
@@ -288,9 +284,7 @@
LOG_IF(WARNING, writer_->Close() != 0) << "Error closing the writer.";
writer_ = nullptr;
}
- if (delegate_) {
- delegate_->SetDownloadStatus(false); // Set to inactive.
- }
+ download_active_= false;
ErrorCode code =
successful ? ErrorCode::kSuccess : ErrorCode::kDownloadTransferError;
if (code == ErrorCode::kSuccess && delta_performer_.get()) {
diff --git a/payload_consumer/download_action.h b/payload_consumer/download_action.h
index c0f0688..e57ffb3 100644
--- a/payload_consumer/download_action.h
+++ b/payload_consumer/download_action.h
@@ -42,15 +42,9 @@
public:
virtual ~DownloadActionDelegate() = default;
- // Called right before starting the download with |active| set to
- // true. Called after completing the download with |active| set to
- // false.
- virtual void SetDownloadStatus(bool active) = 0;
-
- // Called periodically after bytes are received. This method will be
- // invoked only if the download is active. |bytes_received| is the
- // number of bytes downloaded thus far. |total| is the number of
- // bytes expected.
+ // Called periodically after bytes are received. This method will be invoked
+ // only if the DownloadAction is running. |bytes_received| is the number of
+ // bytes downloaded thus far. |total| is the number of bytes expected.
virtual void BytesReceived(uint64_t bytes_received, uint64_t total) = 0;
};
@@ -142,6 +136,7 @@
// For reporting status to outsiders
DownloadActionDelegate* delegate_;
uint64_t bytes_received_;
+ bool download_active_{false};
// The file-id for the file we're sharing or the empty string
// if we're not using p2p to share.
diff --git a/payload_consumer/download_action_unittest.cc b/payload_consumer/download_action_unittest.cc
index 3bef196..901dbd6 100644
--- a/payload_consumer/download_action_unittest.cc
+++ b/payload_consumer/download_action_unittest.cc
@@ -62,7 +62,6 @@
namespace {
class DownloadActionDelegateMock : public DownloadActionDelegate {
public:
- MOCK_METHOD1(SetDownloadStatus, void(bool active));
MOCK_METHOD2(BytesReceived, void(uint64_t bytes_received, uint64_t total));
};
@@ -177,12 +176,10 @@
if (use_download_delegate) {
InSequence s;
download_action.set_delegate(&download_delegate);
- EXPECT_CALL(download_delegate, SetDownloadStatus(true)).Times(1);
if (data.size() > kMockHttpFetcherChunkSize)
EXPECT_CALL(download_delegate,
BytesReceived(1 + kMockHttpFetcherChunkSize, _));
EXPECT_CALL(download_delegate, BytesReceived(_, _)).Times(AtLeast(1));
- EXPECT_CALL(download_delegate, SetDownloadStatus(false)).Times(1);
}
ErrorCode expected_code = ErrorCode::kSuccess;
if (fail_write > 0)
@@ -291,10 +288,8 @@
download_action.SetTestFileWriter(&writer);
DownloadActionDelegateMock download_delegate;
if (use_download_delegate) {
- InSequence s;
download_action.set_delegate(&download_delegate);
- EXPECT_CALL(download_delegate, SetDownloadStatus(true)).Times(1);
- EXPECT_CALL(download_delegate, SetDownloadStatus(false)).Times(1);
+ EXPECT_CALL(download_delegate, BytesReceived(_, _)).Times(0);
}
TerminateEarlyTestProcessorDelegate delegate;
ActionProcessor processor;
