Index: chrome/browser/search_engines/template_url_fetcher_unittest.cc |
diff --git a/chrome/browser/search_engines/template_url_fetcher_unittest.cc b/chrome/browser/search_engines/template_url_fetcher_unittest.cc |
index e760c440d05d114049029c6222d8fe7e7dffc0e8..e7bc75880eb3f86c0d61172824e6be179394db5c 100644 |
--- a/chrome/browser/search_engines/template_url_fetcher_unittest.cc |
+++ b/chrome/browser/search_engines/template_url_fetcher_unittest.cc |
@@ -25,8 +25,32 @@ |
#include "testing/gtest/include/gtest/gtest.h" |
#include "url/gurl.h" |
+namespace { |
+ |
using base::ASCIIToUTF16; |
+class TestTemplateUrlFetcher : public TemplateURLFetcher { |
+ public: |
+ TestTemplateUrlFetcher(TemplateURLService* template_url_service, |
+ net::URLRequestContextGetter* request_context, |
+ const base::Closure& request_completed_callback) |
+ : TemplateURLFetcher(template_url_service, request_context), |
+ callback_(request_completed_callback) {} |
+ ~TestTemplateUrlFetcher() override {} |
+ |
+ protected: |
+ void RequestCompleted(RequestDelegate* request) override { |
+ callback_.Run(); |
+ TemplateURLFetcher::RequestCompleted(request); |
+ } |
+ |
+ private: |
+ // Callback to be run when a request completes. |
+ base::Closure callback_; |
+ |
+ DISALLOW_COPY_AND_ASSIGN(TestTemplateUrlFetcher); |
+}; |
+ |
// Basic set-up for TemplateURLFetcher tests. |
class TemplateURLFetcherTest : public testing::Test { |
public: |
@@ -35,8 +59,10 @@ class TemplateURLFetcherTest : public testing::Test { |
void SetUp() override { |
TestingProfile* profile = test_util_.profile(); |
ASSERT_TRUE(profile->GetRequestContext()); |
- template_url_fetcher_.reset(new TemplateURLFetcher( |
- test_util_.model(), profile->GetRequestContext())); |
+ template_url_fetcher_.reset(new TestTemplateUrlFetcher( |
+ test_util_.model(), profile->GetRequestContext(), |
+ base::Bind(&TemplateURLFetcherTest::RequestCompletedCallback, |
+ base::Unretained(this)))); |
ASSERT_TRUE(test_server_.Start()); |
} |
@@ -45,19 +71,12 @@ class TemplateURLFetcherTest : public testing::Test { |
ASSERT_TRUE(test_server_.ShutdownAndWaitUntilComplete()); |
} |
- // Called when the callback is destroyed. |
- void DestroyedCallback(); |
- |
- // TemplateURLFetcherCallbacks implementation. (Although not derived from |
- // this class, this method handles those calls for the test.) |
- void ConfirmAddSearchProvider( |
- base::ScopedClosureRunner* callback_destruction_notifier, |
- std::unique_ptr<TemplateURL> template_url); |
+ // Called when a request completes. |
+ void RequestCompletedCallback(); |
// Schedules the download of the url. |
void StartDownload(const base::string16& keyword, |
const std::string& osdd_file_name, |
- TemplateURLFetcher::ProviderType provider_type, |
bool check_that_file_exists); |
// Waits for any downloads to finish. |
@@ -67,11 +86,7 @@ class TemplateURLFetcherTest : public testing::Test { |
TemplateURLFetcher* template_url_fetcher() { |
return template_url_fetcher_.get(); |
} |
- const TemplateURL* last_callback_template_url() const { |
- return last_callback_template_url_.get(); |
- } |
- int callbacks_destroyed() const { return callbacks_destroyed_; } |
- int add_provider_called() const { return add_provider_called_; } |
+ int requests_completed() const { return requests_completed_; } |
private: |
content::TestBrowserThreadBundle thread_bundle_; // To set up BrowserThreads. |
@@ -79,14 +94,8 @@ class TemplateURLFetcherTest : public testing::Test { |
std::unique_ptr<TemplateURLFetcher> template_url_fetcher_; |
net::EmbeddedTestServer test_server_; |
- // The last TemplateURL to come from a callback. |
- std::unique_ptr<TemplateURL> last_callback_template_url_; |
- |
- // How many TemplateURLFetcherTestCallbacks have been destructed. |
- int callbacks_destroyed_; |
- |
- // How many times ConfirmAddSearchProvider has been called. |
- int add_provider_called_; |
+ // How many TemplateURKFetcher::RequestDelegate requests have completed. |
+ int requests_completed_; |
// Is the code in WaitForDownloadToFinish in a message loop waiting for a |
// callback to finish? |
@@ -108,31 +117,22 @@ bool GetTestDataDir(base::FilePath* dir) { |
TemplateURLFetcherTest::TemplateURLFetcherTest() |
: thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP), |
- callbacks_destroyed_(0), |
- add_provider_called_(0), |
+ requests_completed_(0), |
waiting_for_download_(false) { |
base::FilePath test_data_dir; |
CHECK(GetTestDataDir(&test_data_dir)); |
test_server_.ServeFilesFromDirectory(test_data_dir); |
} |
-void TemplateURLFetcherTest::DestroyedCallback() { |
- callbacks_destroyed_++; |
+void TemplateURLFetcherTest::RequestCompletedCallback() { |
+ requests_completed_++; |
if (waiting_for_download_) |
base::MessageLoop::current()->QuitWhenIdle(); |
} |
-void TemplateURLFetcherTest::ConfirmAddSearchProvider( |
- base::ScopedClosureRunner* callback_destruction_notifier, |
- std::unique_ptr<TemplateURL> template_url) { |
- last_callback_template_url_ = std::move(template_url); |
- add_provider_called_++; |
-} |
- |
void TemplateURLFetcherTest::StartDownload( |
const base::string16& keyword, |
const std::string& osdd_file_name, |
- TemplateURLFetcher::ProviderType provider_type, |
bool check_that_file_exists) { |
if (check_that_file_exists) { |
base::FilePath osdd_full_path; |
@@ -145,18 +145,9 @@ void TemplateURLFetcherTest::StartDownload( |
// Start the fetch. |
GURL osdd_url = test_server_.GetURL("/" + osdd_file_name); |
GURL favicon_url; |
- base::ScopedClosureRunner* callback_destruction_notifier = |
- new base::ScopedClosureRunner( |
- base::Bind(&TemplateURLFetcherTest::DestroyedCallback, |
- base::Unretained(this))); |
- |
template_url_fetcher_->ScheduleDownload( |
keyword, osdd_url, favicon_url, |
- TemplateURLFetcher::URLFetcherCustomizeCallback(), |
- base::Bind(&TemplateURLFetcherTest::ConfirmAddSearchProvider, |
- base::Unretained(this), |
- base::Owned(callback_destruction_notifier)), |
- provider_type); |
+ TemplateURLFetcher::URLFetcherCustomizeCallback()); |
} |
void TemplateURLFetcherTest::WaitForDownloadToFinish() { |
@@ -173,14 +164,11 @@ TEST_F(TemplateURLFetcherTest, BasicAutodetectedTest) { |
ASSERT_FALSE(test_util()->model()->GetTemplateURLForKeyword(keyword)); |
std::string osdd_file_name("simple_open_search.xml"); |
- StartDownload(keyword, osdd_file_name, |
- TemplateURLFetcher::AUTODETECTED_PROVIDER, true); |
- ASSERT_EQ(0, add_provider_called()); |
- ASSERT_EQ(0, callbacks_destroyed()); |
+ StartDownload(keyword, osdd_file_name, true); |
+ ASSERT_EQ(0, requests_completed()); |
Peter Kasting
2016/05/09 22:23:59
How come here and the next two modified lines foll
Evan Stade
2016/05/09 22:54:53
no reason, should be expect.
|
WaitForDownloadToFinish(); |
- ASSERT_EQ(0, add_provider_called()); |
- ASSERT_EQ(1, callbacks_destroyed()); |
+ ASSERT_EQ(1, requests_completed()); |
const TemplateURL* t_url = test_util()->model()->GetTemplateURLForKeyword( |
keyword); |
@@ -199,97 +187,38 @@ TEST_F(TemplateURLFetcherTest, DuplicatesThrownAway) { |
ASSERT_FALSE(test_util()->model()->GetTemplateURLForKeyword(keyword)); |
std::string osdd_file_name("simple_open_search.xml"); |
- StartDownload(keyword, osdd_file_name, |
- TemplateURLFetcher::AUTODETECTED_PROVIDER, true); |
- ASSERT_EQ(0, add_provider_called()); |
- ASSERT_EQ(0, callbacks_destroyed()); |
+ StartDownload(keyword, osdd_file_name, true); |
+ ASSERT_EQ(0, requests_completed()); |
struct { |
std::string description; |
std::string osdd_file_name; |
base::string16 keyword; |
- TemplateURLFetcher::ProviderType provider_type; |
} test_cases[] = { |
- { "Duplicate osdd url with autodetected provider.", osdd_file_name, |
- keyword + ASCIIToUTF16("1"), |
- TemplateURLFetcher::AUTODETECTED_PROVIDER }, |
- { "Duplicate keyword with autodetected provider.", osdd_file_name + "1", |
- keyword, TemplateURLFetcher::AUTODETECTED_PROVIDER }, |
- { "Duplicate osdd url with explicit provider.", osdd_file_name, |
- base::string16(), TemplateURLFetcher::EXPLICIT_PROVIDER }, |
+ {"Duplicate osdd url with autodetected provider.", osdd_file_name, |
+ keyword + ASCIIToUTF16("1")}, |
+ {"Duplicate keyword with autodetected provider.", osdd_file_name + "1", |
+ keyword}, |
}; |
for (size_t i = 0; i < arraysize(test_cases); ++i) { |
- StartDownload(test_cases[i].keyword, test_cases[i].osdd_file_name, |
- test_cases[i].provider_type, false); |
- ASSERT_EQ(1, template_url_fetcher()->requests_count()) |
+ StartDownload(test_cases[i].keyword, test_cases[i].osdd_file_name, false); |
+ EXPECT_EQ(1, template_url_fetcher()->requests_count()) |
<< test_cases[i].description; |
- ASSERT_EQ(i + 1, static_cast<size_t>(callbacks_destroyed())); |
} |
WaitForDownloadToFinish(); |
- ASSERT_EQ(1 + arraysize(test_cases), |
- static_cast<size_t>(callbacks_destroyed())); |
- ASSERT_EQ(0, add_provider_called()); |
-} |
- |
-TEST_F(TemplateURLFetcherTest, BasicExplicitTest) { |
- base::string16 keyword(ASCIIToUTF16("test")); |
- |
- test_util()->ChangeModelToLoadState(); |
- ASSERT_FALSE(test_util()->model()->GetTemplateURLForKeyword(keyword)); |
- |
- std::string osdd_file_name("simple_open_search.xml"); |
- StartDownload(keyword, osdd_file_name, |
- TemplateURLFetcher::EXPLICIT_PROVIDER, true); |
- ASSERT_EQ(0, add_provider_called()); |
- ASSERT_EQ(0, callbacks_destroyed()); |
- |
- WaitForDownloadToFinish(); |
- ASSERT_EQ(1, add_provider_called()); |
- ASSERT_EQ(1, callbacks_destroyed()); |
- |
- ASSERT_TRUE(last_callback_template_url()); |
- EXPECT_EQ(ASCIIToUTF16("http://example.com/%s/other_stuff"), |
- last_callback_template_url()->url_ref().DisplayURL( |
- test_util()->model()->search_terms_data())); |
- EXPECT_EQ(ASCIIToUTF16("example.com"), |
- last_callback_template_url()->keyword()); |
- EXPECT_FALSE(last_callback_template_url()->safe_for_autoreplace()); |
+ EXPECT_EQ(1, requests_completed()); |
} |
TEST_F(TemplateURLFetcherTest, AutodetectedBeforeLoadTest) { |
base::string16 keyword(ASCIIToUTF16("test")); |
- ASSERT_FALSE(test_util()->model()->GetTemplateURLForKeyword(keyword)); |
+ EXPECT_FALSE(test_util()->model()->GetTemplateURLForKeyword(keyword)); |
std::string osdd_file_name("simple_open_search.xml"); |
- StartDownload(keyword, osdd_file_name, |
- TemplateURLFetcher::AUTODETECTED_PROVIDER, true); |
- ASSERT_EQ(0, add_provider_called()); |
- ASSERT_EQ(1, callbacks_destroyed()); |
-} |
- |
-TEST_F(TemplateURLFetcherTest, ExplicitBeforeLoadTest) { |
- base::string16 keyword(ASCIIToUTF16("test")); |
- ASSERT_FALSE(test_util()->model()->GetTemplateURLForKeyword(keyword)); |
- |
- std::string osdd_file_name("simple_open_search.xml"); |
- StartDownload(keyword, osdd_file_name, |
- TemplateURLFetcher::EXPLICIT_PROVIDER, true); |
- ASSERT_EQ(0, add_provider_called()); |
- ASSERT_EQ(0, callbacks_destroyed()); |
- |
- WaitForDownloadToFinish(); |
- ASSERT_EQ(1, add_provider_called()); |
- ASSERT_EQ(1, callbacks_destroyed()); |
- |
- ASSERT_TRUE(last_callback_template_url()); |
- EXPECT_EQ(ASCIIToUTF16("http://example.com/%s/other_stuff"), |
- last_callback_template_url()->url_ref().DisplayURL( |
- test_util()->model()->search_terms_data())); |
- EXPECT_EQ(ASCIIToUTF16("example.com"), |
- last_callback_template_url()->keyword()); |
- EXPECT_FALSE(last_callback_template_url()->safe_for_autoreplace()); |
+ StartDownload(keyword, osdd_file_name, true); |
+ EXPECT_EQ(0, template_url_fetcher()->requests_count()); |
+ EXPECT_EQ(0, requests_completed()); |
Peter Kasting
2016/05/09 22:23:59
It seems like this test gets the same results as t
Evan Stade
2016/05/09 22:54:53
I think the name of the test explains why the requ
Peter Kasting
2016/05/09 22:57:32
Maybe just add a comment like "This should bail be
Evan Stade
2016/05/10 16:51:13
Done.
|
} |
TEST_F(TemplateURLFetcherTest, DuplicateKeywordsTest) { |
@@ -301,35 +230,31 @@ TEST_F(TemplateURLFetcherTest, DuplicateKeywordsTest) { |
test_util()->model()->Add(new TemplateURL(data)); |
test_util()->ChangeModelToLoadState(); |
- ASSERT_TRUE(test_util()->model()->GetTemplateURLForKeyword(keyword)); |
+ EXPECT_TRUE(test_util()->model()->GetTemplateURLForKeyword(keyword)); |
// This should bail because the keyword already exists. |
std::string osdd_file_name("simple_open_search.xml"); |
- StartDownload(keyword, osdd_file_name, |
- TemplateURLFetcher::AUTODETECTED_PROVIDER, true); |
- ASSERT_EQ(0, add_provider_called()); |
- ASSERT_EQ(1, callbacks_destroyed()); |
- ASSERT_FALSE(last_callback_template_url()); |
+ StartDownload(keyword, osdd_file_name, true); |
+ EXPECT_EQ(0, template_url_fetcher()->requests_count()); |
+ EXPECT_EQ(0, requests_completed()); |
} |
TEST_F(TemplateURLFetcherTest, DuplicateDownloadTest) { |
+ test_util()->ChangeModelToLoadState(); |
+ |
base::string16 keyword(ASCIIToUTF16("test")); |
std::string osdd_file_name("simple_open_search.xml"); |
- StartDownload(keyword, osdd_file_name, |
- TemplateURLFetcher::EXPLICIT_PROVIDER, true); |
- ASSERT_EQ(0, add_provider_called()); |
- ASSERT_EQ(0, callbacks_destroyed()); |
+ StartDownload(keyword, osdd_file_name, true); |
+ EXPECT_EQ(1, template_url_fetcher()->requests_count()); |
+ EXPECT_EQ(0, requests_completed()); |
// This should bail because the keyword already has a pending download. |
- StartDownload(keyword, osdd_file_name, |
- TemplateURLFetcher::EXPLICIT_PROVIDER, true); |
- ASSERT_EQ(0, add_provider_called()); |
- ASSERT_EQ(1, callbacks_destroyed()); |
+ StartDownload(keyword, osdd_file_name, true); |
+ EXPECT_EQ(1, template_url_fetcher()->requests_count()); |
+ EXPECT_EQ(0, requests_completed()); |
WaitForDownloadToFinish(); |
- ASSERT_EQ(1, add_provider_called()); |
- ASSERT_EQ(2, callbacks_destroyed()); |
- ASSERT_TRUE(last_callback_template_url()); |
+ EXPECT_EQ(1, requests_completed()); |
} |
TEST_F(TemplateURLFetcherTest, UnicodeTest) { |
@@ -339,11 +264,12 @@ TEST_F(TemplateURLFetcherTest, UnicodeTest) { |
ASSERT_FALSE(test_util()->model()->GetTemplateURLForKeyword(keyword)); |
std::string osdd_file_name("unicode_open_search.xml"); |
- StartDownload(keyword, osdd_file_name, |
- TemplateURLFetcher::AUTODETECTED_PROVIDER, true); |
+ StartDownload(keyword, osdd_file_name, true); |
WaitForDownloadToFinish(); |
const TemplateURL* t_url = |
test_util()->model()->GetTemplateURLForKeyword(keyword); |
EXPECT_EQ(base::UTF8ToUTF16("\xd1\x82\xd0\xb5\xd1\x81\xd1\x82"), |
t_url->short_name()); |
} |
+ |
+} // namespace |