libaddressinput - Don't make transient copies of entire JSON rule dictionaries

third_party/libaddressinput/chromium/json.cc

Index: third_party/libaddressinput/chromium/json.cc
diff --git a/third_party/libaddressinput/chromium/json.cc b/third_party/libaddressinput/chromium/json.cc
index 37a1b77c64342ae4b5a5899d31c45f960efd07ac..223cfe4dd528b7076f08be4e686b95a20a56b504 100644
--- a/third_party/libaddressinput/chromium/json.cc
+++ b/third_party/libaddressinput/chromium/json.cc
@@ -15,12 +15,29 @@ namespace addressinput {
 
 namespace {
 
+// A base class for Chrome Json objects. Json gets parsed into a

[Dan Beam, 2014/02/01 01:02:36]

nit: s/Json/JSON/g

[Evan Stade, 2014/02/01 01:11:12]

Done.

+// base::DictionaryValue and data is accessed via the Json interface.
 class ChromeJson : public Json {
  public:
+  virtual bool GetStringValueForKey(const std::string& key, std::string* value)
+      const OVERRIDE;
+  virtual bool GetJsonValueForKey(const std::string& key,
+                                  scoped_ptr<Json>* value) const OVERRIDE;
+ protected:
   ChromeJson() {}
-
   virtual ~ChromeJson() {}
 
+  virtual const base::DictionaryValue* GetDict() const = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeJson);
+};
+
+// A Json object that will parse a string and own the parsed data.
+class JsonDataOwner : public ChromeJson {
+ public:
+  JsonDataOwner() {}
+  virtual ~JsonDataOwner() {}
+
   virtual bool ParseObject(const std::string& json) OVERRIDE {
     dict_.reset();
 
@@ -33,43 +50,72 @@ class ChromeJson : public Json {
     return !!dict_;
   }
 
-  virtual bool GetStringValueForKey(const std::string& key, std::string* value)
-      const OVERRIDE {
-    DCHECK(dict_);
-    return dict_->GetStringWithoutPathExpansion(key, value);
+ protected:
+  virtual const base::DictionaryValue* GetDict() const OVERRIDE {
+    return dict_.get();
   }
 
-  virtual bool GetJsonValueForKey(const std::string& key,
-                                  scoped_ptr<Json>* value) const OVERRIDE {
-    DCHECK(dict_);
-    base::DictionaryValue* sub_dict = NULL;  // Owned by |dict_|.
-    if (!dict_->GetDictionaryWithoutPathExpansion(key, &sub_dict) || !sub_dict)
-      return false;
-
-    if (value) {
-      value->reset(new ChromeJson(
-          scoped_ptr<base::DictionaryValue>(sub_dict->DeepCopy())));
-    }
-
-    return true;
+ private:
+  scoped_ptr<base::DictionaryValue> dict_;
+
+  DISALLOW_COPY_AND_ASSIGN(JsonDataOwner);
+};
+
+// A Json object which will point to data that's been parsed by a different
+// ChromeJson. It does not own its data and is only valid as long as its parent
+// ChromeJson is valid.
+class JsonDataCopy : public ChromeJson {
+ public:
+  explicit JsonDataCopy(const base::DictionaryValue* dict) :
+      dict_(dict) {}
+  virtual ~JsonDataCopy() {}
+
+  virtual bool ParseObject(const std::string& json) OVERRIDE {
+    NOTREACHED();
+    return false;
   }
 
- private:
-   explicit ChromeJson(scoped_ptr<base::DictionaryValue> dict)
-       : dict_(dict.Pass()) {}
+ protected:
+  virtual const base::DictionaryValue* GetDict() const OVERRIDE {
+    return dict_;
+  }
 
-  scoped_ptr<base::DictionaryValue> dict_;
+ private:
+  const base::DictionaryValue* dict_;  // weak reference.
 
-  DISALLOW_COPY_AND_ASSIGN(ChromeJson);
+  DISALLOW_COPY_AND_ASSIGN(JsonDataCopy);
 };
 
+// ChromeJson ------------------------------------------------------------------
+
+bool ChromeJson::GetStringValueForKey(const std::string& key,
+                                      std::string* value) const {
+  DCHECK(GetDict());

[Dan Beam, 2014/02/01 01:02:36]

nit: remove if it'll just explode in the next line...

[Evan Stade, 2014/02/01 01:11:12]

Done.

+  return GetDict()->GetStringWithoutPathExpansion(key, value);
+}
+
+bool ChromeJson::GetJsonValueForKey(const std::string& key,
+                                    scoped_ptr<Json>* value) const {
+  DCHECK(GetDict());

[Dan Beam, 2014/02/01 01:02:36]

same

[Evan Stade, 2014/02/01 01:11:12]

Done.

+  const base::DictionaryValue* sub_dict = NULL;
+  if (!GetDict()->GetDictionaryWithoutPathExpansion(key, &sub_dict) ||
+      !sub_dict) {
+    return false;
+  }
+
+  if (value)
+    value->reset(new JsonDataCopy(sub_dict));
+
+  return true;
+}
+
 }  // namespace
 
 Json::~Json() {}
 
 // static
 scoped_ptr<Json> Json::Build() {
-  return scoped_ptr<Json>(new ChromeJson);
+  return scoped_ptr<Json>(new JsonDataOwner);
 }
 
 Json::Json() {}