| Index: chrome/browser/history/top_sites_unittest.cc
 | 
| ===================================================================
 | 
| --- chrome/browser/history/top_sites_unittest.cc	(revision 61074)
 | 
| +++ chrome/browser/history/top_sites_unittest.cc	(working copy)
 | 
| @@ -22,6 +22,7 @@
 | 
|  #include "grit/chromium_strings.h"
 | 
|  #include "grit/generated_resources.h"
 | 
|  #include "grit/locale_settings.h"
 | 
| +#include "testing/gmock/include/gmock/gmock.h"
 | 
|  #include "testing/gtest/include/gtest/gtest.h"
 | 
|  #include "third_party/skia/include/core/SkBitmap.h"
 | 
|  
 | 
| @@ -129,19 +130,34 @@
 | 
|  
 | 
|  
 | 
|  // A mockup of a HistoryService used for testing TopSites.
 | 
| -class MockHistoryServiceImpl : public TopSites::MockHistoryService {
 | 
| +class MockHistoryServiceImpl
 | 
| +    : public base::RefCountedThreadSafe<MockHistoryServiceImpl>,
 | 
| +      public TopSites::MockHistoryService {
 | 
|   public:
 | 
| -  MockHistoryServiceImpl() : num_thumbnail_requests_(0) {}
 | 
| +  MockHistoryServiceImpl() : num_thumbnail_requests_(0), handle_num_(0) {}
 | 
|  
 | 
|    // Calls the callback directly with the results.
 | 
|    HistoryService::Handle QueryMostVisitedURLs(
 | 
|        int result_count, int days_back,
 | 
|        CancelableRequestConsumerBase* consumer,
 | 
|        HistoryService::QueryMostVisitedURLsCallback* callback) {
 | 
| -    callback->Run(CancelableRequestProvider::Handle(0),  // Handle is unused.
 | 
| -                  most_visited_urls_);
 | 
| +    CancelableRequestProvider::Handle handle(++handle_num_);
 | 
| +    // Run the callback on another task in order that the handle can be
 | 
| +    // returned before the callback is run, which is necessary for the
 | 
| +    // refresh callback mechanism to work in |TopSites|.
 | 
| +    MessageLoop::current()->PostTask(
 | 
| +        FROM_HERE,
 | 
| +        NewRunnableMethod(this,
 | 
| +                          &MockHistoryServiceImpl::RunCallback,
 | 
| +                          callback,
 | 
| +                          handle));
 | 
| +    return handle;
 | 
| +  }
 | 
| +
 | 
| +  void RunCallback(HistoryService::QueryMostVisitedURLsCallback* callback,
 | 
| +                   CancelableRequestProvider::Handle handle) {
 | 
| +    callback->Run(handle, most_visited_urls_);
 | 
|      delete callback;
 | 
| -    return 0;
 | 
|    }
 | 
|  
 | 
|    // Add a page to the end of the pages list.
 | 
| @@ -188,6 +204,7 @@
 | 
|   private:
 | 
|    MostVisitedURLList most_visited_urls_;
 | 
|    int num_thumbnail_requests_;  // Number of calls to GetPageThumbnail.
 | 
| +  int handle_num_;
 | 
|  };
 | 
|  
 | 
|  
 | 
| @@ -464,10 +481,10 @@
 | 
|    GURL news("http://news.google.com/");
 | 
|    GURL google("http://google.com/");
 | 
|  
 | 
| -  MockHistoryServiceImpl hs;
 | 
| -  hs.AppendMockPage(news, ASCIIToUTF16("Google News"));
 | 
| -  hs.AppendMockPage(google, ASCIIToUTF16("Google"));
 | 
| -  top_sites().SetMockHistoryService(&hs);
 | 
| +  scoped_refptr<MockHistoryServiceImpl> hs(new MockHistoryServiceImpl);
 | 
| +  hs->AppendMockPage(news, ASCIIToUTF16("Google News"));
 | 
| +  hs->AppendMockPage(google, ASCIIToUTF16("Google"));
 | 
| +  top_sites().SetMockHistoryService(hs.get());
 | 
|  
 | 
|    top_sites().StartQueryForMostVisited();
 | 
|    MessageLoop::current()->RunAllPending();
 | 
| @@ -536,11 +553,11 @@
 | 
|    EXPECT_EQ(welcome_url(), urls()[2].url);
 | 
|    EXPECT_EQ(themes_url(), urls()[3].url);
 | 
|  
 | 
| -  MockHistoryServiceImpl hs;
 | 
| +  scoped_refptr<MockHistoryServiceImpl> hs(new MockHistoryServiceImpl);
 | 
|    // Add one old, one new URL to the history.
 | 
| -  hs.AppendMockPage(google_url, google_title);
 | 
| -  hs.AppendMockPage(news_url, news_title);
 | 
| -  top_sites().SetMockHistoryService(&hs);
 | 
| +  hs->AppendMockPage(google_url, google_title);
 | 
| +  hs->AppendMockPage(news_url, news_title);
 | 
| +  top_sites().SetMockHistoryService(hs.get());
 | 
|  
 | 
|    // This writes the new data to the DB.
 | 
|    top_sites().StartQueryForMostVisited();
 | 
| @@ -717,11 +734,11 @@
 | 
|    EXPECT_EQ(welcome_url(), urls()[2].url);
 | 
|    EXPECT_EQ(themes_url(), urls()[3].url);
 | 
|  
 | 
| -  MockHistoryServiceImpl hs;
 | 
| +  scoped_refptr<MockHistoryServiceImpl> hs(new MockHistoryServiceImpl);
 | 
|    // Add one old, one new URL to the history.
 | 
| -  hs.AppendMockPage(google1_url, google_title);
 | 
| -  hs.AppendMockPage(news_url, news_title);
 | 
| -  top_sites().SetMockHistoryService(&hs);
 | 
| +  hs->AppendMockPage(google1_url, google_title);
 | 
| +  hs->AppendMockPage(news_url, news_title);
 | 
| +  top_sites().SetMockHistoryService(hs.get());
 | 
|  
 | 
|    // This requests data from History Service and  writes it to the DB.
 | 
|    top_sites().StartQueryForMostVisited();
 | 
| @@ -789,13 +806,13 @@
 | 
|    GURL news_url("http://news.google.com");
 | 
|    string16 news_title(ASCIIToUTF16("Google News"));
 | 
|  
 | 
| -  MockHistoryServiceImpl hs;
 | 
| +  scoped_refptr<MockHistoryServiceImpl> hs(new MockHistoryServiceImpl);
 | 
|  
 | 
|    top_sites().Init(file_name());
 | 
|  
 | 
| -  hs.AppendMockPage(google1_url, google_title);
 | 
| -  hs.AppendMockPage(news_url, news_title);
 | 
| -  top_sites().SetMockHistoryService(&hs);
 | 
| +  hs->AppendMockPage(google1_url, google_title);
 | 
| +  hs->AppendMockPage(news_url, news_title);
 | 
| +  top_sites().SetMockHistoryService(hs.get());
 | 
|  
 | 
|    top_sites().StartQueryForMostVisited();
 | 
|    MessageLoop::current()->RunAllPending();
 | 
| @@ -807,7 +824,7 @@
 | 
|    // 2 extra prepopulated URLs.
 | 
|    ASSERT_EQ(4u, urls().size());
 | 
|  
 | 
| -  hs.RemoveMostVisitedURL();
 | 
| +  hs->RemoveMostVisitedURL();
 | 
|  
 | 
|    history::URLsDeletedDetails history_details;
 | 
|    history_details.all_history = false;
 | 
| @@ -826,7 +843,7 @@
 | 
|    EXPECT_EQ(welcome_url(), urls()[1].url);
 | 
|    EXPECT_EQ(themes_url(), urls()[2].url);
 | 
|  
 | 
| -  hs.RemoveMostVisitedURL();
 | 
| +  hs->RemoveMostVisitedURL();
 | 
|    history_details.all_history = true;
 | 
|    details = Details<HistoryDetails>(&history_details);
 | 
|    top_sites().Observe(NotificationType::HISTORY_URLS_DELETED,
 | 
| @@ -851,13 +868,13 @@
 | 
|    GURL news_url("http://news.google.com");
 | 
|    string16 news_title(ASCIIToUTF16("Google News"));
 | 
|  
 | 
| -  MockHistoryServiceImpl hs;
 | 
| +  scoped_refptr<MockHistoryServiceImpl> hs(new MockHistoryServiceImpl);
 | 
|  
 | 
|    top_sites().Init(file_name());
 | 
|  
 | 
| -  hs.AppendMockPage(google1_url, google_title);
 | 
| -  hs.AppendMockPage(news_url, news_title);
 | 
| -  top_sites().SetMockHistoryService(&hs);
 | 
| +  hs->AppendMockPage(google1_url, google_title);
 | 
| +  hs->AppendMockPage(news_url, news_title);
 | 
| +  top_sites().SetMockHistoryService(hs.get());
 | 
|  
 | 
|    top_sites().StartQueryForMostVisited();
 | 
|    MessageLoop::current()->RunAllPending();
 | 
| @@ -873,7 +890,7 @@
 | 
|    top_sites().AddPinnedURL(news_url, 3);
 | 
|    EXPECT_TRUE(top_sites().IsURLPinned(news_url));
 | 
|  
 | 
| -  hs.RemoveMostVisitedURL();
 | 
| +  hs->RemoveMostVisitedURL();
 | 
|    history::URLsDeletedDetails history_details;
 | 
|    history_details.all_history = false;
 | 
|    history_details.urls.insert(news_url);
 | 
| @@ -891,7 +908,7 @@
 | 
|    ASSERT_EQ(3u, urls().size());
 | 
|    EXPECT_FALSE(top_sites().IsURLPinned(news_url));
 | 
|  
 | 
| -  hs.RemoveMostVisitedURL();
 | 
| +  hs->RemoveMostVisitedURL();
 | 
|    history_details.all_history = true;
 | 
|    details = Details<HistoryDetails>(&history_details);
 | 
|    top_sites().Observe(NotificationType::HISTORY_URLS_DELETED,
 | 
| @@ -938,19 +955,19 @@
 | 
|    GURL news_url("http://news.google.com");
 | 
|    string16 news_title(ASCIIToUTF16("Google News"));
 | 
|  
 | 
| -  MockHistoryServiceImpl hs;
 | 
| +  scoped_refptr<MockHistoryServiceImpl> hs(new MockHistoryServiceImpl);
 | 
|  
 | 
|    top_sites().Init(file_name());
 | 
|  
 | 
| -  hs.AppendMockPage(google1_url, google_title);
 | 
| -  hs.AppendMockPage(news_url, news_title);
 | 
| -  top_sites().SetMockHistoryService(&hs);
 | 
| +  hs->AppendMockPage(google1_url, google_title);
 | 
| +  hs->AppendMockPage(news_url, news_title);
 | 
| +  top_sites().SetMockHistoryService(hs.get());
 | 
|    MessageLoop::current()->RunAllPending();
 | 
|  
 | 
|    top_sites().StartMigration();
 | 
|    EXPECT_TRUE(top_sites().migration_in_progress_);
 | 
|    MessageLoop::current()->RunAllPending();
 | 
| -  EXPECT_EQ(2, hs.GetNumberOfThumbnailRequests());
 | 
| +  EXPECT_EQ(2, hs->GetNumberOfThumbnailRequests());
 | 
|    EXPECT_FALSE(top_sites().migration_in_progress_);
 | 
|  }
 | 
|  
 | 
| @@ -985,7 +1002,7 @@
 | 
|    url.url = GURL("http://2.com/");
 | 
|    url.redirects.push_back(url.url);
 | 
|    pages.push_back(url);
 | 
| -  top_sites().OnTopSitesAvailable(0, pages);
 | 
| +  top_sites().OnTopSitesAvailable(1, pages);
 | 
|    MessageLoop::current()->RunAllPending();
 | 
|  
 | 
|    EXPECT_EQ(3u, number_of_callbacks());
 | 
| @@ -1000,7 +1017,7 @@
 | 
|    url.url = GURL("http://3.com/");
 | 
|    url.redirects.push_back(url.url);
 | 
|    pages.push_back(url);
 | 
| -  top_sites().OnTopSitesAvailable(0, pages);
 | 
| +  top_sites().OnTopSitesAvailable(1, pages);
 | 
|    MessageLoop::current()->RunAllPending();
 | 
|  
 | 
|    top_sites().GetMostVisitedURLs(
 | 
| @@ -1054,7 +1071,7 @@
 | 
|    url.url = GURL("http://2.com/");
 | 
|    pages.push_back(url);
 | 
|  
 | 
| -  top_sites().OnTopSitesAvailable(0, pages);
 | 
| +  top_sites().OnTopSitesAvailable(1, pages);
 | 
|    MessageLoop::current()->RunAllPending();
 | 
|  
 | 
|    // 1 request was canceled.
 | 
| @@ -1125,7 +1142,7 @@
 | 
|        &c,
 | 
|        NewCallback(static_cast<TopSitesTest*>(this),
 | 
|                    &TopSitesTest::OnTopSitesAvailable));
 | 
| -  top_sites().OnTopSitesAvailable(0, pages);
 | 
| +  top_sites().OnTopSitesAvailable(1, pages);
 | 
|    MessageLoop::current()->RunAllPending();
 | 
|    {
 | 
|      Lock& l = lock();
 | 
| @@ -1219,7 +1236,7 @@
 | 
|        &c,
 | 
|        NewCallback(static_cast<TopSitesTest*>(this),
 | 
|                    &TopSitesTest::OnTopSitesAvailable));
 | 
| -  top_sites().OnTopSitesAvailable(0, pages);
 | 
| +  top_sites().OnTopSitesAvailable(1, pages);
 | 
|    MessageLoop::current()->RunAllPending();
 | 
|    EXPECT_FALSE(top_sites().IsURLPinned(GURL("http://bbc.com/")));
 | 
|  
 | 
| @@ -1296,7 +1313,7 @@
 | 
|        &c,
 | 
|        NewCallback(static_cast<TopSitesTest*>(this),
 | 
|                    &TopSitesTest::OnTopSitesAvailable));
 | 
| -  top_sites().OnTopSitesAvailable(0, pages);
 | 
| +  top_sites().OnTopSitesAvailable(1, pages);
 | 
|    MessageLoop::current()->RunAllPending();
 | 
|  
 | 
|    ASSERT_EQ(2u, urls().size());
 | 
| @@ -1337,4 +1354,45 @@
 | 
|    EXPECT_EQ(welcome_url(), pages[1].url);
 | 
|  }
 | 
