Chrome Cleaner: download the Chrome Cleaner executable in an empty directory.

chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_fetcher_win_unittest.cc

Index: chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_fetcher_win_unittest.cc
diff --git a/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_fetcher_win_unittest.cc b/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_fetcher_win_unittest.cc
index dc80d573e13ce031c457601f89b1ec03b5f6ec23..5e26a773028108825589f6a051c5445272985a9b 100644
--- a/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_fetcher_win_unittest.cc
+++ b/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_fetcher_win_unittest.cc
@@ -4,12 +4,13 @@
 
 #include "chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_fetcher_win.h"
 
-#include <memory>
-
+#include "base/base_paths.h"
+#include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/files/file_path.h"
-#include "base/memory/ptr_util.h"
+#include "base/files/file_util.h"
+#include "base/run_loop.h"
 #include "chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "net/base/net_errors.h"
@@ -24,7 +25,8 @@
 namespace safe_browsing {
 namespace {
 
-class ChromeCleanerFetcherTest : public ::testing::Test {
+class ChromeCleanerFetcherTest : public ::testing::Test,
+                                 public net::TestURLFetcher::DelegateForTests {
  public:
   void SetUp() override {
     FetchChromeCleaner(base::Bind(&ChromeCleanerFetcherTest::FetchedCallback,
@@ -32,28 +34,65 @@ class ChromeCleanerFetcherTest : public ::testing::Test {
 
     // Get the TestURLFetcher instance used by FetchChromeCleaner.
     fetcher_ = factory_.GetFetcherByID(0);
+    fetcher_->SetDelegateForTests(this);
+
     ASSERT_TRUE(fetcher_);
+
+    // Continue only when URLFetcher's Start() method has been called.
+    run_loop_.Run();
   }
 
-  void FetchedCallback(base::FilePath downloaded_path, int http_response_code) {
+  void TearDown() override {
+    if (!response_path_.empty()) {
+      base::DeleteFile(response_path_, /*recursive=*/false);
+      if (base::IsDirectoryEmpty(response_path_.DirName()))
+        base::DeleteFile(response_path_.DirName(), /*recursive=*/false);
+    }
+  }
+
+  void FetchedCallback(base::FilePath downloaded_path,
+                       ChromeCleanerFetchStatus fetch_status) {
     callback_called_ = true;
     downloaded_path_ = downloaded_path;
-    http_response_code_ = http_response_code;
+    fetch_status_ = fetch_status;
+  }
+
+  // net::TestURLFetcher::DelegateForTests overrides.
+
+  void OnRequestStart(int fetcher_id) override {
+    // Intercept and change the response file path so that the test can clean up
+    // any created temp files.

[robertshield, 2017/06/12 14:36:42]

How does this intercept and change the file path? Looks like it just stores it
for future cleanup?

[alito, 2017/06/12 19:05:36]

Forgot to update the comment after some changes. Updated now. Thx.

+    EXPECT_TRUE(fetcher_->GetResponseAsFilePath(/*take_ownership=*/false,
+                                                &response_path_));
+
+    run_loop_.QuitWhenIdle();
   }
 
+  void OnChunkUpload(int fetcher_id) override {}
+  void OnRequestEnd(int fetcher_id) override {}
+
  protected:
   // TestURLFetcher requires a MessageLoop and an IO thread to release
   // URLRequestContextGetter in URLFetcher::Core.
   content::TestBrowserThreadBundle thread_bundle_;
+
   // TestURLFetcherFactory automatically sets itself as URLFetcher's factory
   net::TestURLFetcherFactory factory_;
 
   // The TestURLFetcher instance used by the FetchChromeCleaner.
   net::TestURLFetcher* fetcher_ = nullptr;
+
+  base::RunLoop run_loop_;
+
+  // File path where TestULRFetcher will save a response as intercepted by
+  // OnRequestStart(). Used to clean up during teardown.
+  base::FilePath response_path_;
+
   // Variables set by FetchedCallback().
   bool callback_called_ = false;
   base::FilePath downloaded_path_;
-  int http_response_code_ = -1;
+  ChromeCleanerFetchStatus fetch_status_ =
+      ChromeCleanerFetchStatus::kOtherFailure;
 };
 
 TEST_F(ChromeCleanerFetcherTest, FetchSuccess) {
@@ -62,32 +101,38 @@ TEST_F(ChromeCleanerFetcherTest, FetchSuccess) {
   // Set up the fetcher to return success.
   fetcher_->set_status(net::URLRequestStatus{});
   fetcher_->set_response_code(net::HTTP_OK);
-  // We need to save the file path where the response will be saved for later
-  // because ChromeCleanerFetcher takes ownership of the file after
-  // OnURLFetchComplete() has been called and we can't call
-  // GetResponseAsFilePath() after that..
-  base::FilePath response_file;
-  fetcher_->GetResponseAsFilePath(/*take_ownership=*/false, &response_file);
-
-  // After this call, the ChromeCleanerFetcher instance will delete itself.
+
   fetcher_->delegate()->OnURLFetchComplete(fetcher_);
 
   EXPECT_TRUE(callback_called_);
-  EXPECT_EQ(downloaded_path_, response_file);
-  EXPECT_EQ(http_response_code_, net::HTTP_OK);
+  EXPECT_EQ(downloaded_path_, response_path_);
+  EXPECT_EQ(fetch_status_, ChromeCleanerFetchStatus::kSuccess);
 }
 
-TEST_F(ChromeCleanerFetcherTest, FetchFailure) {
-  // Set up the fetcher to return failure.
+TEST_F(ChromeCleanerFetcherTest, NotFoundOnServer) {
+  // Set up the fetcher to return a HTTP_NOT_FOUND failure.
   fetcher_->set_status(net::URLRequestStatus::FromError(net::ERR_FAILED));
   fetcher_->set_response_code(net::HTTP_NOT_FOUND);
 
-  // After this call, the ChromeCleanerFetcher instance will delete itself.
   fetcher_->delegate()->OnURLFetchComplete(fetcher_);
 
   EXPECT_TRUE(callback_called_);
   EXPECT_TRUE(downloaded_path_.empty());
-  EXPECT_EQ(http_response_code_, net::HTTP_NOT_FOUND);
+  EXPECT_EQ(fetch_status_, ChromeCleanerFetchStatus::kNotFoundOnServer);
+}
+
+TEST_F(ChromeCleanerFetcherTest, OtherFailure) {
+  // Set up the fetcher to return failure other than HTTP_NOT_FOUND.
+  fetcher_->set_status(net::URLRequestStatus::FromError(net::ERR_FAILED));
+  // For this test, just use any http response code other than net::HTTP_OK and
+  // net::HTTP_NOT_FOUND.
+  fetcher_->set_response_code(net::HTTP_INTERNAL_SERVER_ERROR);
+
+  fetcher_->delegate()->OnURLFetchComplete(fetcher_);
+
+  EXPECT_TRUE(callback_called_);
+  EXPECT_TRUE(downloaded_path_.empty());
+  EXPECT_EQ(fetch_status_, ChromeCleanerFetchStatus::kOtherFailure);
 }
 
 }  // namespace