Change the Downloader and Storage APIs to use heap-allocated objects.

For use with multithreaded implementations of Downloader and Storage, it
is desirable to work with heap-allocated objects that can easily be
passed between threads (instead of having to work with thread-local
objects that need to be copied between threads).

R=rouslan@chromium.org

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

12 files changed, 84 insertions, 45 deletions

diff --git a/cpp/test/fake_downloader.cc b/cpp/test/fake_downloader.cc
index cf54cf7..0f3f8f1 100644
--- a/cpp/test/fake_downloader.cc
+++ b/cpp/test/fake_downloader.cc
@@ -158,17 +158,18 @@ void FakeDownloader::Download(const std::string& url,
   std::map<std::string, std::string>::const_iterator data_it =
       GetData().find(url);
   bool success = data_it != GetData().end();
-  std::string data = success ? data_it->second : std::string();
-  if (!success &&
-      (GetLookupKeyUtil().IsValidationDataUrl(url) ||
-       GetAggregateLookupKeyUtil().IsValidationDataUrl(url))) {
+  std::string* data = NULL;
+  if (success) {
+    data = new std::string(data_it->second);
+  } else if (GetLookupKeyUtil().IsValidationDataUrl(url) ||
+             GetAggregateLookupKeyUtil().IsValidationDataUrl(url)) {
     // URLs that start with "https://i18napis.appspot.com/ssl-address/" or
     // "https://i18napis.appspot.com/ssl-aggregate-address/" prefix, but do not
     // have associated data will always return "{}" with status code 200.
     // FakeDownloader imitates this behavior for URLs that start with
     // "test://address/" and "test://aggregate-address/" prefixes.
     success = true;
-    data = "{}";
+    data = new std::string("{}");
   }
   downloaded(success, url, data);
 }
diff --git a/cpp/test/fake_downloader.h b/cpp/test/fake_downloader.h
index 2dde442..111ce7a 100644
--- a/cpp/test/fake_downloader.h
+++ b/cpp/test/fake_downloader.h
@@ -41,8 +41,9 @@ namespace addressinput {
 //     private:
 //      void OnDownloaded(bool success,
 //                        const std::string& url,
-//                        const std::string& data) {
+//                        std::string* data) {
 //        ...
+//        delete data;
 //      }
 //
 //      FakeDownloader downloader_;
diff --git a/cpp/test/fake_downloader_test.cc b/cpp/test/fake_downloader_test.cc
index 477a462..2d88acc 100644
--- a/cpp/test/fake_downloader_test.cc
+++ b/cpp/test/fake_downloader_test.cc
@@ -18,6 +18,7 @@
 #include <libaddressinput/downloader.h>
 #include <libaddressinput/util/scoped_ptr.h>
 
+#include <cstddef>
 #include <string>
 
 #include <gtest/gtest.h>
@@ -48,12 +49,14 @@ class FakeDownloaderTest : public testing::TestWithParam<std::string> {
   std::string data_;
 
  private:
-  void OnDownloaded(bool success,
-                    const std::string& url,
-                    const std::string& data) {
+  void OnDownloaded(bool success, const std::string& url, std::string* data) {
+    ASSERT_FALSE(success && data == NULL);
     success_ = success;
     url_ = url;
-    data_ = data;
+    if (data != NULL) {
+      data_ = *data;
+      delete data;
+    }
   }
 };
 
diff --git a/cpp/test/fake_storage.cc b/cpp/test/fake_storage.cc
index 0fd8f62..7ba96c8 100644
--- a/cpp/test/fake_storage.cc
+++ b/cpp/test/fake_storage.cc
@@ -14,26 +14,40 @@
 
 #include "fake_storage.h"
 
+#include <cassert>
+#include <cstddef>
 #include <map>
 #include <string>
+#include <utility>
 
 namespace i18n {
 namespace addressinput {
 
 FakeStorage::FakeStorage() {}
 
-FakeStorage::~FakeStorage() {}
+FakeStorage::~FakeStorage() {
+  for (std::map<std::string, std::string*>::const_iterator
+       it = data_.begin(); it != data_.end(); ++it) {
+    delete it->second;
+  }
+}
 
-void FakeStorage::Put(const std::string& key, const std::string& data) {
-  data_[key] = data;
+void FakeStorage::Put(const std::string& key, std::string* data) {
+  assert(data != NULL);
+  std::pair<std::map<std::string, std::string*>::iterator, bool> result =
+      data_.insert(std::make_pair(key, data));
+  if (!result.second) {
+    // Replace data in existing entry for this key.
+    delete result.first->second;
+    result.first->second = data;
+  }
 }
 
 void FakeStorage::Get(const std::string& key,
                       const Callback& data_ready) const {
-  std::map<std::string, std::string>::const_iterator data_it = data_.find(key);
+  std::map<std::string, std::string*>::const_iterator data_it = data_.find(key);
   bool success = data_it != data_.end();
-  std::string data = success ? data_it->second : std::string();
-  data_ready(success, key, data);
+  data_ready(success, key, success ? new std::string(*data_it->second) : NULL);
 }
 
 }  // namespace addressinput
diff --git a/cpp/test/fake_storage.h b/cpp/test/fake_storage.h
index e14c443..5e07cb0 100644
--- a/cpp/test/fake_storage.h
+++ b/cpp/test/fake_storage.h
@@ -45,8 +45,9 @@ namespace addressinput {
 //     private:
 //      void OnDataReady(bool success,
 //                       const std::string& key,
-//                       const std::string& data) {
+//                       std::string* data) {
 //        ...
+//        delete data;
 //      }
 //
 //      FakeStorage storage_;
@@ -60,11 +61,11 @@ class FakeStorage : public Storage {
   virtual ~FakeStorage();
 
   // Storage implementation.
-  virtual void Put(const std::string& key, const std::string& data);
+  virtual void Put(const std::string& key, std::string* data);
   virtual void Get(const std::string& key, const Callback& data_ready) const;
 
  private:
-  std::map<std::string, std::string> data_;
+  std::map<std::string, std::string*> data_;
 };
 
 }  // namespace addressinput
diff --git a/cpp/test/fake_storage_test.cc b/cpp/test/fake_storage_test.cc
index 516d342..49663ca 100644
--- a/cpp/test/fake_storage_test.cc
+++ b/cpp/test/fake_storage_test.cc
@@ -18,6 +18,7 @@
 #include <libaddressinput/storage.h>
 #include <libaddressinput/util/scoped_ptr.h>
 
+#include <cstddef>
 #include <string>
 
 #include <gtest/gtest.h>
@@ -47,10 +48,14 @@ class FakeStorageTest : public testing::Test {
  private:
   void OnDataReady(bool success,
                    const std::string& key,
-                   const std::string& data) {
+                   std::string* data) {
+    ASSERT_FALSE(success && data == NULL);
     success_ = success;
     key_ = key;
-    data_ = data;
+    if (data != NULL) {
+      data_ = *data;
+      delete data;
+    }
   }
 };
 
@@ -64,7 +69,7 @@ TEST_F(FakeStorageTest, GetWithoutPutReturnsEmptyData) {
 }
 
 TEST_F(FakeStorageTest, GetReturnsWhatWasPut) {
-  storage_.Put("key", "value");
+  storage_.Put("key", new std::string("value"));
 
   scoped_ptr<Storage::Callback> callback(BuildCallback());
   storage_.Get("key", *callback);
@@ -75,8 +80,8 @@ TEST_F(FakeStorageTest, GetReturnsWhatWasPut) {
 }
 
 TEST_F(FakeStorageTest, SecondPutOverwritesData) {
-  storage_.Put("key", "bad-value");
-  storage_.Put("key", "good-value");
+  storage_.Put("key", new std::string("bad-value"));
+  storage_.Put("key", new std::string("good-value"));
 
   scoped_ptr<Storage::Callback> callback(BuildCallback());
   storage_.Get("key", *callback);
diff --git a/cpp/test/mock_downloader.cc b/cpp/test/mock_downloader.cc
index 3158b3e..71256aa 100644
--- a/cpp/test/mock_downloader.cc
+++ b/cpp/test/mock_downloader.cc
@@ -15,6 +15,7 @@
 #include "mock_downloader.h"
 
 #include <cassert>
+#include <cstddef>
 #include <map>
 #include <string>
 
@@ -41,7 +42,7 @@ void MockDownloader::Download(const std::string& url,
   std::string key(url, kMockDataUrlLength);
   std::map<std::string, std::string>::const_iterator it = data_.find(key);
   bool success = it != data_.end();
-  downloaded(success, url, success ? it->second : std::string());
+  downloaded(success, url, success ? new std::string(it->second) : NULL);
 }
 
 }  // namespace addressinput
diff --git a/cpp/test/mock_downloader.h b/cpp/test/mock_downloader.h
index 128942e..b423660 100644
--- a/cpp/test/mock_downloader.h
+++ b/cpp/test/mock_downloader.h
@@ -46,8 +46,9 @@ namespace addressinput {
 //     private:
 //      void OnDownloaded(bool success,
 //                        const std::string& url,
-//                        const std::string& data) {
+//                        std::string* data) {
 //        ...
+//        delete data;
 //      }
 //
 //      MockDownloader downloader_;
diff --git a/cpp/test/null_storage_test.cc b/cpp/test/null_storage_test.cc
index c55571d..dca0803 100644
--- a/cpp/test/null_storage_test.cc
+++ b/cpp/test/null_storage_test.cc
@@ -18,6 +18,7 @@
 #include <libaddressinput/util/basictypes.h>
 #include <libaddressinput/util/scoped_ptr.h>
 
+#include <cstddef>
 #include <string>
 
 #include <gtest/gtest.h>
@@ -45,12 +46,14 @@ class NullStorageTest : public testing::Test {
   static const char kKey[];
 
  private:
-  void OnDataReady(bool success,
-                   const std::string& key,
-                   const std::string& data) {
+  void OnDataReady(bool success, const std::string& key, std::string* data) {
+    ASSERT_FALSE(success && data == NULL);
     success_ = success;
     key_ = key;
-    data_ = data;
+    if (data != NULL) {
+      data_ = *data;
+      delete data;
+    }
   }
 
   DISALLOW_COPY_AND_ASSIGN(NullStorageTest);
@@ -61,7 +64,7 @@ const char NullStorageTest::kKey[] = "foo";
 TEST_F(NullStorageTest, Put) {
   // The Put() method should not do anything, so this test only tests that the
   // code compiles and that the call doesn't crash.
-  storage_.Put(kKey, "bar");
+  storage_.Put(kKey, new std::string("bar"));
 }
 
 TEST_F(NullStorageTest, Get) {
diff --git a/cpp/test/retriever_test.cc b/cpp/test/retriever_test.cc
index e968882..aff418c 100644
--- a/cpp/test/retriever_test.cc
+++ b/cpp/test/retriever_test.cc
@@ -19,6 +19,7 @@
 #include <libaddressinput/storage.h>
 #include <libaddressinput/util/scoped_ptr.h>
 
+#include <cstddef>
 #include <string>
 
 #include <gtest/gtest.h>
@@ -141,11 +142,13 @@ class StaleStorage : public Storage {
 
   // Storage implementation.
   virtual void Get(const std::string& key, const Callback& data_ready) const {
-    data_ready(true, key, kStaleWrappedData);
+    data_ready(true, key, new std::string(kStaleWrappedData));
   }
 
-  virtual void Put(const std::string& key, const std::string& value) {
+  virtual void Put(const std::string& key, std::string* value) {
+    ASSERT_TRUE(value != NULL);
     data_updated_ = true;
+    delete value;
   }
 
   bool data_updated_;
diff --git a/cpp/test/validating_storage_test.cc b/cpp/test/validating_storage_test.cc
index 228475c..ced22dc 100644
--- a/cpp/test/validating_storage_test.cc
+++ b/cpp/test/validating_storage_test.cc
@@ -18,6 +18,7 @@
 #include <libaddressinput/storage.h>
 #include <libaddressinput/util/scoped_ptr.h>
 
+#include <cstddef>
 #include <string>
 
 #include <gtest/gtest.h>
@@ -66,17 +67,19 @@ class ValidatingStorageTest : public testing::Test  {
   std::string data_;
 
  private:
-  void OnDataReady(bool success,
-                   const std::string& key,
-                   const std::string& data) {
+  void OnDataReady(bool success, const std::string& key, std::string* data) {
+    ASSERT_FALSE(success && data == NULL);
     success_ = success;
     key_ = key;
-    data_ = data;
+    if (data != NULL) {
+      data_ = *data;
+      delete data;
+    }
   }
 };
 
 TEST_F(ValidatingStorageTest, GoodData) {
-  storage_.Put(kKey, kValidatedData);
+  storage_.Put(kKey, new std::string(kValidatedData));
 
   scoped_ptr<ValidatingStorage::Callback> callback(BuildCallback());
   storage_.Get(kKey, *callback);
@@ -87,7 +90,7 @@ TEST_F(ValidatingStorageTest, GoodData) {
 }
 
 TEST_F(ValidatingStorageTest, EmptyData) {
-  storage_.Put(kKey, kEmptyData);
+  storage_.Put(kKey, new std::string(kEmptyData));
 
   scoped_ptr<ValidatingStorage::Callback> callback(BuildCallback());
   storage_.Get(kKey, *callback);
@@ -107,8 +110,8 @@ TEST_F(ValidatingStorageTest, MissingKey) {
 }
 
 TEST_F(ValidatingStorageTest, GarbageData) {
-  storage_.Put(kKey, kValidatedData);
-  wrapped_storage_->Put(kKey, "garbage");
+  storage_.Put(kKey, new std::string(kValidatedData));
+  wrapped_storage_->Put(kKey, new std::string("garbage"));
 
   scoped_ptr<ValidatingStorage::Callback> callback(BuildCallback());
   storage_.Get(kKey, *callback);
@@ -119,8 +122,8 @@ TEST_F(ValidatingStorageTest, GarbageData) {
 }
 
 TEST_F(ValidatingStorageTest, StaleData) {
-  storage_.Put(kKey, kValidatedData);
-  wrapped_storage_->Put(kKey, kStaleWrappedData);
+  storage_.Put(kKey, new std::string(kValidatedData));
+  wrapped_storage_->Put(kKey, new std::string(kStaleWrappedData));
 
   scoped_ptr<ValidatingStorage::Callback> callback(BuildCallback());
   storage_.Get(kKey, *callback);
diff --git a/cpp/test/validating_util_test.cc b/cpp/test/validating_util_test.cc
index a255741..b7038d5 100644
--- a/cpp/test/validating_util_test.cc
+++ b/cpp/test/validating_util_test.cc
@@ -139,11 +139,14 @@ TEST(ValidatingUtilTest, UnwrapTimestamp) {
 }
 
 TEST(ValidatingUtilTest, Wrap) {
-  EXPECT_EQ(kWrappedData, ValidatingUtil::Wrap(kUnwrappedData, kTimestamp));
+  std::string data = kUnwrappedData;
+  ValidatingUtil::Wrap(kTimestamp, &data);
+  EXPECT_EQ(kWrappedData, data);
 }
 
 TEST(ValidatingUtilTest, WrapUnwrapIt) {
-  std::string data = ValidatingUtil::Wrap(kUnwrappedData, kTimestamp);
+  std::string data = kUnwrappedData;
+  ValidatingUtil::Wrap(kTimestamp, &data);
   EXPECT_TRUE(ValidatingUtil::UnwrapTimestamp(&data, kTimestamp));
   EXPECT_EQ(kChecksummedData, data);
   EXPECT_TRUE(ValidatingUtil::UnwrapChecksum(&data));