From 64c5a3888f21acf96082e89764e5ee66cc3531b6 Mon Sep 17 00:00:00 2001 From: bauerb Date: Mon, 6 Jul 2015 13:08:54 -0700 Subject: [PATCH] Make JSONParser a pure interface. This allows stubbing it out in tests, and using platform-specific implementations in the future. As a side effect, the fact that the default implementation is refcounted is not exposed to clients anymore. Instead, the (newly added) factory method returns a raw pointer, and the object will delete itself after either of the callbacks has been called. In practice, this should not make a difference, as no client currently requires the SafeJsonParser to live longer than that, and in one case it actually simplifies things for the client by breaking a reference cycle. TBR=benwells@chromium.org,brettw@chromium.org BUG=501333 Review URL: https://codereview.chromium.org/1214903010 Cr-Commit-Position: refs/heads/master@{#337459} --- .../chrome_management_api_delegate.cc | 22 ++-- .../extensions/webstore_data_fetcher.cc | 10 +- .../extensions/webstore_install_helper.cc | 18 +-- .../extensions/webstore_install_helper.h | 2 - .../browser/safe_json_parser_browsertest.cc | 4 +- .../supervised_user_site_list.cc | 33 +----- .../supervised_user_site_list.h | 5 - ...ervised_user_whitelist_service_unittest.cc | 8 +- .../search/common/json_response_fetcher.cc | 12 +- .../chrome_web_resource_service.cc | 6 +- chrome/chrome_tests_unit.gypi | 1 + chrome/test/BUILD.gn | 1 + components/safe_json.gypi | 17 +++ components/safe_json/BUILD.gn | 17 +++ components/safe_json/safe_json_parser.cc | 103 +++--------------- components/safe_json/safe_json_parser.h | 59 ++++------ components/safe_json/safe_json_parser_impl.cc | 97 +++++++++++++++++ components/safe_json/safe_json_parser_impl.h | 64 +++++++++++ components/safe_json/testing_json_parser.cc | 58 ++++++++++ components/safe_json/testing_json_parser.h | 48 ++++++++ 20 files changed, 373 insertions(+), 212 deletions(-) create mode 100644 components/safe_json/safe_json_parser_impl.cc create mode 100644 components/safe_json/safe_json_parser_impl.h create mode 100644 components/safe_json/testing_json_parser.cc create mode 100644 components/safe_json/testing_json_parser.h diff --git a/chrome/browser/extensions/api/management/chrome_management_api_delegate.cc b/chrome/browser/extensions/api/management/chrome_management_api_delegate.cc index 0d06b3bb7b0461..661bf478158998 100644 --- a/chrome/browser/extensions/api/management/chrome_management_api_delegate.cc +++ b/chrome/browser/extensions/api/management/chrome_management_api_delegate.cc @@ -184,18 +184,16 @@ void ChromeManagementAPIDelegate:: GetPermissionWarningsByManifestFunctionDelegate( extensions::ManagementGetPermissionWarningsByManifestFunction* function, const std::string& manifest_str) const { - scoped_refptr parser( - new safe_json::SafeJsonParser( - manifest_str, - base::Bind( - &extensions::ManagementGetPermissionWarningsByManifestFunction:: - OnParseSuccess, - function), - base::Bind( - &extensions::ManagementGetPermissionWarningsByManifestFunction:: - OnParseFailure, - function))); - parser->Start(); + safe_json::SafeJsonParser::Parse( + manifest_str, + base::Bind( + &extensions::ManagementGetPermissionWarningsByManifestFunction:: + OnParseSuccess, + function), + base::Bind( + &extensions::ManagementGetPermissionWarningsByManifestFunction:: + OnParseFailure, + function)); } scoped_ptr diff --git a/chrome/browser/extensions/webstore_data_fetcher.cc b/chrome/browser/extensions/webstore_data_fetcher.cc index c6e5a57074e0d6..1b082b90761a5c 100644 --- a/chrome/browser/extensions/webstore_data_fetcher.cc +++ b/chrome/browser/extensions/webstore_data_fetcher.cc @@ -82,13 +82,11 @@ void WebstoreDataFetcher::OnURLFetchComplete(const net::URLFetcher* source) { std::string webstore_json_data; fetcher->GetResponseAsString(&webstore_json_data); - scoped_refptr parser = - new safe_json::SafeJsonParser( - webstore_json_data, - base::Bind(&WebstoreDataFetcher::OnJsonParseSuccess, AsWeakPtr()), - base::Bind(&WebstoreDataFetcher::OnJsonParseFailure, AsWeakPtr())); // The parser will call us back via one of the callbacks. - parser->Start(); + safe_json::SafeJsonParser::Parse( + webstore_json_data, + base::Bind(&WebstoreDataFetcher::OnJsonParseSuccess, AsWeakPtr()), + base::Bind(&WebstoreDataFetcher::OnJsonParseFailure, AsWeakPtr())); } } // namespace extensions diff --git a/chrome/browser/extensions/webstore_install_helper.cc b/chrome/browser/extensions/webstore_install_helper.cc index 37710ba393c34c..55fefbe48cb882 100644 --- a/chrome/browser/extensions/webstore_install_helper.cc +++ b/chrome/browser/extensions/webstore_install_helper.cc @@ -43,19 +43,9 @@ WebstoreInstallHelper::~WebstoreInstallHelper() {} void WebstoreInstallHelper::Start() { CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - // No existing |json_parser_| to avoid unbalanced AddRef(). - CHECK(!json_parser_.get()); - AddRef(); // Balanced in OnJSONParseSucceeded()/OnJSONParseFailed(). - // Use base::Unretained so that base::Bind won't AddRef() on us. Otherwise, - // we'll have two callbacks holding references to us, only one of which will - // ever be called. - json_parser_ = new safe_json::SafeJsonParser( - manifest_, - base::Bind(&WebstoreInstallHelper::OnJSONParseSucceeded, - base::Unretained(this)), - base::Bind(&WebstoreInstallHelper::OnJSONParseFailed, - base::Unretained(this))); - json_parser_->Start(); + safe_json::SafeJsonParser::Parse( + manifest_, base::Bind(&WebstoreInstallHelper::OnJSONParseSucceeded, this), + base::Bind(&WebstoreInstallHelper::OnJSONParseFailed, this)); if (icon_url_.is_empty()) { icon_decode_complete_ = true; @@ -103,7 +93,6 @@ void WebstoreInstallHelper::OnJSONParseSucceeded( parse_error_ = Delegate::MANIFEST_ERROR; ReportResultsIfComplete(); - Release(); // Balanced in Start(). } void WebstoreInstallHelper::OnJSONParseFailed( @@ -113,7 +102,6 @@ void WebstoreInstallHelper::OnJSONParseFailed( error_ = error_message; parse_error_ = Delegate::MANIFEST_ERROR; ReportResultsIfComplete(); - Release(); // Balanced in Start(). } void WebstoreInstallHelper::ReportResultsIfComplete() { diff --git a/chrome/browser/extensions/webstore_install_helper.h b/chrome/browser/extensions/webstore_install_helper.h index 90c6b65a9d7daa..7b1f62c185d062 100644 --- a/chrome/browser/extensions/webstore_install_helper.h +++ b/chrome/browser/extensions/webstore_install_helper.h @@ -96,8 +96,6 @@ class WebstoreInstallHelper : public base::RefCounted, // The manifest to parse. std::string manifest_; - scoped_refptr json_parser_; - // If |icon_url_| is non-empty, it needs to be fetched and decoded into an // SkBitmap. GURL icon_url_; diff --git a/chrome/browser/safe_json_parser_browsertest.cc b/chrome/browser/safe_json_parser_browsertest.cc index 4472a2499d94a8..ec45551edae584 100644 --- a/chrome/browser/safe_json_parser_browsertest.cc +++ b/chrome/browser/safe_json_parser_browsertest.cc @@ -54,9 +54,7 @@ class SafeJsonParserTest : public InProcessBrowserTest { error_callback = base::Bind(&SafeJsonParserTest::ExpectError, base::Unretained(this), error); } - scoped_refptr parser = - new SafeJsonParser(json, success_callback, error_callback); - parser->Start(); + SafeJsonParser::Parse(json, success_callback, error_callback); message_loop_runner_->Run(); message_loop_runner_ = nullptr; diff --git a/chrome/browser/supervised_user/supervised_user_site_list.cc b/chrome/browser/supervised_user/supervised_user_site_list.cc index 9c4d547be73142..9b78e64252714a 100644 --- a/chrome/browser/supervised_user/supervised_user_site_list.cc +++ b/chrome/browser/supervised_user/supervised_user_site_list.cc @@ -25,8 +25,6 @@ const char kWhitelistKey[] = "whitelist"; namespace { -bool g_load_in_process = false; - std::string ReadFileOnBlockingThread(const base::FilePath& path) { SCOPED_UMA_HISTOGRAM_TIMER("ManagedUsers.Whitelist.ReadDuration"); std::string contents; @@ -114,11 +112,6 @@ void SupervisedUserSiteList::Load(const base::FilePath& path, base::Bind(&SupervisedUserSiteList::ParseJson, path, callback)); } -// static -void SupervisedUserSiteList::SetLoadInProcessForTesting(bool in_process) { - g_load_in_process = in_process; -} - SupervisedUserSiteList::SupervisedUserSiteList(const base::ListValue& sites) { for (const base::Value* site : sites) { const base::DictionaryValue* entry = nullptr; @@ -142,26 +135,12 @@ void SupervisedUserSiteList::ParseJson( const base::FilePath& path, const SupervisedUserSiteList::LoadedCallback& callback, const std::string& json) { - if (g_load_in_process) { - JSONFileValueDeserializer deserializer(path); - std::string error; - scoped_ptr value(deserializer.Deserialize(nullptr, &error)); - if (!value) { - HandleError(path, error); - return; - } - - OnJsonParseSucceeded(path, base::TimeTicks(), callback, value.Pass()); - return; - } - - // TODO(bauerb): Use batch mode to load multiple whitelists? - scoped_refptr parser( - new safe_json::SafeJsonParser( - json, base::Bind(&SupervisedUserSiteList::OnJsonParseSucceeded, path, - base::TimeTicks::Now(), callback), - base::Bind(&HandleError, path))); - parser->Start(); + // TODO(bauerb): Use JSONSanitizer to sanitize whitelists on installation + // instead of using the expensive SafeJsonParser on every load. + safe_json::SafeJsonParser::Parse( + json, base::Bind(&SupervisedUserSiteList::OnJsonParseSucceeded, path, + base::TimeTicks::Now(), callback), + base::Bind(&HandleError, path)); } // static diff --git a/chrome/browser/supervised_user/supervised_user_site_list.h b/chrome/browser/supervised_user/supervised_user_site_list.h index 61625e769fcbd6..22794f2b4af7bf 100644 --- a/chrome/browser/supervised_user/supervised_user_site_list.h +++ b/chrome/browser/supervised_user/supervised_user_site_list.h @@ -60,11 +60,6 @@ class SupervisedUserSiteList // the newly created object. static void Load(const base::FilePath& file, const LoadedCallback& callback); - // Sets whether the site list should be loaded in-process or out-of-process. - // In-process loading should only be used in tests (to avoid having to set up - // child process handling). - static void SetLoadInProcessForTesting(bool in_process); - // Returns a list of all sites in this site list. const std::vector& sites() const { return sites_; } diff --git a/chrome/browser/supervised_user/supervised_user_whitelist_service_unittest.cc b/chrome/browser/supervised_user/supervised_user_whitelist_service_unittest.cc index a269d8d162a2d9..5a7ef974a65880 100644 --- a/chrome/browser/supervised_user/supervised_user_whitelist_service_unittest.cc +++ b/chrome/browser/supervised_user/supervised_user_whitelist_service_unittest.cc @@ -21,6 +21,7 @@ #include "chrome/common/chrome_paths.h" #include "chrome/common/pref_names.h" #include "chrome/test/base/testing_profile.h" +#include "components/safe_json/testing_json_parser.h" #include "content/public/test/test_browser_thread_bundle.h" #include "sync/api/sync_change.h" #include "sync/api/sync_error_factory.h" @@ -90,11 +91,6 @@ class SupervisedUserWhitelistServiceTest : public testing::Test { service_->AddSiteListsChangedCallback( base::Bind(&SupervisedUserWhitelistServiceTest::OnSiteListsChanged, base::Unretained(this))); - SupervisedUserSiteList::SetLoadInProcessForTesting(true); - } - - ~SupervisedUserWhitelistServiceTest() override { - SupervisedUserSiteList::SetLoadInProcessForTesting(false); } protected: @@ -156,6 +152,8 @@ class SupervisedUserWhitelistServiceTest : public testing::Test { content::TestBrowserThreadBundle thread_bundle_; TestingProfile profile_; + safe_json::TestingJsonParser::ScopedFactoryOverride factory_override_; + scoped_ptr installer_; scoped_ptr service_; diff --git a/chrome/browser/ui/app_list/search/common/json_response_fetcher.cc b/chrome/browser/ui/app_list/search/common/json_response_fetcher.cc index f175e44b019815..5eb72b96ef8fb5 100644 --- a/chrome/browser/ui/app_list/search/common/json_response_fetcher.cc +++ b/chrome/browser/ui/app_list/search/common/json_response_fetcher.cc @@ -72,14 +72,12 @@ void JSONResponseFetcher::OnURLFetchComplete( std::string json_data; fetcher->GetResponseAsString(&json_data); - scoped_refptr parser = - new safe_json::SafeJsonParser( - json_data, base::Bind(&JSONResponseFetcher::OnJsonParseSuccess, - weak_factory_.GetWeakPtr()), - base::Bind(&JSONResponseFetcher::OnJsonParseError, - weak_factory_.GetWeakPtr())); // The parser will call us back via one of the callbacks. - parser->Start(); + safe_json::SafeJsonParser::Parse( + json_data, base::Bind(&JSONResponseFetcher::OnJsonParseSuccess, + weak_factory_.GetWeakPtr()), + base::Bind(&JSONResponseFetcher::OnJsonParseError, + weak_factory_.GetWeakPtr())); } } // namespace app_list diff --git a/chrome/browser/web_resource/chrome_web_resource_service.cc b/chrome/browser/web_resource/chrome_web_resource_service.cc index 6e1e35ea1ca00e..47a1335316ebe8 100644 --- a/chrome/browser/web_resource/chrome_web_resource_service.cc +++ b/chrome/browser/web_resource/chrome_web_resource_service.cc @@ -38,9 +38,5 @@ void ChromeWebResourceService::ParseJSON( const std::string& data, const SuccessCallback& success_callback, const ErrorCallback& error_callback) { - // SafeJsonParser releases itself on completion. - scoped_refptr json_parser( - new safe_json::SafeJsonParser(data, success_callback, - error_callback)); - json_parser->Start(); + safe_json::SafeJsonParser::Parse(data, success_callback, error_callback); } diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 750fb257fef627..d638fbeb835323 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -2221,6 +2221,7 @@ '../components/components.gyp:autofill_content_test_support', '../components/components.gyp:component_metrics_proto', '../components/components.gyp:data_reduction_proxy_test_support', + '../components/components.gyp:safe_json_test_support', '../components/components.gyp:webdata_services_test_support', '../components/components_strings.gyp:components_strings', '../content/app/resources/content_resources.gyp:content_resources', diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 2638a7d970d137..1d250bb8fe520e 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn @@ -1274,6 +1274,7 @@ if (!is_android) { "//components/autofill/content/renderer:test_support", "//components/metrics/proto", "//components/data_reduction_proxy/core/browser:test_support", + "//components/safe_json:test_support", "//components/webdata_services:test_support", "//components/strings", "//device/bluetooth:mocks", diff --git a/components/safe_json.gypi b/components/safe_json.gypi index d3c55126079b02..b7ab6084ec9779 100644 --- a/components/safe_json.gypi +++ b/components/safe_json.gypi @@ -25,6 +25,23 @@ 'sources': [ 'safe_json/safe_json_parser.cc', 'safe_json/safe_json_parser.h', + 'safe_json/safe_json_parser_impl.cc', + 'safe_json/safe_json_parser_impl.h', + ], + }, + { + 'target_name': 'safe_json_test_support', + 'type': 'static_library', + 'dependencies': [ + '../base/base.gyp:base', + ':safe_json', + ], + 'include_dirs': [ + '..', + ], + 'sources': [ + 'safe_json/testing_json_parser.cc', + 'safe_json/testing_json_parser.h', ], }, { diff --git a/components/safe_json/BUILD.gn b/components/safe_json/BUILD.gn index 9238befcb33341..09ce9ad26985f3 100644 --- a/components/safe_json/BUILD.gn +++ b/components/safe_json/BUILD.gn @@ -7,6 +7,8 @@ static_library("safe_json") { sources = [ "safe_json_parser.cc", "safe_json_parser.h", + "safe_json_parser_impl.cc", + "safe_json_parser_impl.h", ] defines = [ "SAFE_JSON_IMPLEMENTATION" ] @@ -20,6 +22,21 @@ static_library("safe_json") { ] } +# GYP version: components/safe_json.gypi:safe_json_test_support +source_set("test_support") { + testonly = true + sources = [ + "testing_json_parser.cc", + "testing_json_parser.h", + ] + + deps = [ + ":safe_json", + "//base", + ] +} + +# GYP version: components/safe_json.gypi:safe_json_parser_message_filter static_library("safe_json_parser_message_filter") { sources = [ "safe_json_parser_message_filter.cc", diff --git a/components/safe_json/safe_json_parser.cc b/components/safe_json/safe_json_parser.cc index af30a19f2d7be7..8193d0358e42fe 100644 --- a/components/safe_json/safe_json_parser.cc +++ b/components/safe_json/safe_json_parser.cc @@ -4,101 +4,30 @@ #include "components/safe_json/safe_json_parser.h" -#include - -#include "base/single_thread_task_runner.h" -#include "base/strings/utf_string_conversions.h" -#include "base/thread_task_runner_handle.h" -#include "base/tuple.h" -#include "base/values.h" -#include "components/safe_json/safe_json_parser_messages.h" -#include "content/public/browser/browser_thread.h" -#include "content/public/browser/utility_process_host.h" -#include "grit/components_strings.h" -#include "ipc/ipc_message_macros.h" -#include "ui/base/l10n/l10n_util.h" - -using content::BrowserThread; -using content::UtilityProcessHost; +#include "components/safe_json/safe_json_parser_impl.h" namespace safe_json { -SafeJsonParser::SafeJsonParser(const std::string& unsafe_json, - const SuccessCallback& success_callback, - const ErrorCallback& error_callback) - : unsafe_json_(unsafe_json), - success_callback_(success_callback), - error_callback_(error_callback) { -} - -void SafeJsonParser::Start() { - caller_task_runner_ = base::ThreadTaskRunnerHandle::Get(); - - BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::Bind(&SafeJsonParser::StartWorkOnIOThread, this)); -} - -SafeJsonParser::~SafeJsonParser() { -} - -void SafeJsonParser::StartWorkOnIOThread() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - UtilityProcessHost* host = UtilityProcessHost::Create( - this, base::ThreadTaskRunnerHandle::Get().get()); - host->SetName( - l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_JSON_PARSER_NAME)); - host->Send(new SafeJsonParserMsg_ParseJSON(unsafe_json_)); -} - -void SafeJsonParser::OnJSONParseSucceeded(const base::ListValue& wrapper) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - const base::Value* value = NULL; - CHECK(wrapper.Get(0, &value)); - - parsed_json_.reset(value->DeepCopy()); - ReportResults(); -} - -void SafeJsonParser::OnJSONParseFailed(const std::string& error_message) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - error_ = error_message; - ReportResults(); -} +namespace { -void SafeJsonParser::ReportResults() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); +SafeJsonParser::Factory g_factory = nullptr; - caller_task_runner_->PostTask( - FROM_HERE, - base::Bind(&SafeJsonParser::ReportResultsOnOriginThread, this)); -} +} // namespace -void SafeJsonParser::ReportResultsOnOriginThread() { - // Current callers of this class are known to either come from - // the UI or IO threads. This DCHECK can be removed/further enhanced - // in case this class is useable outside of these contexts. - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) || - BrowserThread::CurrentlyOn(BrowserThread::IO)); - if (error_.empty() && parsed_json_) { - if (!success_callback_.is_null()) - success_callback_.Run(parsed_json_.Pass()); - } else { - if (!error_callback_.is_null()) - error_callback_.Run(error_); - } +// static +void SafeJsonParser::SetFactoryForTesting(Factory factory) { + g_factory = factory; } -bool SafeJsonParser::OnMessageReceived(const IPC::Message& message) { - bool handled = true; - IPC_BEGIN_MESSAGE_MAP(SafeJsonParser, message) - IPC_MESSAGE_HANDLER(SafeJsonParserHostMsg_ParseJSON_Succeeded, - OnJSONParseSucceeded) - IPC_MESSAGE_HANDLER(SafeJsonParserHostMsg_ParseJSON_Failed, - OnJSONParseFailed) - IPC_MESSAGE_UNHANDLED(handled = false) - IPC_END_MESSAGE_MAP() - return handled; +// static +void SafeJsonParser::Parse(const std::string& unsafe_json, + const SuccessCallback& success_callback, + const ErrorCallback& error_callback) { + SafeJsonParser* parser = + g_factory ? g_factory(unsafe_json, success_callback, error_callback) + : new SafeJsonParserImpl(unsafe_json, success_callback, + error_callback); + parser->Start(); } } // namespace safe_json diff --git a/components/safe_json/safe_json_parser.h b/components/safe_json/safe_json_parser.h index 8fb703b3e9a619..704f0680cb39b2 100644 --- a/components/safe_json/safe_json_parser.h +++ b/components/safe_json/safe_json_parser.h @@ -10,57 +10,40 @@ #include "base/basictypes.h" #include "base/callback.h" #include "base/memory/scoped_ptr.h" -#include "content/public/browser/utility_process_host_client.h" namespace base { -class ListValue; -class SingleThreadTaskRunner; class Value; } -namespace IPC { -class Message; -} - namespace safe_json { -// SafeJsonParser parses a given JSON safely via a utility process. The object -// is ref-counted and kept alive after Start() is called until one of the two -// callbacks is called. -class SafeJsonParser : public content::UtilityProcessHostClient { +// SafeJsonParser parses a given JSON safely via a platform-dependent mechanism +// (like parsing it in a utility process or in a memory-safe environment). +// Internally, an instance of this class is created when Parse() is called and +// is kept alive until one of the two callbacks is called, after which it +// deletes itself. +class SafeJsonParser { public: - typedef base::Callback)> SuccessCallback; - typedef base::Callback ErrorCallback; - - SafeJsonParser(const std::string& unsafe_json, - const SuccessCallback& success_callback, - const ErrorCallback& error_callback); + using SuccessCallback = base::Callback)>; + using ErrorCallback = base::Callback; - void Start(); - - private: - ~SafeJsonParser() override; + using Factory = SafeJsonParser* (*)(const std::string& unsafe_json, + const SuccessCallback& success_callback, + const ErrorCallback& error_callback); - void StartWorkOnIOThread(); + // Starts parsing the passed in |unsafe_json| and calls either + // |success_callback| or |error_callback| when finished. + static void Parse(const std::string& unsafe_json, + const SuccessCallback& success_callback, + const ErrorCallback& error_callback); - void OnJSONParseSucceeded(const base::ListValue& wrapper); - void OnJSONParseFailed(const std::string& error_message); + static void SetFactoryForTesting(Factory factory); - void ReportResults(); - void ReportResultsOnOriginThread(); + protected: + virtual ~SafeJsonParser() {} - // Implementing pieces of the UtilityProcessHostClient interface. - bool OnMessageReceived(const IPC::Message& message) override; - - const std::string unsafe_json_; - SuccessCallback success_callback_; - ErrorCallback error_callback_; - scoped_refptr caller_task_runner_; - - scoped_ptr parsed_json_; - std::string error_; - - DISALLOW_COPY_AND_ASSIGN(SafeJsonParser); + private: + virtual void Start() = 0; }; } // namespace safe_json diff --git a/components/safe_json/safe_json_parser_impl.cc b/components/safe_json/safe_json_parser_impl.cc new file mode 100644 index 00000000000000..0b4823e43b883d --- /dev/null +++ b/components/safe_json/safe_json_parser_impl.cc @@ -0,0 +1,97 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/safe_json/safe_json_parser_impl.h" + +#include "base/sequenced_task_runner.h" +#include "base/strings/utf_string_conversions.h" +#include "base/thread_task_runner_handle.h" +#include "base/tuple.h" +#include "base/values.h" +#include "components/safe_json/safe_json_parser_messages.h" +#include "content/public/browser/browser_thread.h" +#include "content/public/browser/utility_process_host.h" +#include "grit/components_strings.h" +#include "ipc/ipc_message_macros.h" +#include "ui/base/l10n/l10n_util.h" + +using content::BrowserThread; +using content::UtilityProcessHost; + +namespace safe_json { + +SafeJsonParserImpl::SafeJsonParserImpl(const std::string& unsafe_json, + const SuccessCallback& success_callback, + const ErrorCallback& error_callback) + : unsafe_json_(unsafe_json), + success_callback_(success_callback), + error_callback_(error_callback) {} + +SafeJsonParserImpl::~SafeJsonParserImpl() { +} + +void SafeJsonParserImpl::StartWorkOnIOThread() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + UtilityProcessHost* host = UtilityProcessHost::Create( + this, base::ThreadTaskRunnerHandle::Get().get()); + host->SetName( + l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_JSON_PARSER_NAME)); + host->Send(new SafeJsonParserMsg_ParseJSON(unsafe_json_)); +} + +void SafeJsonParserImpl::OnJSONParseSucceeded(const base::ListValue& wrapper) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + const base::Value* value = NULL; + CHECK(wrapper.Get(0, &value)); + + parsed_json_.reset(value->DeepCopy()); + ReportResults(); +} + +void SafeJsonParserImpl::OnJSONParseFailed(const std::string& error_message) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + error_ = error_message; + ReportResults(); +} + +void SafeJsonParserImpl::ReportResults() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + + caller_task_runner_->PostTask( + FROM_HERE, + base::Bind(&SafeJsonParserImpl::ReportResultsOnOriginThread, this)); +} + +void SafeJsonParserImpl::ReportResultsOnOriginThread() { + DCHECK(caller_task_runner_->RunsTasksOnCurrentThread()); + if (error_.empty() && parsed_json_) { + if (!success_callback_.is_null()) + success_callback_.Run(parsed_json_.Pass()); + } else { + if (!error_callback_.is_null()) + error_callback_.Run(error_); + } +} + +bool SafeJsonParserImpl::OnMessageReceived(const IPC::Message& message) { + bool handled = true; + IPC_BEGIN_MESSAGE_MAP(SafeJsonParserImpl, message) + IPC_MESSAGE_HANDLER(SafeJsonParserHostMsg_ParseJSON_Succeeded, + OnJSONParseSucceeded) + IPC_MESSAGE_HANDLER(SafeJsonParserHostMsg_ParseJSON_Failed, + OnJSONParseFailed) + IPC_MESSAGE_UNHANDLED(handled = false) + IPC_END_MESSAGE_MAP() + return handled; +} + +void SafeJsonParserImpl::Start() { + caller_task_runner_ = base::ThreadTaskRunnerHandle::Get(); + + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&SafeJsonParserImpl::StartWorkOnIOThread, this)); +} + +} // namespace safe_json diff --git a/components/safe_json/safe_json_parser_impl.h b/components/safe_json/safe_json_parser_impl.h new file mode 100644 index 00000000000000..277ee08f95674c --- /dev/null +++ b/components/safe_json/safe_json_parser_impl.h @@ -0,0 +1,64 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMPONENTS_SAFE_JSON_SAFE_JSON_PARSER_IMPL_H_ +#define COMPONENTS_SAFE_JSON_SAFE_JSON_PARSER_IMPL_H_ + +#include + +#include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" +#include "components/safe_json/safe_json_parser.h" +#include "content/public/browser/utility_process_host_client.h" + +namespace base { +class ListValue; +class SequencedTaskRunner; +class Value; +} + +namespace IPC { +class Message; +} + +namespace safe_json { + +class SafeJsonParserImpl : public content::UtilityProcessHostClient, + public SafeJsonParser { + public: + SafeJsonParserImpl(const std::string& unsafe_json, + const SuccessCallback& success_callback, + const ErrorCallback& error_callback); + + private: + ~SafeJsonParserImpl() override; + + void StartWorkOnIOThread(); + + void OnJSONParseSucceeded(const base::ListValue& wrapper); + void OnJSONParseFailed(const std::string& error_message); + + void ReportResults(); + void ReportResultsOnOriginThread(); + + // Implementing pieces of the UtilityProcessHostClient interface. + bool OnMessageReceived(const IPC::Message& message) override; + + // SafeJsonParser implementation. + void Start() override; + + const std::string unsafe_json_; + SuccessCallback success_callback_; + ErrorCallback error_callback_; + scoped_refptr caller_task_runner_; + + scoped_ptr parsed_json_; + std::string error_; + + DISALLOW_COPY_AND_ASSIGN(SafeJsonParserImpl); +}; + +} // namespace safe_json + +#endif // COMPONENTS_SAFE_JSON_SAFE_JSON_PARSER_IMPL_H_ diff --git a/components/safe_json/testing_json_parser.cc b/components/safe_json/testing_json_parser.cc new file mode 100644 index 00000000000000..ea7e9c5469b45c --- /dev/null +++ b/components/safe_json/testing_json_parser.cc @@ -0,0 +1,58 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/safe_json/testing_json_parser.h" + +#include "base/bind.h" +#include "base/bind_helpers.h" +#include "base/json/json_reader.h" +#include "base/location.h" +#include "base/memory/scoped_ptr.h" +#include "base/message_loop/message_loop.h" +#include "base/values.h" + +namespace safe_json { + +namespace { + +SafeJsonParser* CreateTestingJsonParser( + const std::string& unsafe_json, + const SafeJsonParser::SuccessCallback& success_callback, + const SafeJsonParser::ErrorCallback& error_callback) { + return new TestingJsonParser(unsafe_json, success_callback, error_callback); +} + +} // namespace + +TestingJsonParser::ScopedFactoryOverride::ScopedFactoryOverride() { + SafeJsonParser::SetFactoryForTesting(&CreateTestingJsonParser); +} + +TestingJsonParser::ScopedFactoryOverride::~ScopedFactoryOverride() { + SafeJsonParser::SetFactoryForTesting(nullptr); +} + +TestingJsonParser::TestingJsonParser(const std::string& unsafe_json, + const SuccessCallback& success_callback, + const ErrorCallback& error_callback) + : unsafe_json_(unsafe_json), + success_callback_(success_callback), + error_callback_(error_callback) {} + +TestingJsonParser::~TestingJsonParser() {} + +void TestingJsonParser::Start() { + int error_code; + std::string error; + scoped_ptr value = base::JSONReader::ReadAndReturnError( + unsafe_json_, base::JSON_PARSE_RFC, &error_code, &error); + + // Run the callback asynchronously. + base::MessageLoop::current()->PostTask( + FROM_HERE, value ? base::Bind(success_callback_, base::Passed(&value)) + : base::Bind(error_callback_, error)); + base::MessageLoop::current()->DeleteSoon(FROM_HERE, this); +} + +} // namespace diff --git a/components/safe_json/testing_json_parser.h b/components/safe_json/testing_json_parser.h new file mode 100644 index 00000000000000..53bdb1d0ffe155 --- /dev/null +++ b/components/safe_json/testing_json_parser.h @@ -0,0 +1,48 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMPONENTS_SAFE_JSON_TESTING_JSON_PARSER_H_ +#define COMPONENTS_SAFE_JSON_TESTING_JSON_PARSER_H_ + +#include "components/safe_json/safe_json_parser.h" + +namespace safe_json { + +// An implementation of SafeJsonParser that parses JSON in process. This can be +// used in unit tests to avoid having to set up the multiprocess infrastructure +// necessary for the out-of-process SafeJsonParser. +class TestingJsonParser : public SafeJsonParser { + public: + // A helper class that will temporarily override the SafeJsonParser factory to + // create instances of this class. + class ScopedFactoryOverride { + public: + ScopedFactoryOverride(); + ~ScopedFactoryOverride(); + + private: + DISALLOW_COPY_AND_ASSIGN(ScopedFactoryOverride); + }; + + // If instantiating this class manually, it must be allocated with |new| (i.e. + // not on the stack). It will delete itself after calling one of the + // callbacks. + TestingJsonParser(const std::string& unsafe_json, + const SuccessCallback& success_callback, + const ErrorCallback& error_callback); + ~TestingJsonParser() override; + + private: + void Start() override; + + const std::string unsafe_json_; + SuccessCallback success_callback_; + ErrorCallback error_callback_; + + DISALLOW_COPY_AND_ASSIGN(TestingJsonParser); +}; + +} // namespace + +#endif // COMPONENTS_SAFE_JSON_TESTING_JSON_PARSER_H_