Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6295)

Unified Diff: chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_fetcher_win_unittest.cc

Issue 2929483002: Chrome Cleaner: download the Chrome Cleaner executable in an empty directory. (Closed)
Patch Set: Nits Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..84134526ec63fb9dc4823aa29769f4bbaf7d65a5 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,64 @@ 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 {
+ // Save the file path where the response will be saved for later tests.
+ 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 +100,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

Powered by Google App Engine
This is Rietveld 408576698