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

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: Convert to HistoryDBTask. 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
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..dd9d84023771c35c93ad23bcc16e8c8211d5f941 100644
--- a/chrome/browser/autocomplete/history_quick_provider_unittest.cc
+++ b/chrome/browser/autocomplete/history_quick_provider_unittest.cc
@@ -51,6 +51,8 @@ using base::TimeDelta;
using content::BrowserThread;
+namespace {
+
struct TestURLInfo {
std::string url;
std::string title;
@@ -150,6 +152,36 @@ void WaitForURLsDeletedNotification(history::HistoryService* history_service) {
runner.Run();
}
+// Post history_backend->GetURL() to the history thread and stop the originating
+// thread's message loop when done.
+class GetURLTask : public history::HistoryDBTask {
+ public:
+ GetURLTask(const GURL& url, bool* result_storage)
+ : result_storage_(result_storage),
+ url_(url) {
+ }
+
+ bool RunOnDBThread(history::HistoryBackend* backend,
+ history::HistoryDatabase* db) override {
+ *result_storage_ = backend->GetURL(url_, NULL);
+ return true;
+ }
+
+ void DoneRunOnMainThread() override {
+ base::MessageLoop::current()->Quit();
+ }
+
+ private:
+ ~GetURLTask() override {}
+
+ bool* result_storage_;
+ const GURL url_;
+
+ DISALLOW_COPY_AND_ASSIGN(GetURLTask);
+};
+
+} // namespace
+
class HistoryQuickProviderTest : public testing::Test {
public:
HistoryQuickProviderTest()
@@ -208,10 +240,19 @@ 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);
+
base::MessageLoopForUI message_loop_;
content::TestBrowserThread ui_thread_;
content::TestBrowserThread file_thread_;
@@ -394,6 +435,18 @@ void HistoryQuickProviderTest::RunTestWithCursor(
EXPECT_EQ(expected_fill_into_edit, ac_matches_[0].fill_into_edit);
}
+bool HistoryQuickProviderTest::GetURLProxy(const GURL& url) {
+ base::CancelableTaskTracker task_tracker;
+ bool result = false;
+ history_service_->ScheduleDBTask(
+ scoped_ptr<history::HistoryDBTask>(new GetURLTask(url, &result)),
+ &task_tracker);
+ // Run the message loop until GetURLTask::DoneRunOnMainThread stops it. If
+ // the test hangs, DoneRunOnMainThread isn't being invoked correctly.
+ base::MessageLoop::current()->Run();
+ return result;
+}
+
TEST_F(HistoryQuickProviderTest, SimpleSingleMatch) {
std::vector<std::string> expected_urls;
expected_urls.push_back("http://slashdot.org/favorite_page.html");
@@ -616,7 +669,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 +679,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 | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698