Unconditionally delete the unpack path in the UpdateClient.
Bug: 726959
Change-Id: I002aa30ee0d85d845dba915d9dbd6e5599218184
Reviewed-on: https://chromium-review.googlesource.com/517463
Reviewed-by: Joshua Pawlicki <[email protected]>
Commit-Queue: Sorin Jianu <[email protected]>
Cr-Commit-Position: refs/heads/master@{#475648}diff --git a/components/update_client/component.cc b/components/update_client/component.cc
index dec191d..a63b5cd 100644
--- a/components/update_client/component.cc
+++ b/components/update_client/component.cc
@@ -95,9 +95,11 @@
InstallOnBlockingTaskRunnerCompleteCallback callback) {
DCHECK(blocking_task_runner->RunsTasksOnCurrentThread());
+ DCHECK(base::DirectoryExists(unpack_path));
const auto result = DoInstallOnBlockingTaskRunner(
main_task_runner, blocking_task_runner, unpack_path, fingerprint,
installer, callback);
+ base::DeleteFile(unpack_path, true);
const ErrorCategory error_category =
result.error ? ErrorCategory::kInstallError : ErrorCategory::kErrorNone;
diff --git a/components/update_client/test_installer.cc b/components/update_client/test_installer.cc
index 3232bd6..7a003dbf 100644
--- a/components/update_client/test_installer.cc
+++ b/components/update_client/test_installer.cc
@@ -10,12 +10,21 @@
#include "base/files/file_util.h"
#include "base/values.h"
#include "components/update_client/update_client_errors.h"
+#include "testing/gtest/include/gtest/gtest.h"
namespace update_client {
TestInstaller::TestInstaller() : error_(0), install_count_(0) {
}
+TestInstaller::~TestInstaller() {
+ // The unpack path is deleted unconditionally by the component state code,
+ // which is driving this installer. Therefore, the unpack path must not
+ // exist when this object is destroyed.
+ if (!unpack_path_.empty())
+ EXPECT_FALSE(base::DirectoryExists(unpack_path_));
+}
+
void TestInstaller::OnUpdateError(int error) {
error_ = error;
}
@@ -24,8 +33,8 @@
const base::DictionaryValue& manifest,
const base::FilePath& unpack_path) {
++install_count_;
- if (!base::DeleteFile(unpack_path, true))
- return Result(InstallError::GENERIC_ERROR);
+
+ unpack_path_ = unpack_path;
return Result(InstallError::NONE);
}
@@ -35,9 +44,6 @@
return false;
}
-TestInstaller::~TestInstaller() {
-}
-
bool TestInstaller::Uninstall() {
return false;
}
diff --git a/components/update_client/test_installer.h b/components/update_client/test_installer.h
index 1b5bea0..473772e 100644
--- a/components/update_client/test_installer.h
+++ b/components/update_client/test_installer.h
@@ -42,6 +42,10 @@
int error_;
int install_count_;
+
+ private:
+ // Contains the |unpack_path| argument of the Install call.
+ base::FilePath unpack_path_;
};
// A ReadOnlyTestInstaller is an installer that knows about files in an existing
diff --git a/components/update_client/update_client_unittest.cc b/components/update_client/update_client_unittest.cc
index 73e9e9d..fae978c 100644
--- a/components/update_client/update_client_unittest.cc
+++ b/components/update_client/update_client_unittest.cc
@@ -1370,13 +1370,24 @@
bool(const std::string& file, base::FilePath* installed_file));
MOCK_METHOD0(Uninstall, bool());
- static void OnInstall(const base::DictionaryValue& manifest,
- const base::FilePath& unpack_path) {
- base::DeleteFile(unpack_path, true);
+ void OnInstall(const base::DictionaryValue& manifest,
+ const base::FilePath& unpack_path) {
+ unpack_path_ = unpack_path;
+ EXPECT_TRUE(base::DirectoryExists(unpack_path_));
}
protected:
- ~MockInstaller() override {}
+ ~MockInstaller() override {
+ // The unpack path is deleted unconditionally by the component state code,
+ // which is driving this installer. Therefore, the unpack path must not
+ // exist when this object is destroyed.
+ if (!unpack_path_.empty())
+ EXPECT_FALSE(base::DirectoryExists(unpack_path_));
+ }
+
+ private:
+ // Contains the |unpack_path| argument of the Install call.
+ base::FilePath unpack_path_;
};
class DataCallbackFake {
@@ -1389,7 +1400,7 @@
EXPECT_CALL(*installer, OnUpdateError(_)).Times(0);
EXPECT_CALL(*installer, Install(_, _))
.WillOnce(
- DoAll(Invoke(MockInstaller::OnInstall),
+ DoAll(Invoke(installer.get(), &MockInstaller::OnInstall),
Return(CrxInstaller::Result(InstallError::GENERIC_ERROR))));
EXPECT_CALL(*installer, GetInstalledFile(_, _)).Times(0);
EXPECT_CALL(*installer, Uninstall()).Times(0);
