Get rid of BuildCallback() helper functions.

Replace them with const Callback objects created in the constructor.
(This is simpler, and the way most of the code already does it.)

R=rouslan@chromium.org
BUG=

Review URL: https://codereview.appspot.com/116980043

git-svn-id: http://libaddressinput.googlecode.com/svn/trunk@312 38ededc0-08b8-5190-f2ac-b31f878777ad

6 files changed, 60 insertions, 93 deletions

diff --git a/cpp/test/fake_downloader_test.cc b/cpp/test/fake_downloader_test.cc
index 2d9825f..29a3983 100644
--- a/cpp/test/fake_downloader_test.cc
+++ b/cpp/test/fake_downloader_test.cc
@@ -28,6 +28,7 @@
 
 namespace {
 
+using i18n::addressinput::BuildCallback;
 using i18n::addressinput::Downloader;
 using i18n::addressinput::FakeDownloader;
 using i18n::addressinput::RegionDataConstants;
@@ -36,18 +37,20 @@ using i18n::addressinput::scoped_ptr;
 // Tests for FakeDownloader object.
 class FakeDownloaderTest : public testing::TestWithParam<std::string> {
  protected:
-  FakeDownloaderTest() : downloader_(), success_(false), url_(), data_() {}
-  virtual ~FakeDownloaderTest() {}
+  FakeDownloaderTest()
+      : downloader_(),
+        success_(false),
+        url_(),
+        data_(),
+        downloaded_(BuildCallback(this, &FakeDownloaderTest::OnDownloaded)) {}
 
-  Downloader::Callback* BuildCallback() {
-    return i18n::addressinput::BuildCallback(
-        this, &FakeDownloaderTest::OnDownloaded);
-  }
+  virtual ~FakeDownloaderTest() {}
 
   FakeDownloader downloader_;
   bool success_;
   std::string url_;
   std::string data_;
+  const scoped_ptr<const Downloader::Callback> downloaded_;
 
  private:
   void OnDownloaded(bool success, const std::string& url, std::string* data) {
@@ -95,8 +98,7 @@ testing::AssertionResult DataIsValid(const std::string& data,
 TEST_P(FakeDownloaderTest, FakeDownloaderHasValidDataForRegion) {
   std::string key = "data/" + GetParam();
   std::string url = std::string(FakeDownloader::kFakeDataUrl) + key;
-  const scoped_ptr<const Downloader::Callback> callback(BuildCallback());
-  downloader_.Download(url, *callback);
+  downloader_.Download(url, *downloaded_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ(url, url_);
@@ -136,8 +138,7 @@ testing::AssertionResult AggregateDataIsValid(const std::string& data,
 TEST_P(FakeDownloaderTest, FakeDownloaderHasValidAggregatedDataForRegion) {
   std::string key = "data/" + GetParam();
   std::string url = std::string(FakeDownloader::kFakeAggregateDataUrl) + key;
-  const scoped_ptr<const Downloader::Callback> callback(BuildCallback());
-  downloader_.Download(url, *callback);
+  downloader_.Download(url, *downloaded_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ(url, url_);
@@ -154,8 +155,7 @@ TEST_F(FakeDownloaderTest, DownloadExistingData) {
   static const std::string kKey = "data";
   static const std::string kUrl =
       std::string(FakeDownloader::kFakeDataUrl) + kKey;
-  const scoped_ptr<const Downloader::Callback> callback(BuildCallback());
-  downloader_.Download(kUrl, *callback);
+  downloader_.Download(kUrl, *downloaded_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ(kUrl, url_);
@@ -166,8 +166,7 @@ TEST_F(FakeDownloaderTest, DownloadExistingData) {
 TEST_F(FakeDownloaderTest, DownloadMissingKeyReturnsEmptyDictionary) {
   static const std::string kJunkUrl =
       std::string(FakeDownloader::kFakeDataUrl) + "junk";
-  const scoped_ptr<const Downloader::Callback> callback(BuildCallback());
-  downloader_.Download(kJunkUrl, *callback);
+  downloader_.Download(kJunkUrl, *downloaded_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ(kJunkUrl, url_);
@@ -178,8 +177,7 @@ TEST_F(FakeDownloaderTest, DownloadMissingKeyReturnsEmptyDictionary) {
 TEST_F(FakeDownloaderTest, AggregateDownloadMissingKeyReturnsEmptyDictionary) {
   static const std::string kJunkUrl =
       std::string(FakeDownloader::kFakeAggregateDataUrl) + "junk";
-  const scoped_ptr<const Downloader::Callback> callback(BuildCallback());
-  downloader_.Download(kJunkUrl, *callback);
+  downloader_.Download(kJunkUrl, *downloaded_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ(kJunkUrl, url_);
@@ -189,8 +187,7 @@ TEST_F(FakeDownloaderTest, AggregateDownloadMissingKeyReturnsEmptyDictionary) {
 // Verifies that downloading an empty key will return "{}".
 TEST_F(FakeDownloaderTest, DownloadEmptyKeyReturnsEmptyDictionary) {
   static const std::string kPrefixOnlyUrl = FakeDownloader::kFakeDataUrl;
-  const scoped_ptr<const Downloader::Callback> callback(BuildCallback());
-  downloader_.Download(kPrefixOnlyUrl, *callback);
+  downloader_.Download(kPrefixOnlyUrl, *downloaded_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ(kPrefixOnlyUrl, url_);
@@ -200,8 +197,7 @@ TEST_F(FakeDownloaderTest, DownloadEmptyKeyReturnsEmptyDictionary) {
 // Verifies that downloading a real URL fails.
 TEST_F(FakeDownloaderTest, DownloadRealUrlFals) {
   static const std::string kRealUrl = "http://www.google.com/";
-  const scoped_ptr<const Downloader::Callback> callback(BuildCallback());
-  downloader_.Download(kRealUrl, *callback);
+  downloader_.Download(kRealUrl, *downloaded_);
 
   EXPECT_FALSE(success_);
   EXPECT_EQ(kRealUrl, url_);
diff --git a/cpp/test/fake_storage_test.cc b/cpp/test/fake_storage_test.cc
index ec1695d..94fed9e 100644
--- a/cpp/test/fake_storage_test.cc
+++ b/cpp/test/fake_storage_test.cc
@@ -26,6 +26,7 @@
 
 namespace {
 
+using i18n::addressinput::BuildCallback;
 using i18n::addressinput::FakeStorage;
 using i18n::addressinput::scoped_ptr;
 using i18n::addressinput::Storage;
@@ -33,18 +34,20 @@ using i18n::addressinput::Storage;
 // Tests for FakeStorage object.
 class FakeStorageTest : public testing::Test {
  protected:
-  FakeStorageTest() : storage_(), success_(false), key_(), data_() {}
-  virtual ~FakeStorageTest() {}
+  FakeStorageTest()
+      : storage_(),
+        success_(false),
+        key_(),
+        data_(),
+        data_ready_(BuildCallback(this, &FakeStorageTest::OnDataReady)) {}
 
-  Storage::Callback* BuildCallback() {
-    return i18n::addressinput::BuildCallback(
-        this, &FakeStorageTest::OnDataReady);
-  }
+  virtual ~FakeStorageTest() {}
 
   FakeStorage storage_;
   bool success_;
   std::string key_;
   std::string data_;
+  const scoped_ptr<const Storage::Callback> data_ready_;
 
  private:
   void OnDataReady(bool success,
@@ -63,8 +66,7 @@ class FakeStorageTest : public testing::Test {
 };
 
 TEST_F(FakeStorageTest, GetWithoutPutReturnsEmptyData) {
-  const scoped_ptr<const Storage::Callback> callback(BuildCallback());
-  storage_.Get("key", *callback);
+  storage_.Get("key", *data_ready_);
 
   EXPECT_FALSE(success_);
   EXPECT_EQ("key", key_);
@@ -73,9 +75,7 @@ TEST_F(FakeStorageTest, GetWithoutPutReturnsEmptyData) {
 
 TEST_F(FakeStorageTest, GetReturnsWhatWasPut) {
   storage_.Put("key", new std::string("value"));
-
-  const scoped_ptr<const Storage::Callback> callback(BuildCallback());
-  storage_.Get("key", *callback);
+  storage_.Get("key", *data_ready_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ("key", key_);
@@ -85,9 +85,7 @@ TEST_F(FakeStorageTest, GetReturnsWhatWasPut) {
 TEST_F(FakeStorageTest, SecondPutOverwritesData) {
   storage_.Put("key", new std::string("bad-value"));
   storage_.Put("key", new std::string("good-value"));
-
-  const scoped_ptr<const Storage::Callback> callback(BuildCallback());
-  storage_.Get("key", *callback);
+  storage_.Get("key", *data_ready_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ("key", key_);
diff --git a/cpp/test/null_storage_test.cc b/cpp/test/null_storage_test.cc
index 82ad0f5..8284554 100644
--- a/cpp/test/null_storage_test.cc
+++ b/cpp/test/null_storage_test.cc
@@ -26,24 +26,23 @@
 
 namespace {
 
+using i18n::addressinput::BuildCallback;
 using i18n::addressinput::NullStorage;
 using i18n::addressinput::scoped_ptr;
 using i18n::addressinput::Storage;
 
 class NullStorageTest : public testing::Test {
  protected:
-  NullStorageTest() {}
-  virtual ~NullStorageTest() {}
+  NullStorageTest()
+      : data_ready_(BuildCallback(this, &NullStorageTest::OnDataReady)) {}
 
-  Storage::Callback* BuildCallback() {
-    return i18n::addressinput::BuildCallback(
-        this, &NullStorageTest::OnDataReady);
-  }
+  virtual ~NullStorageTest() {}
 
   NullStorage storage_;
   bool success_;
   std::string key_;
   std::string data_;
+  const scoped_ptr<const Storage::Callback> data_ready_;
 
   static const char kKey[];
 
@@ -70,8 +69,7 @@ TEST_F(NullStorageTest, Put) {
 }
 
 TEST_F(NullStorageTest, Get) {
-  const scoped_ptr<const Storage::Callback> callback(BuildCallback());
-  storage_.Get(kKey, *callback);
+  storage_.Get(kKey, *data_ready_);
   EXPECT_FALSE(success_);
   EXPECT_EQ(kKey, key_);
   EXPECT_TRUE(data_.empty());
diff --git a/cpp/test/retriever_test.cc b/cpp/test/retriever_test.cc
index 54a752b..978e9f0 100644
--- a/cpp/test/retriever_test.cc
+++ b/cpp/test/retriever_test.cc
@@ -34,6 +34,7 @@
 
 namespace {
 
+using i18n::addressinput::BuildCallback;
 using i18n::addressinput::FakeDownloader;
 using i18n::addressinput::MockDownloader;
 using i18n::addressinput::NullStorage;
@@ -63,19 +64,16 @@ class RetrieverTest : public testing::Test {
                    new NullStorage),
         success_(false),
         key_(),
-        data_() {}
+        data_(),
+        data_ready_(BuildCallback(this, &RetrieverTest::OnDataReady)) {}
 
   virtual ~RetrieverTest() {}
 
-  Retriever::Callback* BuildCallback() {
-    return i18n::addressinput::BuildCallback(
-        this, &RetrieverTest::OnDataReady);
-  }
-
   Retriever retriever_;
   bool success_;
   std::string key_;
   std::string data_;
+  const scoped_ptr<const Retriever::Callback> data_ready_;
 
  private:
   void OnDataReady(bool success,
@@ -90,8 +88,7 @@ class RetrieverTest : public testing::Test {
 };
 
 TEST_F(RetrieverTest, RetrieveData) {
-  const scoped_ptr<const Retriever::Callback> callback(BuildCallback());
-  retriever_.Retrieve(kKey, *callback);
+  retriever_.Retrieve(kKey, *data_ready_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ(kKey, key_);
@@ -100,11 +97,8 @@ TEST_F(RetrieverTest, RetrieveData) {
 }
 
 TEST_F(RetrieverTest, ReadDataFromStorage) {
-  const scoped_ptr<const Retriever::Callback> callback1(BuildCallback());
-  retriever_.Retrieve(kKey, *callback1);
-
-  const scoped_ptr<const Retriever::Callback> callback2(BuildCallback());
-  retriever_.Retrieve(kKey, *callback2);
+  retriever_.Retrieve(kKey, *data_ready_);
+  retriever_.Retrieve(kKey, *data_ready_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ(kKey, key_);
@@ -115,8 +109,7 @@ TEST_F(RetrieverTest, ReadDataFromStorage) {
 TEST_F(RetrieverTest, MissingKeyReturnsEmptyData) {
   static const char kMissingKey[] = "junk";
 
-  const scoped_ptr<const Retriever::Callback> callback(BuildCallback());
-  retriever_.Retrieve(kMissingKey, *callback);
+  retriever_.Retrieve(kMissingKey, *data_ready_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ(kMissingKey, key_);
@@ -129,8 +122,7 @@ TEST_F(RetrieverTest, FaultyDownloader) {
                           new MockDownloader,
                           new NullStorage);
 
-  const scoped_ptr<const Retriever::Callback> callback(BuildCallback());
-  bad_retriever.Retrieve(kKey, *callback);
+  bad_retriever.Retrieve(kKey, *data_ready_);
 
   EXPECT_FALSE(success_);
   EXPECT_EQ(kKey, key_);
@@ -167,8 +159,7 @@ TEST_F(RetrieverTest, UseStaleDataWhenDownloaderFails) {
   Retriever resilient_retriever(
       MockDownloader::kMockDataUrl, new MockDownloader, stale_storage);
 
-  const scoped_ptr<const Retriever::Callback> callback(BuildCallback());
-  resilient_retriever.Retrieve(kKey, *callback);
+  resilient_retriever.Retrieve(kKey, *data_ready_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ(kKey, key_);
@@ -182,8 +173,7 @@ TEST_F(RetrieverTest, DoNotUseStaleDataWhenDownloaderSucceeds) {
   Retriever resilient_retriever(
       FakeDownloader::kFakeDataUrl, new FakeDownloader, stale_storage);
 
-  const scoped_ptr<const Retriever::Callback> callback(BuildCallback());
-  resilient_retriever.Retrieve(kKey, *callback);
+  resilient_retriever.Retrieve(kKey, *data_ready_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ(kKey, key_);
diff --git a/cpp/test/rule_retriever_test.cc b/cpp/test/rule_retriever_test.cc
index bc7fdbf..7442bb4 100644
--- a/cpp/test/rule_retriever_test.cc
+++ b/cpp/test/rule_retriever_test.cc
@@ -29,6 +29,7 @@
 
 namespace {
 
+using i18n::addressinput::BuildCallback;
 using i18n::addressinput::FakeDownloader;
 using i18n::addressinput::NullStorage;
 using i18n::addressinput::Retriever;
@@ -45,19 +46,16 @@ class RuleRetrieverTest : public testing::Test {
                                       new NullStorage)),
         success_(false),
         key_(),
-        rule_() {}
+        rule_(),
+        rule_ready_(BuildCallback(this, &RuleRetrieverTest::OnRuleReady)) {}
 
   virtual ~RuleRetrieverTest() {}
 
-  RuleRetriever::Callback* BuildCallback() {
-    return i18n::addressinput::BuildCallback(
-        this, &RuleRetrieverTest::OnRuleReady);
-  }
-
   RuleRetriever rule_retriever_;
   bool success_;
   std::string key_;
   Rule rule_;
+  const scoped_ptr<const RuleRetriever::Callback> rule_ready_;
 
  private:
   void OnRuleReady(bool success,
@@ -74,8 +72,7 @@ class RuleRetrieverTest : public testing::Test {
 TEST_F(RuleRetrieverTest, ExistingRule) {
   static const char kExistingKey[] = "data/CA";
 
-  const scoped_ptr<const RuleRetriever::Callback> callback(BuildCallback());
-  rule_retriever_.RetrieveRule(kExistingKey, *callback);
+  rule_retriever_.RetrieveRule(kExistingKey, *rule_ready_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ(kExistingKey, key_);
@@ -85,8 +82,7 @@ TEST_F(RuleRetrieverTest, ExistingRule) {
 TEST_F(RuleRetrieverTest, MissingRule) {
   static const char kMissingKey[] = "junk";
 
-  const scoped_ptr<const RuleRetriever::Callback> callback(BuildCallback());
-  rule_retriever_.RetrieveRule(kMissingKey, *callback);
+  rule_retriever_.RetrieveRule(kMissingKey, *rule_ready_);
 
   EXPECT_TRUE(success_);  // The server returns "{}" for bad keys.
   EXPECT_EQ(kMissingKey, key_);
diff --git a/cpp/test/validating_storage_test.cc b/cpp/test/validating_storage_test.cc
index 1efe60e..f1be1cf 100644
--- a/cpp/test/validating_storage_test.cc
+++ b/cpp/test/validating_storage_test.cc
@@ -32,6 +32,7 @@
 
 namespace {
 
+using i18n::addressinput::BuildCallback;
 using i18n::addressinput::FakeStorage;
 using i18n::addressinput::scoped_ptr;
 using i18n::addressinput::Storage;
@@ -52,20 +53,17 @@ class ValidatingStorageTest : public testing::Test  {
         storage_(wrapped_storage_),
         success_(false),
         key_(),
-        data_() {}
+        data_(),
+        data_ready_(BuildCallback(this, &ValidatingStorageTest::OnDataReady)) {}
 
   virtual ~ValidatingStorageTest() {}
 
-  ValidatingStorage::Callback* BuildCallback() {
-    return i18n::addressinput::BuildCallback(
-        this, &ValidatingStorageTest::OnDataReady);
-  }
-
   Storage* const wrapped_storage_;  // Owned by |storage_|.
   ValidatingStorage storage_;
   bool success_;
   std::string key_;
   std::string data_;
+  const scoped_ptr<const ValidatingStorage::Callback> data_ready_;
 
  private:
   void OnDataReady(bool success, const std::string& key, std::string* data) {
@@ -83,9 +81,7 @@ class ValidatingStorageTest : public testing::Test  {
 
 TEST_F(ValidatingStorageTest, GoodData) {
   storage_.Put(kKey, new std::string(kValidatedData));
-
-  const scoped_ptr<const ValidatingStorage::Callback> callback(BuildCallback());
-  storage_.Get(kKey, *callback);
+  storage_.Get(kKey, *data_ready_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ(kKey, key_);
@@ -94,9 +90,7 @@ TEST_F(ValidatingStorageTest, GoodData) {
 
 TEST_F(ValidatingStorageTest, EmptyData) {
   storage_.Put(kKey, new std::string(kEmptyData));
-
-  const scoped_ptr<const ValidatingStorage::Callback> callback(BuildCallback());
-  storage_.Get(kKey, *callback);
+  storage_.Get(kKey, *data_ready_);
 
   EXPECT_TRUE(success_);
   EXPECT_EQ(kKey, key_);
@@ -104,8 +98,7 @@ TEST_F(ValidatingStorageTest, EmptyData) {
 }
 
 TEST_F(ValidatingStorageTest, MissingKey) {
-  const scoped_ptr<const ValidatingStorage::Callback> callback(BuildCallback());
-  storage_.Get(kKey, *callback);
+  storage_.Get(kKey, *data_ready_);
 
   EXPECT_FALSE(success_);
   EXPECT_EQ(kKey, key_);
@@ -115,9 +108,7 @@ TEST_F(ValidatingStorageTest, MissingKey) {
 TEST_F(ValidatingStorageTest, GarbageData) {
   storage_.Put(kKey, new std::string(kValidatedData));
   wrapped_storage_->Put(kKey, new std::string("garbage"));
-
-  const scoped_ptr<const ValidatingStorage::Callback> callback(BuildCallback());
-  storage_.Get(kKey, *callback);
+  storage_.Get(kKey, *data_ready_);
 
   EXPECT_FALSE(success_);
   EXPECT_EQ(kKey, key_);
@@ -127,9 +118,7 @@ TEST_F(ValidatingStorageTest, GarbageData) {
 TEST_F(ValidatingStorageTest, StaleData) {
   storage_.Put(kKey, new std::string(kValidatedData));
   wrapped_storage_->Put(kKey, new std::string(kStaleWrappedData));
-
-  const scoped_ptr<const ValidatingStorage::Callback> callback(BuildCallback());
-  storage_.Get(kKey, *callback);
+  storage_.Get(kKey, *data_ready_);
 
   EXPECT_FALSE(success_);
   EXPECT_EQ(kKey, key_);