Add error messages to JSONReader and friends. This required a bit of refactoring to do cleanly. This CL doesn't actually use this capability anywhere except for unit tests. I will add that in a future CL.

Review URL: http://codereview.chromium.org/13169

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

14 files changed, 42 insertions, 32 deletions

diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc
index 47ccdd8..0bad51c 100644
--- a/chrome/browser/autocomplete/search_provider.cc
+++ b/chrome/browser/autocomplete/search_provider.cc
@@ -133,7 +133,8 @@ void SearchProvider::OnURLFetchComplete(const URLFetcher* source,
   deserializer.set_allow_trailing_comma(true);
   Value* root_val = NULL;
   have_suggest_results_ = status.is_success() && (response_code == 200) &&
-      deserializer.Deserialize(&root_val) && ParseSuggestResults(root_val);
+      deserializer.Deserialize(&root_val, NULL) &&
+      ParseSuggestResults(root_val);
   delete root_val;
   ConvertResultsToAutocompleteMatches();
   listener_->OnProviderUpdate(!suggest_results_.empty());
diff --git a/chrome/browser/bookmarks/bookmark_storage.cc b/chrome/browser/bookmarks/bookmark_storage.cc
index 591f190..e6e9a50 100644
--- a/chrome/browser/bookmarks/bookmark_storage.cc
+++ b/chrome/browser/bookmarks/bookmark_storage.cc
@@ -167,7 +167,7 @@ void BookmarkStorageBackend::Read(scoped_refptr<BookmarkStorage> service,
   Value* root = NULL;
   if (bookmark_file_exists) {
     JSONFileValueSerializer serializer(path);
-    serializer.Deserialize(&root);
+    serializer.Deserialize(&root, NULL);
   }
 
   // BookmarkStorage takes ownership of root.
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 67d508e..32cc861 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -86,7 +86,7 @@ bool ExtensionsServiceBackend::LoadExtensionsFromDirectory(
 
     JSONFileValueSerializer serializer(manifest_path.ToWStringHack());
     Value* root = NULL;
-    if (!serializer.Deserialize(&root)) {
+    if (!serializer.Deserialize(&root, NULL)) {
       ReportExtensionLoadError(frontend.get(), 
                                Extension::kInvalidManifestError);
       continue;
diff --git a/chrome/browser/page_state.cc b/chrome/browser/page_state.cc
index e0c350f..f47a4cf 100644
--- a/chrome/browser/page_state.cc
+++ b/chrome/browser/page_state.cc
@@ -42,7 +42,7 @@ void PageState::InitWithBytes(const std::string& bytes) {
   JSONStringValueSerializer serializer(bytes);
   Value* root = NULL;
 
-  if (!serializer.Deserialize(&root))
+  if (!serializer.Deserialize(&root, NULL))
     NOTREACHED();
 
   if (root != NULL && root->GetType() == Value::TYPE_DICTIONARY) {
diff --git a/chrome/common/json_value_serializer.cc b/chrome/common/json_value_serializer.cc
index b07fb29..82938ec 100644
--- a/chrome/common/json_value_serializer.cc
+++ b/chrome/common/json_value_serializer.cc
@@ -21,11 +21,14 @@ bool JSONStringValueSerializer::Serialize(const Value& root) {
   return true;
 }
 
-bool JSONStringValueSerializer::Deserialize(Value** root) {
+bool JSONStringValueSerializer::Deserialize(Value** root,
+                                            std::string* error_message) {
   if (!json_string_)
     return false;
 
-  return JSONReader::Read(*json_string_, root, allow_trailing_comma_);
+  return JSONReader::ReadAndReturnError(*json_string_, root,
+                                        allow_trailing_comma_,
+                                        error_message);
 }
 
 /******* File Serializer *******/
@@ -47,12 +50,13 @@ bool JSONFileValueSerializer::Serialize(const Value& root) {
   return true;
 }
 
-bool JSONFileValueSerializer::Deserialize(Value** root) {
+bool JSONFileValueSerializer::Deserialize(Value** root,
+                                          std::string* error_message) {
   std::string json_string;
   if (!file_util::ReadFileToString(json_file_path_, &json_string)) {
     return false;
   }
   JSONStringValueSerializer serializer(json_string);
-  return serializer.Deserialize(root);
+  return serializer.Deserialize(root, error_message);
 }
 
diff --git a/chrome/common/json_value_serializer.h b/chrome/common/json_value_serializer.h
index a86369d..2e581ee 100644
--- a/chrome/common/json_value_serializer.h
+++ b/chrome/common/json_value_serializer.h
@@ -41,8 +41,10 @@ class JSONStringValueSerializer : public ValueSerializer {
   // in to the constructor into a structure of Value objects.  If the return
   // value is true, the |root| parameter will be set to point to a new Value
   // object that corresponds to the values represented in the string.  The
-  // caller takes ownership of the returned Value objects.
-  bool Deserialize(Value** root);
+  // caller takes ownership of the returned Value objects. Otherwise, the root
+  // value will be changed and if |error_message| is non-null, it will contain
+  // a string describing the error.
+  bool Deserialize(Value** root, std::string* error_message);
 
   void set_pretty_print(bool new_value) { pretty_print_ = new_value; }
   bool pretty_print() { return pretty_print_; }
@@ -86,8 +88,10 @@ class JSONFileValueSerializer : public ValueSerializer {
   // in to the constructor into a structure of Value objects.  If the return
   // value is true, the |root| parameter will be set to point to a new Value
   // object that corresponds to the values represented in the file.  The
-  // caller takes ownership of the returned Value objects.
-  bool Deserialize(Value** root);
+  // caller takes ownership of the returned Value objects. Otherwise, the root
+  // value will be changed and if |error_message| is non-null, it will contain
+  // a string describing the error.
+  bool Deserialize(Value** root, std::string* error_message);
 
  private:
   std::wstring json_file_path_;
diff --git a/chrome/common/json_value_serializer_perftest.cc b/chrome/common/json_value_serializer_perftest.cc
index ddc34db..6570d34 100644
--- a/chrome/common/json_value_serializer_perftest.cc
+++ b/chrome/common/json_value_serializer_perftest.cc
@@ -54,7 +54,7 @@ TEST_F(JSONValueSerializerTests, Reading) {
     for (size_t j = 0; j < test_cases_.size(); ++j) {
       Value* root = NULL;
       JSONStringValueSerializer reader(test_cases_[j]);
-      ASSERT_TRUE(reader.Deserialize(&root));
+      ASSERT_TRUE(reader.Deserialize(&root, NULL));
       delete root;
     }
   }
@@ -69,7 +69,7 @@ TEST_F(JSONValueSerializerTests, CompactWriting) {
   for (size_t i = 0; i < test_cases_.size(); ++i) {
     Value* root = NULL;
     JSONStringValueSerializer reader(test_cases_[i]);
-    ASSERT_TRUE(reader.Deserialize(&root));
+    ASSERT_TRUE(reader.Deserialize(&root, NULL));
     test_cases.push_back(root);
   }
 
diff --git a/chrome/common/json_value_serializer_unittest.cc b/chrome/common/json_value_serializer_unittest.cc
index 3997205..3dd175b 100644
--- a/chrome/common/json_value_serializer_unittest.cc
+++ b/chrome/common/json_value_serializer_unittest.cc
@@ -18,7 +18,7 @@ TEST(JSONValueSerializerTest, Roundtrip) {
     "{\"bool\":true,\"int\":42,\"list\":[1,2],\"null\":null,\"real\":3.14}";
   Value* root = NULL;
   JSONStringValueSerializer serializer(original_serialization);
-  ASSERT_TRUE(serializer.Deserialize(&root));
+  ASSERT_TRUE(serializer.Deserialize(&root, NULL));
   ASSERT_TRUE(root);
   ASSERT_TRUE(root->IsType(Value::TYPE_DICTIONARY));
 
@@ -121,7 +121,7 @@ TEST(JSONValueSerializerTest, UnicodeStrings) {
   // escaped ascii text -> json
   Value* deserial_root = NULL;
   JSONStringValueSerializer deserializer(expected);
-  ASSERT_TRUE(deserializer.Deserialize(&deserial_root));
+  ASSERT_TRUE(deserializer.Deserialize(&deserial_root, NULL));
   DictionaryValue* dict_root = static_cast<DictionaryValue*>(deserial_root);
   std::wstring web_value;
   ASSERT_TRUE(dict_root->GetString(L"web", &web_value));
@@ -145,7 +145,7 @@ TEST(JSONValueSerializerTest, HexStrings) {
   // escaped ascii text -> json
   Value* deserial_root = NULL;
   JSONStringValueSerializer deserializer(expected);
-  ASSERT_TRUE(deserializer.Deserialize(&deserial_root));
+  ASSERT_TRUE(deserializer.Deserialize(&deserial_root, NULL));
   DictionaryValue* dict_root = static_cast<DictionaryValue*>(deserial_root);
   std::wstring test_value;
   ASSERT_TRUE(dict_root->GetString(L"test", &test_value));
@@ -156,7 +156,7 @@ TEST(JSONValueSerializerTest, HexStrings) {
   deserial_root = NULL;
   std::string escaped_chars = "{\"test\":\"\\x67\\x6f\"}";
   JSONStringValueSerializer deserializer2(escaped_chars);
-  ASSERT_TRUE(deserializer2.Deserialize(&deserial_root));
+  ASSERT_TRUE(deserializer2.Deserialize(&deserial_root, NULL));
   dict_root = static_cast<DictionaryValue*>(deserial_root);
   ASSERT_TRUE(dict_root->GetString(L"test", &test_value));
   ASSERT_EQ(std::wstring(L"go"), test_value);
@@ -172,8 +172,8 @@ TEST(JSONValueSerializerTest, AllowTrailingComma) {
   JSONStringValueSerializer serializer(test_with_commas);
   serializer.set_allow_trailing_comma(true);
   JSONStringValueSerializer serializer_expected(test_no_commas);
-  ASSERT_TRUE(serializer.Deserialize(&root));
-  ASSERT_TRUE(serializer_expected.Deserialize(&root_expected));
+  ASSERT_TRUE(serializer.Deserialize(&root, NULL));
+  ASSERT_TRUE(serializer_expected.Deserialize(&root_expected, NULL));
   ASSERT_TRUE(root->Equals(root_expected));
 
   delete root;
@@ -262,7 +262,7 @@ TEST_F(JSONFileValueSerializerTest, Roundtrip) {
 
   JSONFileValueSerializer deserializer(original_file_path);
   Value* root;
-  ASSERT_TRUE(deserializer.Deserialize(&root));
+  ASSERT_TRUE(deserializer.Deserialize(&root, NULL));
 
   ASSERT_TRUE(root);
   ASSERT_TRUE(root->IsType(Value::TYPE_DICTIONARY));
@@ -312,7 +312,7 @@ TEST_F(JSONFileValueSerializerTest, RoundtripNested) {
 
   JSONFileValueSerializer deserializer(original_file_path);
   Value* root;
-  ASSERT_TRUE(deserializer.Deserialize(&root));
+  ASSERT_TRUE(deserializer.Deserialize(&root, NULL));
 
   // Now try writing.
   std::wstring written_file_path = test_dir_;
@@ -338,7 +338,7 @@ TEST_F(JSONFileValueSerializerTest, NoWhitespace) {
   ASSERT_TRUE(file_util::PathExists(source_file_path));
   JSONFileValueSerializer serializer(source_file_path);
   Value* root;
-  ASSERT_TRUE(serializer.Deserialize(&root));
+  ASSERT_TRUE(serializer.Deserialize(&root, NULL));
   ASSERT_TRUE(root);
   delete root;
 }
diff --git a/chrome/common/pref_service.cc b/chrome/common/pref_service.cc
index a942e32..8f3322a 100644
--- a/chrome/common/pref_service.cc
+++ b/chrome/common/pref_service.cc
@@ -137,7 +137,7 @@ bool PrefService::LoadPersistentPrefs(const std::wstring& file_path) {
 
   JSONFileValueSerializer serializer(file_path);
   Value* root = NULL;
-  if (serializer.Deserialize(&root)) {
+  if (serializer.Deserialize(&root, NULL)) {
     // Preferences should always have a dictionary root.
     if (!root->IsType(Value::TYPE_DICTIONARY)) {
       delete root;
@@ -156,7 +156,7 @@ void PrefService::ReloadPersistentPrefs() {
 
   JSONFileValueSerializer serializer(pref_filename_);
   Value* root;
-  if (serializer.Deserialize(&root)) {
+  if (serializer.Deserialize(&root, NULL)) {
     // Preferences should always have a dictionary root.
     if (!root->IsType(Value::TYPE_DICTIONARY)) {
       delete root;
diff --git a/chrome/common/pref_service_uitest.cc b/chrome/common/pref_service_uitest.cc
index 9c46d9d..b9f0774 100644
--- a/chrome/common/pref_service_uitest.cc
+++ b/chrome/common/pref_service_uitest.cc
@@ -83,7 +83,7 @@ TEST_F(PreferenceServiceTest, PreservedWindowPlacementIsLoaded) {
 
   JSONFileValueSerializer deserializer(tmp_pref_file_);
   Value* root = NULL;
-  ASSERT_TRUE(deserializer.Deserialize(&root));
+  ASSERT_TRUE(deserializer.Deserialize(&root, NULL));
 
   ASSERT_TRUE(root);
   ASSERT_TRUE(root->IsType(Value::TYPE_DICTIONARY));
diff --git a/chrome/common/pref_service_unittest.cc b/chrome/common/pref_service_unittest.cc
index 37d04de..6c63756 100644
--- a/chrome/common/pref_service_unittest.cc
+++ b/chrome/common/pref_service_unittest.cc
@@ -155,19 +155,20 @@ TEST_F(PrefServiceTest, Overlay) {
   Value* transient_value;
   {
     JSONStringValueSerializer serializer(transient);
-    ASSERT_TRUE(serializer.Deserialize(&transient_value));
+    ASSERT_TRUE(serializer.Deserialize(&transient_value, NULL));
   }
   prefs.transient()->Set(transient_string, transient_value);
 
   Value* both_transient_value;
   {
     JSONStringValueSerializer serializer(transient);
-    ASSERT_TRUE(serializer.Deserialize(&both_transient_value));
+    ASSERT_TRUE(serializer.Deserialize(&both_transient_value, NULL));
   }
   prefs.transient()->Set(L"both", both_transient_value);
 
   // Register test prefs
-  const wchar_t* kTypes[] = { L"neither.", L"persistent.", L"transient.", L"both." };
+  const wchar_t* kTypes[] =
+      { L"neither.", L"persistent.", L"transient.", L"both." };
   for (size_t i = 0; i < arraysize(kTypes); ++i) {
     wchar_t temp[1024];
     wcscpy_s(temp, kTypes[i]);
diff --git a/chrome/installer/util/master_preferences.cc b/chrome/installer/util/master_preferences.cc
index ee09b48..34d10a9 100644
--- a/chrome/installer/util/master_preferences.cc
+++ b/chrome/installer/util/master_preferences.cc
@@ -13,7 +13,7 @@ namespace {
 DictionaryValue* ReadJSONPrefs(const std::string& data) {
   JSONStringValueSerializer json(data);
   Value* root;
-  if (!json.Deserialize(&root))
+  if (!json.Deserialize(&root, NULL))
     return NULL;
   if (!root->IsType(Value::TYPE_DICTIONARY)) {
     delete root;
diff --git a/chrome/test/automation/tab_proxy.cc b/chrome/test/automation/tab_proxy.cc
index 0b87689..91cdd1b 100644
--- a/chrome/test/automation/tab_proxy.cc
+++ b/chrome/test/automation/tab_proxy.cc
@@ -555,7 +555,7 @@ bool TabProxy::ExecuteAndExtractValue(const std::wstring& frame_xpath,
   json.append("]");
 
   JSONStringValueSerializer deserializer(json);
-  succeeded = deserializer.Deserialize(value);
+  succeeded = deserializer.Deserialize(value, NULL);
 
   delete response;
   return succeeded;
diff --git a/chrome/test/ui/ui_test.cc b/chrome/test/ui/ui_test.cc
index 415533e..5b549a0 100644
--- a/chrome/test/ui/ui_test.cc
+++ b/chrome/test/ui/ui_test.cc
@@ -463,7 +463,7 @@ static DictionaryValue* LoadDictionaryValueFromPath(const std::wstring& path) {
 
   JSONFileValueSerializer serializer(path);
   Value* root_value = NULL;
-  if (serializer.Deserialize(&root_value) &&
+  if (serializer.Deserialize(&root_value, NULL) &&
       root_value->GetType() != Value::TYPE_DICTIONARY) {
     delete root_value;
     return NULL;