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

BUG=337679

Review URL: https://codereview.chromium.org/147843008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@248530 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 70 insertions, 25 deletions

diff --git a/third_party/libaddressinput/chromium/cpp/src/util/json.h b/third_party/libaddressinput/chromium/cpp/src/util/json.h
index 9af7f1c..c728472 100644
--- a/third_party/libaddressinput/chromium/cpp/src/util/json.h
+++ b/third_party/libaddressinput/chromium/cpp/src/util/json.h
@@ -50,7 +50,8 @@ class Json {
   // Sets |value| to the dictionary for |key| if it exists and has a dictionary
   // value. Returns false if the key doesn't exist or doesn't correspond to a
   // dictionary. The JSON object must be parsed successfully in ParseObject()
-  // before invoking this method.
+  // before invoking this method. |value| is only guaranteed to be valid as long
+  // as |this| is valid.
   virtual bool GetJsonValueForKey(const std::string& key,
                                   scoped_ptr<Json>* value) const = 0;
 
diff --git a/third_party/libaddressinput/chromium/json.cc b/third_party/libaddressinput/chromium/json.cc
index 37a1b77..5437e1e 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
+// 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,70 @@ 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 {
+  return GetDict()->GetStringWithoutPathExpansion(key, value);
+}
+
+bool ChromeJson::GetJsonValueForKey(const std::string& key,
+                                    scoped_ptr<Json>* value) const {
+  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() {}