|  
 | 
| +class MockRefreshObserver {
 | 
| + public:
 | 
| +  MOCK_METHOD0(OnRefresh, void ());  // NOLINT
 | 
| +};
 | 
| +
 | 
| +ACTION_P3(GetMostVisitedURLs, top_sites, consumer, callback_obj) {
 | 
| +  top_sites->GetMostVisitedURLs(
 | 
| +      consumer,
 | 
| +      NewCallback(static_cast<TopSitesTest*>(callback_obj),
 | 
| +                  &TopSitesTest::OnTopSitesAvailable));
 | 
| +}
 | 
| +
 | 
| +TEST_F(TopSitesTest, RefreshCallback) {
 | 
| +  ChromeThread db_loop(ChromeThread::DB, MessageLoop::current());
 | 
| +
 | 
| +  scoped_refptr<MockHistoryServiceImpl> hs(new MockHistoryServiceImpl);
 | 
| +  hs->AppendMockPage(GURL("http://google.com"), ASCIIToUTF16("Google"));
 | 
| +  top_sites().SetMockHistoryService(hs.get());
 | 
| +
 | 
| +  top_sites().StartQueryForMostVisited();
 | 
| +  MessageLoop::current()->RunAllPending();
 | 
| +
 | 
| +  // Test that the callback is called.
 | 
| +  MockRefreshObserver refresh_mock;
 | 
| +  EXPECT_CALL(refresh_mock, OnRefresh())
 | 
| +      .WillOnce(GetMostVisitedURLs(&top_sites(), consumer(), this));
 | 
| +  top_sites().RefreshAndCallback(
 | 
| +      consumer(),
 | 
| +      NewCallback(&refresh_mock, &MockRefreshObserver::OnRefresh));
 | 
| +  MessageLoop::current()->RunAllPending();
 | 
| +  MessageLoop::current()->RunAllPending();
 | 
| +  ASSERT_EQ(3u, urls().size());
 | 
| +
 | 
| +  // Test that the callback is not called again.
 | 
| +  top_sites().GetMostVisitedURLs(
 | 
| +      consumer(),
 | 
| +      NewCallback(static_cast<TopSitesTest*>(this),
 | 
| +                  &TopSitesTest::OnTopSitesAvailable));
 | 
| +  MessageLoop::current()->RunAllPending();
 | 
| +}
 | 
| +
 | 
|  }  // namespace history
 | 
| 
 |