Plugin VM setup fails if there is no download hash

If no hash is returned from DownloadService, no image verification
can take place, so the image_manager should always indicate a failure
in this case.

Also adapt unit tests and add the possibility to set a hash in
test_download_service.

Bug: 938253
Test: browser_tests --gtest_filter="PluginVm*" && unit_tests --gtest_filter="PluginVm*"
Change-Id: I8bf9f3151d44910cdc44b51179a5ac16cf67a298
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1596673Reviewed-by: Olya Kalitova <okalitova@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Commit-Queue: Alex Oldemeier <aoldemeier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659161}

chrome/browser/chromeos/plugin_vm/plugin_vm_image_manager.cc

@@ -323,16 +323,9 @@ void PluginVmImageManager::OnStartDownload(
 
 bool PluginVmImageManager::VerifyDownload(
     const std::string& downloaded_archive_hash) {
-  // Hash should be there in the common case. However, there are situations,
-  // when hash could not be available, for example, when download is parallel
-  // or the completion of download is reported after restart. Therefore, hash
-  // not being specified should not resolve in download being considered
-  // unsuccessful, but should be logged.
-  // TODO(okalitova): Consider download as unsuccessful once hash should always
-  // be in place.
   if (downloaded_archive_hash.empty()) {
-    LOG(WARNING) << "No hash for downloaded PluginVm image archive";
-    return true;
+    LOG(ERROR) << "No hash found for downloaded PluginVm image archive";
+    return false;
   }
   const base::Value* plugin_vm_image_hash_ptr =
       profile_->GetPrefs()

chrome/browser/chromeos/plugin_vm/plugin_vm_image_manager_unittest.cc

@@ -87,6 +87,7 @@ class PluginVmImageManagerTest : public testing::Test {
 
     manager_ = PluginVmImageManagerFactory::GetForProfile(profile_.get());
     download_service_->SetIsReady(true);
+    download_service_->SetHash256(kHash);
     download_service_->set_client(
         new PluginVmImageDownloadClient(profile_.get()));
     manager_->SetDownloadServiceForTesting(download_service_);
@@ -394,6 +395,7 @@ TEST_F(PluginVmImageManagerTest, VerifyDownloadTest) {
   EXPECT_FALSE(manager_->VerifyDownload(kHash2));
   EXPECT_TRUE(manager_->VerifyDownload(kHashUppercase));
   EXPECT_TRUE(manager_->VerifyDownload(kHash));
+  EXPECT_FALSE(manager_->VerifyDownload(std::string()));
 }
 
 }  // namespace plugin_vm

components/download/public/background_service/test/test_download_service.cc

@@ -127,6 +127,10 @@ void TestDownloadService::SetIsReady(bool is_ready) {
     ProcessDownload();
 }
 
+void TestDownloadService::SetHash256(const std::string& hash256) {
+  hash256_ = hash256;
+}
+
 void TestDownloadService::ProcessDownload() {
   DCHECK(!fail_at_start_);
   if (!is_ready_ || downloads_.empty())
@@ -142,6 +146,7 @@ void TestDownloadService::ProcessDownload() {
   } else {
     CompletionInfo completion_info(base::FilePath(), file_size_,
                                    {params.request_params.url}, nullptr);
+    completion_info.hash256 = hash256_;
     OnDownloadSucceeded(params.guid, completion_info);
   }
 }

components/download/public/background_service/test/test_download_service.h

@@ -48,6 +48,8 @@ class TestDownloadService : public DownloadService {
 
   void SetIsReady(bool is_ready);
 
+  void SetHash256(const std::string& hash256);
+
   void set_client(Client* client) { client_ = client; }
 
  private:
@@ -67,6 +69,7 @@ class TestDownloadService : public DownloadService {
   std::unique_ptr<Logger> logger_;
 
   bool is_ready_;
+  std::string hash256_;
   std::string failed_download_id_;
   bool fail_at_start_;
   uint64_t file_size_;