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

Unified Diff: chrome/browser/autocomplete/history_quick_provider_unittest.cc

Issue 1368293012: HistoryQuickProviderTest.DeleteMatch thread safety fix. (Closed) Base URL: http://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 3 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/autocomplete/history_quick_provider_unittest.cc
diff --git a/chrome/browser/autocomplete/history_quick_provider_unittest.cc b/chrome/browser/autocomplete/history_quick_provider_unittest.cc
index acdf836e35d93e6c5097483edaa52beef07b16d1..95c61a872e1832b6e65b52d745145086124935f6 100644
--- a/chrome/browser/autocomplete/history_quick_provider_unittest.cc
+++ b/chrome/browser/autocomplete/history_quick_provider_unittest.cc
@@ -208,10 +208,24 @@ class HistoryQuickProviderTest : public testing::Test {
base::string16 expected_fill_into_edit,
base::string16 autocompletion);
+ // TODO(shess): From history_service.h in reference to history_backend:
+ // > This class has most of the implementation and runs on the 'thread_'.
+ // > You MUST communicate with this class ONLY through the thread_'s
+ // > message_loop().
+ // Direct use of this object in tests is almost certainly not thread-safe.
history::HistoryBackend* history_backend() {
return history_service_->history_backend_.get();
}
+ // Call history_backend()->GetURL(url, NULL) on the history thread, returning
+ // the result.
+ bool GetURLProxy(const GURL& url);
+
+ // Background task posted by GetURLProxy().
+ static void GetURLTask(history::HistoryBackend* backend,
+ const GURL& url,
+ bool* result_storage);
+
base::MessageLoopForUI message_loop_;
content::TestBrowserThread ui_thread_;
content::TestBrowserThread file_thread_;
@@ -394,6 +408,24 @@ void HistoryQuickProviderTest::RunTestWithCursor(
EXPECT_EQ(expected_fill_into_edit, ac_matches_[0].fill_into_edit);
}
+bool HistoryQuickProviderTest::GetURLProxy(const GURL& url) {
+ bool result = false;
+ base::RunLoop runner;
+ history_service_->PostTaskAndReplyForTest(
+ base::Bind(&GetURLTask, base::Unretained(history_backend()),
+ url, &result),
+ runner.QuitClosure());
+ runner.Run();
+ return result;
+}
+
+// static
+void HistoryQuickProviderTest::GetURLTask(history::HistoryBackend* backend,
+ const GURL& url,
+ bool* result_storage) {
+ *result_storage = backend->GetURL(url, NULL);
+}
+
TEST_F(HistoryQuickProviderTest, SimpleSingleMatch) {
std::vector<std::string> expected_urls;
expected_urls.push_back("http://slashdot.org/favorite_page.html");
@@ -616,7 +648,7 @@ TEST_F(HistoryQuickProviderTest, DeleteMatch) {
ASCIIToUTF16("slashdot.org/favorite_page.html"),
ASCIIToUTF16(".org/favorite_page.html"));
EXPECT_EQ(1U, ac_matches_.size());
- EXPECT_TRUE(history_backend()->GetURL(test_url, NULL));
+ EXPECT_TRUE(GetURLProxy(test_url));
provider_->DeleteMatch(ac_matches_[0]);
// Check that the underlying URL is deleted from the history DB (this implies
@@ -626,7 +658,7 @@ TEST_F(HistoryQuickProviderTest, DeleteMatch) {
// To ensure that the deletion has been propagated everywhere before we start
// verifying post-deletion states, first wait until we see the notification.
WaitForURLsDeletedNotification(history_service_);
- EXPECT_FALSE(history_backend()->GetURL(test_url, NULL));
+ EXPECT_FALSE(GetURLProxy(test_url));
// Just to be on the safe side, explicitly verify that we have deleted enough
// data so that we will not be serving the same result again.
« no previous file with comments | « no previous file | components/history/core/browser/history_service.h » ('j') | components/history/core/browser/history_service.h » ('J')

Powered by Google App Engine
This is Rietveld 408576698