AU: Push seeks in http fetching to the progress percentage.
The updater was reporting status as a fraction of bytes downloaded vs
total, ignoring any bytes that were skipped due to resuming an
upload. The result was that a user may see progress go from 0 to 60
percent, then reboot, then see progress go from 0 to 40 percent, then
finish.
This CL allows an HTTP fetcher to report to the delegate that it's
seeking to a particular offset.
BUG=8151
TEST=applied updates on the machine; unittest
Review URL: http://codereview.chromium.org/4131005
Change-Id: Ib9fe034ca3ffd17455af4cf89d5b28ec236104a0
diff --git a/download_action.cc b/download_action.cc
index b6f7f81..bb5d189 100644
--- a/download_action.cc
+++ b/download_action.cc
@@ -107,6 +107,10 @@
}
}
+void DownloadAction::SeekToOffset(off_t offset) {
+ bytes_received_ = offset;
+}
+
void DownloadAction::ReceivedBytes(HttpFetcher *fetcher,
const char* bytes,
int length) {
diff --git a/download_action.h b/download_action.h
index 08035f0..d0ac175 100644
--- a/download_action.h
+++ b/download_action.h
@@ -86,6 +86,7 @@
// HttpFetcherDelegate methods (see http_fetcher.h)
virtual void ReceivedBytes(HttpFetcher *fetcher,
const char* bytes, int length);
+ virtual void SeekToOffset(off_t offset);
virtual void TransferComplete(HttpFetcher *fetcher, bool successful);
DownloadActionDelegate* delegate() const { return delegate_; }
diff --git a/download_action_unittest.cc b/download_action_unittest.cc
index 7571fb4..2f37dfb 100644
--- a/download_action_unittest.cc
+++ b/download_action_unittest.cc
@@ -80,9 +80,18 @@
ActionProcessor *processor;
};
+struct StartProcessorInRunLoopArgs {
+ ActionProcessor* processor;
+ MockHttpFetcher* http_fetcher;
+};
+
gboolean StartProcessorInRunLoop(gpointer data) {
- ActionProcessor *processor = reinterpret_cast<ActionProcessor*>(data);
+ ActionProcessor* processor =
+ reinterpret_cast<StartProcessorInRunLoopArgs*>(data)->processor;
processor->StartProcessing();
+ MockHttpFetcher* http_fetcher =
+ reinterpret_cast<StartProcessorInRunLoopArgs*>(data)->http_fetcher;
+ http_fetcher->SetOffset(1);
return FALSE;
}
@@ -96,10 +105,11 @@
ScopedTempFile output_temp_file;
DirectFileWriter writer;
- // takes ownership of passed in HttpFetcher
+ // We pull off the first byte from data and seek past it.
+
string hash = hash_test ?
OmahaHashCalculator::OmahaHashOfString("random string") :
- OmahaHashCalculator::OmahaHashOfData(data);
+ OmahaHashCalculator::OmahaHashOfBytes(&data[1], data.size() - 1);
uint64_t size = data.size() + (size_test ? 1 : 0);
InstallPlan install_plan(true,
false,
@@ -111,8 +121,9 @@
ObjectFeederAction<InstallPlan> feeder_action;
feeder_action.set_obj(install_plan);
PrefsMock prefs;
- DownloadAction download_action(&prefs, new MockHttpFetcher(&data[0],
- data.size()));
+ MockHttpFetcher* http_fetcher = new MockHttpFetcher(&data[0], data.size());
+ // takes ownership of passed in HttpFetcher
+ DownloadAction download_action(&prefs, http_fetcher);
download_action.SetTestFileWriter(&writer);
BondActions(&feeder_action, &download_action);
DownloadActionDelegateMock download_delegate;
@@ -120,7 +131,11 @@
InSequence s;
download_action.set_delegate(&download_delegate);
EXPECT_CALL(download_delegate, SetDownloadStatus(true)).Times(1);
- EXPECT_CALL(download_delegate, BytesReceived(_, _)).Times(AtLeast(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);
}
ActionExitCode expected_code = kActionCodeSuccess;
@@ -130,14 +145,17 @@
expected_code = kActionCodeDownloadSizeMismatchError;
DownloadActionTestProcessorDelegate delegate(expected_code);
delegate.loop_ = loop;
- delegate.expected_data_ = data;
+ delegate.expected_data_ = vector<char>(data.begin() + 1, data.end());
delegate.path_ = output_temp_file.GetPath();
ActionProcessor processor;
processor.set_delegate(&delegate);
processor.EnqueueAction(&feeder_action);
processor.EnqueueAction(&download_action);
- g_timeout_add(0, &StartProcessorInRunLoop, &processor);
+ StartProcessorInRunLoopArgs args;
+ args.processor = &processor;
+ args.http_fetcher = http_fetcher;
+ g_timeout_add(0, &StartProcessorInRunLoop, &args);
g_main_loop_run(loop);
g_main_loop_unref(loop);
}
diff --git a/http_fetcher.h b/http_fetcher.h
index f61116a..d0f9cb2 100644
--- a/http_fetcher.h
+++ b/http_fetcher.h
@@ -89,12 +89,14 @@
// Interface for delegates
class HttpFetcherDelegate {
public:
- // Called every time bytes are received, even if they are automatically
- // delivered to an output file.
+ // Called every time bytes are received.
virtual void ReceivedBytes(HttpFetcher* fetcher,
const char* bytes,
int length) = 0;
+ // Called if the fetcher seeks to a particular offset.
+ virtual void SeekToOffset(off_t offset) {}
+
// Called when the transfer has completed successfully or been somehow
// aborted.
virtual void TransferComplete(HttpFetcher* fetcher, bool successful) = 0;
diff --git a/mock_http_fetcher.h b/mock_http_fetcher.h
index 91d50dd..f0830f0 100644
--- a/mock_http_fetcher.h
+++ b/mock_http_fetcher.h
@@ -35,7 +35,11 @@
~MockHttpFetcher();
// Ignores this.
- virtual void SetOffset(off_t offset) {}
+ virtual void SetOffset(off_t offset) {
+ sent_size_ = offset;
+ if (delegate_)
+ delegate_->SeekToOffset(offset);
+ }
// Begins the transfer if it hasn't already begun.
virtual void BeginTransfer(const std::string& url);
diff --git a/multi_http_fetcher.h b/multi_http_fetcher.h
index 4328199..692054d 100644
--- a/multi_http_fetcher.h
+++ b/multi_http_fetcher.h
@@ -122,6 +122,8 @@
<< ranges_[current_index_].second << ")";
bytes_received_this_fetcher_ = 0;
fetchers_[current_index_]->SetOffset(ranges_[current_index_].first);
+ if (delegate_)
+ delegate_->SeekToOffset(ranges_[current_index_].first);
fetchers_[current_index_]->BeginTransfer(url_);
}
@@ -141,8 +143,9 @@
bytes_received_this_fetcher_);
}
LOG_IF(WARNING, next_size <= 0) << "Asked to write length <= 0";
- if (delegate_)
+ if (delegate_) {
delegate_->ReceivedBytes(this, bytes, next_size);
+ }
bytes_received_this_fetcher_ += length;
if (ranges_[current_index_].second >= 0 &&
bytes_received_this_fetcher_ >= ranges_[current_index_].second) {
diff --git a/prefs.cc b/prefs.cc
index bdd620b..223fc9b 100644
--- a/prefs.cc
+++ b/prefs.cc
@@ -35,7 +35,11 @@
bool Prefs::GetString(const string& key, string* value) {
FilePath filename;
TEST_AND_RETURN_FALSE(GetFileNameForKey(key, &filename));
- TEST_AND_RETURN_FALSE(file_util::ReadFileToString(filename, value));
+ if (!file_util::ReadFileToString(filename, value)) {
+ PLOG(INFO) << "Unable to read prefs from " << filename.value()
+ << ". This is likely harmless.";
+ return false;
+ }
return true;
}