diff options
21 files changed, 383 insertions, 226 deletions
diff --git a/chrome/browser/extensions/api/developer_private/developer_private_api.cc b/chrome/browser/extensions/api/developer_private/developer_private_api.cc index 2604a1a..52b4ff9 100644 --- a/chrome/browser/extensions/api/developer_private/developer_private_api.cc +++ b/chrome/browser/extensions/api/developer_private/developer_private_api.cc @@ -388,25 +388,19 @@ void DeveloperPrivateEventRouter::BroadcastItemStateChangedHelper( developer::EventType event_type, const std::string& extension_id, scoped_ptr<ExtensionInfoGenerator> info_generator, - const ExtensionInfoGenerator::ExtensionInfoList& infos) { + ExtensionInfoGenerator::ExtensionInfoList infos) { DCHECK_LE(infos.size(), 1u); developer::EventData event_data; event_data.event_type = event_type; event_data.item_id = extension_id; - scoped_ptr<base::DictionaryValue> dict = event_data.ToValue(); - if (!infos.empty()) { - // Hack: Ideally, we would use event_data.extension_info to set the - // extension info, but since it's an optional field, it's implemented as a - // scoped ptr, and so ownership between that and the vector of linked ptrs - // here is, well, messy. Easier to just set it like this. - dict->SetWithoutPathExpansion("extensionInfo", - infos[0]->ToValue().release()); + event_data.extension_info.reset( + new developer::ExtensionInfo(std::move(infos[0]))); } scoped_ptr<base::ListValue> args(new base::ListValue()); - args->Append(dict.release()); + args->Append(event_data.ToValue()); scoped_ptr<Event> event( new Event(events::DEVELOPER_PRIVATE_ON_ITEM_STATE_CHANGED, developer::OnItemStateChanged::kEventName, std::move(args))); @@ -510,7 +504,7 @@ DeveloperPrivateGetExtensionsInfoFunction::Run() { } void DeveloperPrivateGetExtensionsInfoFunction::OnInfosGenerated( - const ExtensionInfoGenerator::ExtensionInfoList& list) { + ExtensionInfoGenerator::ExtensionInfoList list) { Respond(ArgumentList(developer::GetExtensionsInfo::Results::Create(list))); } @@ -538,11 +532,10 @@ DeveloperPrivateGetExtensionInfoFunction::Run() { } void DeveloperPrivateGetExtensionInfoFunction::OnInfosGenerated( - const ExtensionInfoGenerator::ExtensionInfoList& list) { - DCHECK_EQ(1u, list.size()); - const linked_ptr<developer::ExtensionInfo>& info = list[0]; - Respond(info.get() ? OneArgument(info->ToValue()) : - Error(kNoSuchExtensionError)); + ExtensionInfoGenerator::ExtensionInfoList list) { + DCHECK_LE(1u, list.size()); + Respond(list.empty() ? Error(kNoSuchExtensionError) + : OneArgument(list[0].ToValue())); } DeveloperPrivateGetItemsInfoFunction::DeveloperPrivateGetItemsInfoFunction() {} @@ -564,10 +557,10 @@ ExtensionFunction::ResponseAction DeveloperPrivateGetItemsInfoFunction::Run() { } void DeveloperPrivateGetItemsInfoFunction::OnInfosGenerated( - const ExtensionInfoGenerator::ExtensionInfoList& list) { - std::vector<linked_ptr<developer::ItemInfo>> item_list; - for (const linked_ptr<developer::ExtensionInfo>& info : list) - item_list.push_back(developer_private_mangle::MangleExtensionInfo(*info)); + ExtensionInfoGenerator::ExtensionInfoList list) { + std::vector<developer::ItemInfo> item_list; + for (const developer::ExtensionInfo& info : list) + item_list.push_back(developer_private_mangle::MangleExtensionInfo(info)); Respond(ArgumentList(developer::GetItemsInfo::Results::Create(item_list))); } diff --git a/chrome/browser/extensions/api/developer_private/developer_private_api.h b/chrome/browser/extensions/api/developer_private/developer_private_api.h index 94fc694..ffbf718 100644 --- a/chrome/browser/extensions/api/developer_private/developer_private_api.h +++ b/chrome/browser/extensions/api/developer_private/developer_private_api.h @@ -132,8 +132,7 @@ class DeveloperPrivateEventRouter : public ExtensionRegistryObserver, api::developer_private::EventType event_type, const std::string& extension_id, scoped_ptr<ExtensionInfoGenerator> info_generator, - const std::vector<linked_ptr<api::developer_private::ExtensionInfo>>& - infos); + std::vector<api::developer_private::ExtensionInfo> infos); ScopedObserver<ExtensionRegistry, ExtensionRegistryObserver> extension_registry_observer_; @@ -261,8 +260,7 @@ class DeveloperPrivateGetItemsInfoFunction ResponseAction Run() override; void OnInfosGenerated( - const std::vector<linked_ptr<api::developer_private::ExtensionInfo>>& - infos); + std::vector<api::developer_private::ExtensionInfo> infos); scoped_ptr<ExtensionInfoGenerator> info_generator_; @@ -281,8 +279,7 @@ class DeveloperPrivateGetExtensionsInfoFunction ResponseAction Run() override; void OnInfosGenerated( - const std::vector<linked_ptr<api::developer_private::ExtensionInfo>>& - infos); + std::vector<api::developer_private::ExtensionInfo> infos); scoped_ptr<ExtensionInfoGenerator> info_generator_; @@ -301,8 +298,7 @@ class DeveloperPrivateGetExtensionInfoFunction ResponseAction Run() override; void OnInfosGenerated( - const std::vector<linked_ptr<api::developer_private::ExtensionInfo>>& - infos); + std::vector<api::developer_private::ExtensionInfo> infos); scoped_ptr<ExtensionInfoGenerator> info_generator_; diff --git a/chrome/browser/extensions/api/developer_private/developer_private_apitest.cc b/chrome/browser/extensions/api/developer_private/developer_private_apitest.cc index ec28633..40bb0eb 100644 --- a/chrome/browser/extensions/api/developer_private/developer_private_apitest.cc +++ b/chrome/browser/extensions/api/developer_private/developer_private_apitest.cc @@ -57,10 +57,10 @@ IN_PROC_BROWSER_TEST_F(DeveloperPrivateApiTest, InspectAppWindowView) { // There should be two inspectable views - the background page and the app // window. Find the app window. ASSERT_EQ(2u, info->views.size()); - api::developer_private::ExtensionView* window_view = nullptr; + const api::developer_private::ExtensionView* window_view = nullptr; for (const auto& view : info->views) { - if (view->type == api::developer_private::VIEW_TYPE_APP_WINDOW) { - window_view = view.get(); + if (view.type == api::developer_private::VIEW_TYPE_APP_WINDOW) { + window_view = &view; break; } } @@ -110,21 +110,20 @@ IN_PROC_BROWSER_TEST_F(DeveloperPrivateApiTest, InspectEmbeddedOptionsPage) { // The embedded options page should show up. ASSERT_EQ(1u, info->views.size()); - api::developer_private::ExtensionView* view = info->views[0].get(); - ASSERT_TRUE(view); - ASSERT_EQ(api::developer_private::VIEW_TYPE_EXTENSION_GUEST, view->type); + const api::developer_private::ExtensionView& view = info->views[0]; + ASSERT_EQ(api::developer_private::VIEW_TYPE_EXTENSION_GUEST, view.type); // Inspect the embedded options page. function = new api::DeveloperPrivateOpenDevToolsFunction(); extension_function_test_utils::RunFunction( function.get(), base::StringPrintf("[{\"renderViewId\": %d, \"renderProcessId\": %d}]", - view->render_view_id, view->render_process_id), + view.render_view_id, view.render_process_id), browser(), extension_function_test_utils::NONE); // Verify that dev tools opened. content::RenderFrameHost* rfh = content::RenderFrameHost::FromID( - view->render_process_id, view->render_view_id); + view.render_process_id, view.render_view_id); ASSERT_TRUE(rfh); content::WebContents* wc = content::WebContents::FromRenderFrameHost(rfh); ASSERT_TRUE(wc); diff --git a/chrome/browser/extensions/api/developer_private/developer_private_mangle.cc b/chrome/browser/extensions/api/developer_private/developer_private_mangle.cc index 073df5b..7f43b82 100644 --- a/chrome/browser/extensions/api/developer_private/developer_private_mangle.cc +++ b/chrome/browser/extensions/api/developer_private/developer_private_mangle.cc @@ -13,37 +13,36 @@ namespace extensions { namespace developer_private_mangle { -linked_ptr<api::developer_private::ItemInfo> MangleExtensionInfo( +api::developer_private::ItemInfo MangleExtensionInfo( const api::developer_private::ExtensionInfo& info) { - linked_ptr<api::developer_private::ItemInfo> result( - new api::developer_private::ItemInfo()); - result->id = info.id; - result->name = info.name; - result->version = info.version; - result->description = info.description; - result->may_disable = info.user_may_modify; - result->enabled = + api::developer_private::ItemInfo result; + result.id = info.id; + result.name = info.name; + result.version = info.version; + result.description = info.description; + result.may_disable = info.user_may_modify; + result.enabled = info.state == api::developer_private::EXTENSION_STATE_ENABLED; switch (info.type) { case api::developer_private::EXTENSION_TYPE_HOSTED_APP: - result->type = api::developer_private::ITEM_TYPE_HOSTED_APP; - result->is_app = true; + result.type = api::developer_private::ITEM_TYPE_HOSTED_APP; + result.is_app = true; break; case api::developer_private::EXTENSION_TYPE_PLATFORM_APP: - result->type = api::developer_private::ITEM_TYPE_PACKAGED_APP; - result->is_app = true; + result.type = api::developer_private::ITEM_TYPE_PACKAGED_APP; + result.is_app = true; break; case api::developer_private::EXTENSION_TYPE_LEGACY_PACKAGED_APP: - result->type = api::developer_private::ITEM_TYPE_LEGACY_PACKAGED_APP; - result->is_app = true; + result.type = api::developer_private::ITEM_TYPE_LEGACY_PACKAGED_APP; + result.is_app = true; break; case api::developer_private::EXTENSION_TYPE_EXTENSION: - result->type = api::developer_private::ITEM_TYPE_EXTENSION; - result->is_app = false; + result.type = api::developer_private::ITEM_TYPE_EXTENSION; + result.is_app = false; break; case api::developer_private::EXTENSION_TYPE_THEME: - result->type = api::developer_private::ITEM_TYPE_THEME; - result->is_app = false; + result.type = api::developer_private::ITEM_TYPE_THEME; + result.is_app = false; break; case api::developer_private::EXTENSION_TYPE_SHARED_MODULE: // Old api doesn't account for this. @@ -51,75 +50,70 @@ linked_ptr<api::developer_private::ItemInfo> MangleExtensionInfo( default: NOTREACHED(); } - result->allow_file_access = info.file_access.is_active; - result->wants_file_access = info.file_access.is_enabled; + result.allow_file_access = info.file_access.is_active; + result.wants_file_access = info.file_access.is_enabled; - result->icon_url = info.icon_url; + result.icon_url = info.icon_url; - result->incognito_enabled = info.incognito_access.is_active; - result->allow_incognito = info.incognito_access.is_enabled; + result.incognito_enabled = info.incognito_access.is_active; + result.allow_incognito = info.incognito_access.is_enabled; - result->is_unpacked = + result.is_unpacked = info.location == api::developer_private::LOCATION_UNPACKED; - result->allow_reload = result->is_unpacked; - result->terminated = + result.allow_reload = result.is_unpacked; + result.terminated = info.state == api::developer_private::EXTENSION_STATE_TERMINATED; if (info.path) - result->path.reset(new std::string(*info.path)); + result.path.reset(new std::string(*info.path)); if (info.options_page) - result->options_url.reset(new std::string(info.options_page->url)); + result.options_url.reset(new std::string(info.options_page->url)); if (info.launch_url) - result->app_launch_url.reset(new std::string(*info.launch_url)); + result.app_launch_url.reset(new std::string(*info.launch_url)); if (!info.home_page.url.empty()) - result->homepage_url.reset(new std::string(info.home_page.url)); - result->update_url.reset(new std::string(info.update_url)); + result.homepage_url.reset(new std::string(info.home_page.url)); + result.update_url.reset(new std::string(info.update_url)); for (const std::string& str_warning : info.install_warnings) { - scoped_ptr<api::developer_private::InstallWarning> warning( - new api::developer_private::InstallWarning()); - warning->message = str_warning; - result->install_warnings.push_back(make_linked_ptr(warning.release())); + api::developer_private::InstallWarning warning; + warning.message = str_warning; + result.install_warnings.push_back(std::move(warning)); } - for (const linked_ptr<api::developer_private::ManifestError>& error : - info.manifest_errors) { - CHECK(error.get()); - scoped_ptr<base::DictionaryValue> value = error->ToValue(); + for (const api::developer_private::ManifestError& error : + info.manifest_errors) { + scoped_ptr<base::DictionaryValue> value = error.ToValue(); value->SetInteger("type", static_cast<int>(ExtensionError::MANIFEST_ERROR)); value->SetInteger("level", static_cast<int>(logging::LOG_WARNING)); - result->manifest_errors.push_back(make_linked_ptr(value.release())); + result.manifest_errors.push_back(make_linked_ptr(value.release())); } - for (const linked_ptr<api::developer_private::RuntimeError>& error : - info.runtime_errors) { - CHECK(error.get()); - scoped_ptr<base::DictionaryValue> value = error->ToValue(); + for (const api::developer_private::RuntimeError& error : + info.runtime_errors) { + scoped_ptr<base::DictionaryValue> value = error.ToValue(); value->SetInteger("type", static_cast<int>(ExtensionError::RUNTIME_ERROR)); logging::LogSeverity severity = logging::LOG_INFO; - if (error->severity == api::developer_private::ERROR_LEVEL_WARN) + if (error.severity == api::developer_private::ERROR_LEVEL_WARN) severity = logging::LOG_WARNING; - else if (error->severity == api::developer_private::ERROR_LEVEL_ERROR) + else if (error.severity == api::developer_private::ERROR_LEVEL_ERROR) severity = logging::LOG_ERROR; value->SetInteger("level", static_cast<int>(severity)); - result->runtime_errors.push_back(make_linked_ptr(value.release())); + result.runtime_errors.push_back(make_linked_ptr(value.release())); } - result->offline_enabled = info.offline_enabled; - for (const linked_ptr<api::developer_private::ExtensionView>& view : - info.views) { - linked_ptr<api::developer_private::ItemInspectView> view_copy( - new api::developer_private::ItemInspectView()); - GURL url(view->url); + result.offline_enabled = info.offline_enabled; + for (const api::developer_private::ExtensionView& view : info.views) { + api::developer_private::ItemInspectView view_copy; + GURL url(view.url); if (url.scheme() == kExtensionScheme) { // No leading slash. - view_copy->path = url.path().substr(1); + view_copy.path = url.path().substr(1); } else { // For live pages, use the full URL. - view_copy->path = url.spec(); + view_copy.path = url.spec(); } - view_copy->render_process_id = view->render_process_id; - view_copy->render_view_id = view->render_view_id; - view_copy->incognito = view->incognito; - view_copy->generated_background_page = - view_copy->path == kGeneratedBackgroundPageFilename; - result->views.push_back(view_copy); + view_copy.render_process_id = view.render_process_id; + view_copy.render_view_id = view.render_view_id; + view_copy.incognito = view.incognito; + view_copy.generated_background_page = + view_copy.path == kGeneratedBackgroundPageFilename; + result.views.push_back(std::move(view_copy)); } return result; diff --git a/chrome/browser/extensions/api/developer_private/developer_private_mangle.h b/chrome/browser/extensions/api/developer_private/developer_private_mangle.h index 12f38e4..c3d4b5e 100644 --- a/chrome/browser/extensions/api/developer_private/developer_private_mangle.h +++ b/chrome/browser/extensions/api/developer_private/developer_private_mangle.h @@ -21,7 +21,7 @@ namespace developer_private_mangle { // Converts a developer_private::ExtensionInfo into a // developer_private::ItemInfo for compatability with deprecated API // functions. -linked_ptr<api::developer_private::ItemInfo> MangleExtensionInfo( +api::developer_private::ItemInfo MangleExtensionInfo( const api::developer_private::ExtensionInfo& info); } // namespace developer_private_mangle diff --git a/chrome/browser/extensions/api/developer_private/extension_info_generator.cc b/chrome/browser/extensions/api/developer_private/extension_info_generator.cc index 858e9b8..e19393b 100644 --- a/chrome/browser/extensions/api/developer_private/extension_info_generator.cc +++ b/chrome/browser/extensions/api/developer_private/extension_info_generator.cc @@ -7,6 +7,7 @@ #include <utility> #include "base/base64.h" +#include "base/callback_helpers.h" #include "base/location.h" #include "base/single_thread_task_runner.h" #include "base/strings/utf_string_conversions.h" @@ -103,13 +104,12 @@ void PopulateErrorBase(const ExtensionError& error, ErrorType* out) { // Given a ManifestError object, converts it into its developer_private // counterpart. -linked_ptr<developer::ManifestError> ConstructManifestError( - const ManifestError& error) { - linked_ptr<developer::ManifestError> result(new developer::ManifestError()); - PopulateErrorBase(error, result.get()); - result->manifest_key = base::UTF16ToUTF8(error.manifest_key()); +developer::ManifestError ConstructManifestError(const ManifestError& error) { + developer::ManifestError result; + PopulateErrorBase(error, &result); + result.manifest_key = base::UTF16ToUTF8(error.manifest_key()); if (!error.manifest_specific().empty()) { - result->manifest_specific.reset( + result.manifest_specific.reset( new std::string(base::UTF16ToUTF8(error.manifest_specific()))); } return result; @@ -117,40 +117,39 @@ linked_ptr<developer::ManifestError> ConstructManifestError( // Given a RuntimeError object, converts it into its developer_private // counterpart. -linked_ptr<developer::RuntimeError> ConstructRuntimeError( - const RuntimeError& error) { - linked_ptr<developer::RuntimeError> result(new developer::RuntimeError()); - PopulateErrorBase(error, result.get()); +developer::RuntimeError ConstructRuntimeError(const RuntimeError& error) { + developer::RuntimeError result; + PopulateErrorBase(error, &result); switch (error.level()) { case logging::LOG_VERBOSE: case logging::LOG_INFO: - result->severity = developer::ERROR_LEVEL_LOG; + result.severity = developer::ERROR_LEVEL_LOG; break; case logging::LOG_WARNING: - result->severity = developer::ERROR_LEVEL_WARN; + result.severity = developer::ERROR_LEVEL_WARN; break; case logging::LOG_FATAL: case logging::LOG_ERROR: - result->severity = developer::ERROR_LEVEL_ERROR; + result.severity = developer::ERROR_LEVEL_ERROR; break; default: NOTREACHED(); } - result->occurrences = error.occurrences(); + result.occurrences = error.occurrences(); // NOTE(devlin): This is called "render_view_id" in the api for legacy // reasons, but it's not a high priority to change. - result->render_view_id = error.render_frame_id(); - result->render_process_id = error.render_process_id(); - result->can_inspect = + result.render_view_id = error.render_frame_id(); + result.render_process_id = error.render_process_id(); + result.can_inspect = content::RenderFrameHost::FromID(error.render_process_id(), error.render_frame_id()) != nullptr; for (const StackFrame& f : error.stack_trace()) { - linked_ptr<developer::StackFrame> frame(new developer::StackFrame()); - frame->line_number = f.line_number; - frame->column_number = f.column_number; - frame->url = base::UTF16ToUTF8(f.source); - frame->function_name = base::UTF16ToUTF8(f.function); - result->stack_trace.push_back(frame); + developer::StackFrame frame; + frame.line_number = f.line_number; + frame.column_number = f.column_number; + frame.url = base::UTF16ToUTF8(f.source); + frame.function_name = base::UTF16ToUTF8(f.function); + result.stack_trace.push_back(std::move(frame)); } return result; } @@ -159,21 +158,21 @@ linked_ptr<developer::RuntimeError> ConstructRuntimeError( // to the list of |commands|. void ConstructCommands(CommandService* command_service, const std::string& extension_id, - std::vector<linked_ptr<developer::Command>>* commands) { - auto construct_command = [](const Command& command, - bool active, + std::vector<developer::Command>* commands) { + auto construct_command = [](const Command& command, bool active, bool is_extension_action) { - developer::Command* command_value = new developer::Command(); - command_value->description = is_extension_action ? - l10n_util::GetStringUTF8(IDS_EXTENSION_COMMANDS_GENERIC_ACTIVATE) : - base::UTF16ToUTF8(command.description()); - command_value->keybinding = + developer::Command command_value; + command_value.description = + is_extension_action + ? l10n_util::GetStringUTF8(IDS_EXTENSION_COMMANDS_GENERIC_ACTIVATE) + : base::UTF16ToUTF8(command.description()); + command_value.keybinding = base::UTF16ToUTF8(command.accelerator().GetShortcutText()); - command_value->name = command.command_name(); - command_value->is_active = active; - command_value->scope = command.global() ? developer::COMMAND_SCOPE_GLOBAL : - developer::COMMAND_SCOPE_CHROME; - command_value->is_extension_action = is_extension_action; + command_value.name = command.command_name(); + command_value.is_active = active; + command_value.scope = command.global() ? developer::COMMAND_SCOPE_GLOBAL + : developer::COMMAND_SCOPE_CHROME; + command_value.is_extension_action = is_extension_action; return command_value; }; bool active = false; @@ -182,8 +181,7 @@ void ConstructCommands(CommandService* command_service, CommandService::ALL, &browser_action, &active)) { - commands->push_back( - make_linked_ptr(construct_command(browser_action, active, true))); + commands->push_back(construct_command(browser_action, active, true)); } Command page_action; @@ -191,8 +189,7 @@ void ConstructCommands(CommandService* command_service, CommandService::ALL, &page_action, &active)) { - commands->push_back( - make_linked_ptr(construct_command(page_action, active, true))); + commands->push_back(construct_command(page_action, active, true)); } CommandMap named_commands; @@ -214,8 +211,7 @@ void ConstructCommands(CommandService* command_service, command_to_use.set_accelerator(active_command.accelerator()); command_to_use.set_global(active_command.global()); bool active = command_to_use.accelerator().key_code() != ui::VKEY_UNKNOWN; - commands->push_back( - make_linked_ptr(construct_command(command_to_use, active, false))); + commands->push_back(construct_command(command_to_use, active, false)); } } } @@ -260,8 +256,8 @@ void ExtensionInfoGenerator::CreateExtensionInfo( if (pending_image_loads_ == 0) { // Don't call the callback re-entrantly. - base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, - base::Bind(callback, list_)); + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::Bind(callback, base::Passed(&list_))); list_.clear(); } else { callback_ = callback; @@ -296,8 +292,8 @@ void ExtensionInfoGenerator::CreateExtensionsInfo( if (pending_image_loads_ == 0) { // Don't call the callback re-entrantly. - base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, - base::Bind(callback, list_)); + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::Bind(callback, base::Passed(&list_))); list_.clear(); } else { callback_ = callback; @@ -540,7 +536,7 @@ void ExtensionInfoGenerator::CreateExtensionInfoHelper( ExtensionIconSet::MATCH_BIGGER); if (icon.empty()) { info->icon_url = GetDefaultIconUrl(extension.is_app(), !is_enabled); - list_.push_back(make_linked_ptr(info.release())); + list_.push_back(std::move(*info)); } else { ++pending_image_loads_; // Max size of 128x128 is a random guess at a nice balance between being @@ -550,7 +546,7 @@ void ExtensionInfoGenerator::CreateExtensionInfoHelper( image_loader_->LoadImageAsync( &extension, icon, max_size, base::Bind(&ExtensionInfoGenerator::OnImageLoaded, - weak_factory_.GetWeakPtr(), base::Passed(std::move(info)))); + weak_factory_.GetWeakPtr(), base::Passed(&info))); } } @@ -615,18 +611,15 @@ void ExtensionInfoGenerator::OnImageLoaded( is_app, info->state != developer::EXTENSION_STATE_ENABLED); } - list_.push_back(make_linked_ptr(info.release())); + list_.push_back(std::move(*info)); --pending_image_loads_; if (pending_image_loads_ == 0) { // All done! - // We assign to a temporary callback and list and reset the stored values so - // that at the end of the method, any stored refs are destroyed. - ExtensionInfoList list; - list.swap(list_); - ExtensionInfosCallback callback = callback_; - callback_.Reset(); - callback.Run(list); // WARNING: |this| is possibly deleted after this line! + ExtensionInfoList list = std::move(list_); + list_.clear(); + base::ResetAndReturn(&callback_).Run(std::move(list)); + // WARNING: |this| is possibly deleted after this line! } } diff --git a/chrome/browser/extensions/api/developer_private/extension_info_generator.h b/chrome/browser/extensions/api/developer_private/extension_info_generator.h index 2b82e4b..5a47e84 100644 --- a/chrome/browser/extensions/api/developer_private/extension_info_generator.h +++ b/chrome/browser/extensions/api/developer_private/extension_info_generator.h @@ -35,10 +35,9 @@ class WarningService; // This class is designed to only have one generation running at a time! class ExtensionInfoGenerator { public: - using ExtensionInfoList = - std::vector<linked_ptr<api::developer_private::ExtensionInfo>>; + using ExtensionInfoList = std::vector<api::developer_private::ExtensionInfo>; - using ExtensionInfosCallback = base::Callback<void(const ExtensionInfoList&)>; + using ExtensionInfosCallback = base::Callback<void(ExtensionInfoList)>; explicit ExtensionInfoGenerator(content::BrowserContext* context); ~ExtensionInfoGenerator(); diff --git a/chrome/browser/extensions/api/developer_private/extension_info_generator_unittest.cc b/chrome/browser/extensions/api/developer_private/extension_info_generator_unittest.cc index c008601..90b6c11 100644 --- a/chrome/browser/extensions/api/developer_private/extension_info_generator_unittest.cc +++ b/chrome/browser/extensions/api/developer_private/extension_info_generator_unittest.cc @@ -4,6 +4,7 @@ #include <utility> +#include "base/callback_helpers.h" #include "base/json/json_file_value_serializer.h" #include "base/json/json_writer.h" #include "base/macros.h" @@ -57,18 +58,17 @@ class ExtensionInfoGeneratorUnitTest : public ExtensionServiceTestBase { InitializeEmptyExtensionService(); } - void OnInfosGenerated(linked_ptr<developer::ExtensionInfo>* info_out, - const ExtensionInfoGenerator::ExtensionInfoList& list) { + void OnInfosGenerated(scoped_ptr<developer::ExtensionInfo>* info_out, + ExtensionInfoGenerator::ExtensionInfoList list) { EXPECT_EQ(1u, list.size()); if (!list.empty()) - *info_out = list[0]; - quit_closure_.Run(); - quit_closure_.Reset(); + info_out->reset(new developer::ExtensionInfo(std::move(list[0]))); + base::ResetAndReturn(&quit_closure_).Run(); } scoped_ptr<developer::ExtensionInfo> GenerateExtensionInfo( const std::string& extension_id) { - linked_ptr<developer::ExtensionInfo> info; + scoped_ptr<developer::ExtensionInfo> info; base::RunLoop run_loop; quit_closure_ = run_loop.QuitClosure(); scoped_ptr<ExtensionInfoGenerator> generator( @@ -79,7 +79,7 @@ class ExtensionInfoGeneratorUnitTest : public ExtensionServiceTestBase { base::Unretained(this), base::Unretained(&info))); run_loop.Run(); - return make_scoped_ptr(info.release()); + return info; } const scoped_refptr<const Extension> CreateExtension( @@ -126,7 +126,7 @@ class ExtensionInfoGeneratorUnitTest : public ExtensionServiceTestBase { void CompareExpectedAndActualOutput( const base::FilePath& extension_path, - const InspectableViewsFinder::ViewList& views, + InspectableViewsFinder::ViewList views, const base::FilePath& expected_output_path) { std::string error; scoped_ptr<base::DictionaryValue> expected_output_data( @@ -136,7 +136,7 @@ class ExtensionInfoGeneratorUnitTest : public ExtensionServiceTestBase { // Produce test output. scoped_ptr<developer::ExtensionInfo> info = CreateExtensionInfoFromPath(extension_path, Manifest::INVALID_LOCATION); - info->views = views; + info->views = std::move(views); scoped_ptr<base::DictionaryValue> actual_output_data = info->ToValue(); ASSERT_TRUE(actual_output_data); @@ -253,7 +253,7 @@ TEST_F(ExtensionInfoGeneratorUnitTest, BasicInfoTest) { EXPECT_FALSE(info->incognito_access.is_active); ASSERT_EQ(2u, info->runtime_errors.size()); const api::developer_private::RuntimeError& runtime_error = - *info->runtime_errors[0]; + info->runtime_errors[0]; EXPECT_EQ(extension->id(), runtime_error.extension_id); EXPECT_EQ(api::developer_private::ERROR_TYPE_RUNTIME, runtime_error.type); EXPECT_EQ(api::developer_private::ERROR_LEVEL_ERROR, @@ -261,11 +261,11 @@ TEST_F(ExtensionInfoGeneratorUnitTest, BasicInfoTest) { EXPECT_EQ(1u, runtime_error.stack_trace.size()); ASSERT_EQ(1u, info->manifest_errors.size()); const api::developer_private::RuntimeError& runtime_error_verbose = - *info->runtime_errors[1]; + info->runtime_errors[1]; EXPECT_EQ(api::developer_private::ERROR_LEVEL_LOG, runtime_error_verbose.severity); const api::developer_private::ManifestError& manifest_error = - *info->manifest_errors[0]; + info->manifest_errors[0]; EXPECT_EQ(extension->id(), manifest_error.extension_id); // Test an extension that isn't unpacked. @@ -292,24 +292,25 @@ TEST_F(ExtensionInfoGeneratorUnitTest, GenerateExtensionsJSONData) { .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj") .AppendASCII("1.0.0.0"); - InspectableViewsFinder::ViewList views; - views.push_back(InspectableViewsFinder::ConstructView( - GURL("chrome-extension://behllobkkfkfnphdnhnkndlbkcpglgmj/bar.html"), - 42, 88, true, false, VIEW_TYPE_TAB_CONTENTS)); - views.push_back(InspectableViewsFinder::ConstructView( - GURL("chrome-extension://behllobkkfkfnphdnhnkndlbkcpglgmj/dog.html"), - 0, 0, false, true, VIEW_TYPE_TAB_CONTENTS)); - base::FilePath expected_outputs_path = data_dir().AppendASCII("api_test") .AppendASCII("developer") .AppendASCII("generated_output"); - CompareExpectedAndActualOutput( - extension_path, - views, - expected_outputs_path.AppendASCII( - "behllobkkfkfnphdnhnkndlbkcpglgmj.json")); + { + InspectableViewsFinder::ViewList views; + views.push_back(InspectableViewsFinder::ConstructView( + GURL("chrome-extension://behllobkkfkfnphdnhnkndlbkcpglgmj/bar.html"), + 42, 88, true, false, VIEW_TYPE_TAB_CONTENTS)); + views.push_back(InspectableViewsFinder::ConstructView( + GURL("chrome-extension://behllobkkfkfnphdnhnkndlbkcpglgmj/dog.html"), 0, + 0, false, true, VIEW_TYPE_TAB_CONTENTS)); + + CompareExpectedAndActualOutput( + extension_path, std::move(views), + expected_outputs_path.AppendASCII( + "behllobkkfkfnphdnhnkndlbkcpglgmj.json")); + } #if !defined(OS_CHROMEOS) // Test Extension2 @@ -318,16 +319,21 @@ TEST_F(ExtensionInfoGeneratorUnitTest, GenerateExtensionsJSONData) { .AppendASCII("hpiknbiabeeppbpihjehijgoemciehgk") .AppendASCII("2"); - // It's OK to have duplicate URLs, so long as the IDs are different. - views[0]->url = - "chrome-extension://hpiknbiabeeppbpihjehijgoemciehgk/bar.html"; - views[1]->url = views[0]->url; - - CompareExpectedAndActualOutput( - extension_path, - views, - expected_outputs_path.AppendASCII( - "hpiknbiabeeppbpihjehijgoemciehgk.json")); + { + // It's OK to have duplicate URLs, so long as the IDs are different. + InspectableViewsFinder::ViewList views; + views.push_back(InspectableViewsFinder::ConstructView( + GURL("chrome-extension://hpiknbiabeeppbpihjehijgoemciehgk/bar.html"), + 42, 88, true, false, VIEW_TYPE_TAB_CONTENTS)); + views.push_back(InspectableViewsFinder::ConstructView( + GURL("chrome-extension://hpiknbiabeeppbpihjehijgoemciehgk/bar.html"), 0, + 0, false, true, VIEW_TYPE_TAB_CONTENTS)); + + CompareExpectedAndActualOutput( + extension_path, std::move(views), + expected_outputs_path.AppendASCII( + "hpiknbiabeeppbpihjehijgoemciehgk.json")); + } #endif // Test Extension3 @@ -335,13 +341,10 @@ TEST_F(ExtensionInfoGeneratorUnitTest, GenerateExtensionsJSONData) { .AppendASCII("Extensions") .AppendASCII("bjafgdebaacbbbecmhlhpofkepfkgcpa") .AppendASCII("1.0"); - views.clear(); - - CompareExpectedAndActualOutput( - extension_path, - views, - expected_outputs_path.AppendASCII( - "bjafgdebaacbbbecmhlhpofkepfkgcpa.json")); + CompareExpectedAndActualOutput(extension_path, + InspectableViewsFinder::ViewList(), + expected_outputs_path.AppendASCII( + "bjafgdebaacbbbecmhlhpofkepfkgcpa.json")); } // Test that the all_urls checkbox only shows up for extensions that want all diff --git a/chrome/browser/extensions/api/developer_private/inspectable_views_finder.cc b/chrome/browser/extensions/api/developer_private/inspectable_views_finder.cc index f12df3a..e505194 100644 --- a/chrome/browser/extensions/api/developer_private/inspectable_views_finder.cc +++ b/chrome/browser/extensions/api/developer_private/inspectable_views_finder.cc @@ -38,45 +38,44 @@ InspectableViewsFinder::View InspectableViewsFinder::ConstructView( bool incognito, bool is_iframe, ViewType type) { - linked_ptr<api::developer_private::ExtensionView> view( - new api::developer_private::ExtensionView()); - view->url = url.spec(); - view->render_process_id = render_process_id; + api::developer_private::ExtensionView view; + view.url = url.spec(); + view.render_process_id = render_process_id; // NOTE(devlin): This is called "render_view_id" in the api for legacy // reasons, but it's not a high priority to change. - view->render_view_id = render_frame_id; - view->incognito = incognito; - view->is_iframe = is_iframe; + view.render_view_id = render_frame_id; + view.incognito = incognito; + view.is_iframe = is_iframe; switch (type) { case VIEW_TYPE_APP_WINDOW: - view->type = api::developer_private::VIEW_TYPE_APP_WINDOW; + view.type = api::developer_private::VIEW_TYPE_APP_WINDOW; break; case VIEW_TYPE_BACKGROUND_CONTENTS: - view->type = api::developer_private::VIEW_TYPE_BACKGROUND_CONTENTS; + view.type = api::developer_private::VIEW_TYPE_BACKGROUND_CONTENTS; break; case VIEW_TYPE_COMPONENT: - view->type = api::developer_private::VIEW_TYPE_COMPONENT; + view.type = api::developer_private::VIEW_TYPE_COMPONENT; break; case VIEW_TYPE_EXTENSION_BACKGROUND_PAGE: - view->type = api::developer_private::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE; + view.type = api::developer_private::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE; break; case VIEW_TYPE_EXTENSION_DIALOG: - view->type = api::developer_private::VIEW_TYPE_EXTENSION_DIALOG; + view.type = api::developer_private::VIEW_TYPE_EXTENSION_DIALOG; break; case VIEW_TYPE_EXTENSION_GUEST: - view->type = api::developer_private::VIEW_TYPE_EXTENSION_GUEST; + view.type = api::developer_private::VIEW_TYPE_EXTENSION_GUEST; break; case VIEW_TYPE_EXTENSION_POPUP: - view->type = api::developer_private::VIEW_TYPE_EXTENSION_POPUP; + view.type = api::developer_private::VIEW_TYPE_EXTENSION_POPUP; break; case VIEW_TYPE_LAUNCHER_PAGE: - view->type = api::developer_private::VIEW_TYPE_LAUNCHER_PAGE; + view.type = api::developer_private::VIEW_TYPE_LAUNCHER_PAGE; break; case VIEW_TYPE_PANEL: - view->type = api::developer_private::VIEW_TYPE_PANEL; + view.type = api::developer_private::VIEW_TYPE_PANEL; break; case VIEW_TYPE_TAB_CONTENTS: - view->type = api::developer_private::VIEW_TYPE_TAB_CONTENTS; + view.type = api::developer_private::VIEW_TYPE_TAB_CONTENTS; break; default: NOTREACHED(); diff --git a/chrome/browser/extensions/api/developer_private/inspectable_views_finder.h b/chrome/browser/extensions/api/developer_private/inspectable_views_finder.h index 20346aa..f904c37 100644 --- a/chrome/browser/extensions/api/developer_private/inspectable_views_finder.h +++ b/chrome/browser/extensions/api/developer_private/inspectable_views_finder.h @@ -28,7 +28,7 @@ struct ExtensionView; // by the developerPrivate API structure and schema compiler. class InspectableViewsFinder { public: - using View = linked_ptr<api::developer_private::ExtensionView>; + using View = api::developer_private::ExtensionView; using ViewList = std::vector<View>; explicit InspectableViewsFinder(Profile* profile); diff --git a/chrome/common/extensions/api/developer_private.idl b/chrome/common/extensions/api/developer_private.idl index 564b1cb..bf6e490 100644 --- a/chrome/common/extensions/api/developer_private.idl +++ b/chrome/common/extensions/api/developer_private.idl @@ -5,7 +5,7 @@ // developerPrivate API. // This is a private API exposing developing and debugging functionalities for // apps and extensions. -namespace developerPrivate { +[use_movable_types=true] namespace developerPrivate { // DEPRECATED: Prefer ExtensionType. enum ItemType { diff --git a/tools/json_schema_compiler/cc_generator.py b/tools/json_schema_compiler/cc_generator.py index ac1679b..499671f 100644 --- a/tools/json_schema_compiler/cc_generator.py +++ b/tools/json_schema_compiler/cc_generator.py @@ -126,8 +126,17 @@ class _Generator(object): (c.Append('%s::%s()' % (classname_in_namespace, classname)) .Cblock(self._GenerateInitializersAndBody(type_)) .Append('%s::~%s() {}' % (classname_in_namespace, classname)) - .Append() ) + if 'use_movable_types' in type_.namespace.compiler_options: + # Note: we use 'rhs' because some API objects have a member 'other'. + (c.Append('%s::%s(%s&& rhs)' % + (classname_in_namespace, classname, classname)) + .Cblock(self._GenerateMoveCtor(type_)) + .Append('%s& %s::operator=(%s&& rhs)' % + (classname_in_namespace, classname_in_namespace, + classname)) + .Cblock(self._GenerateMoveAssignOperator(type_)) + ) if type_.origin.from_json: c.Cblock(self._GenerateTypePopulate(classname_in_namespace, type_)) if cpp_namespace is None: # only generate for top-level types @@ -178,12 +187,78 @@ class _Generator(object): raise TypeError(t) if items: - s = ': %s' % (', '.join(items)) + s = ': %s' % (',\n'.join(items)) else: s = '' s = s + ' {}' return Code().Append(s) + def _GetMoveProps(self, type_, copy_str, move_str): + """Returns a tuple of (props, dicts) for the type. + + |props| is a list of all the copyable or movable properties generated using + the copy_str and move_str, and |dicts| is a list of all the dictionary + properties by name. + + Properties: + - |type_| the Type to get the properties from + - |copy_str| the string to use when copying a value; should have two + placeholders to take the property name. + - |move_str| the string to use when moving a value; should have two + placeholders to take the property name. + """ + props = [] + dicts = [] + for prop in type_.properties.values(): + t = prop.type_ + + real_t = self._type_helper.FollowRef(t) + if (prop.optional or + t.property_type == PropertyType.ANY or + t.property_type == PropertyType.ARRAY or + t.property_type == PropertyType.BINARY or + t.property_type == PropertyType.CHOICES or + t.property_type == PropertyType.OBJECT or + t.property_type == PropertyType.REF or + t.property_type == PropertyType.STRING): + props.append(move_str % (prop.unix_name, prop.unix_name)) + elif t.property_type == PropertyType.FUNCTION: + dicts.append(prop.unix_name) + elif (real_t.property_type == PropertyType.ENUM or + t.property_type == PropertyType.INTEGER or + t.property_type == PropertyType.DOUBLE or + t.property_type == PropertyType.BOOLEAN): + props.append(copy_str % (prop.unix_name, prop.unix_name)) + else: + raise TypeError(t) + + return (props, dicts) + + def _GenerateMoveCtor(self, type_): + props, dicts = self._GetMoveProps(type_, '%s(rhs.%s)', + '%s(std::move(rhs.%s))') + s = '' + if props: + s = s + ': %s' % (',\n'.join(props)) + s = s + '{' + for item in dicts: + s = s + ('\n%s.Swap(&rhs.%s);' % (item, item)) + s = s + '\n}' + + return Code().Append(s) + + def _GenerateMoveAssignOperator(self, type_): + props, dicts = self._GetMoveProps(type_, '%s = rhs.%s;', + '%s = std::move(rhs.%s);') + s = '{\n' + if props: + s = s + '\n'.join(props) + for item in dicts: + s = s + ('\n%s.Swap(&rhs.%s);' % (item, item)) + s = s + '\nreturn *this;\n}' + + return Code().Append(s) + def _GenerateTypePopulate(self, cpp_namespace, type_): """Generates the function for populating a type given a pointer to it. diff --git a/tools/json_schema_compiler/cpp_type_generator.py b/tools/json_schema_compiler/cpp_type_generator.py index 3d307a9..24cbf61 100644 --- a/tools/json_schema_compiler/cpp_type_generator.py +++ b/tools/json_schema_compiler/cpp_type_generator.py @@ -121,7 +121,12 @@ class CppTypeGenerator(object): # TODO(kalman): change this - but it's an exceedingly far-reaching change. if not self.FollowRef(type_).property_type == PropertyType.ENUM: if is_in_container and (is_ptr or not self.IsCopyable(type_)): - cpp_type = 'linked_ptr<%s>' % cpp_util.PadForGenerics(cpp_type) + # Only wrap the object in a linked_ptr if this API isn't using movable + # types. Since base::Values aren't yet movable, wrap them too. + # TODO(devlin): Eventually, movable types should be the default. + if (not 'use_movable_types' in type_.namespace.compiler_options or + cpp_type == 'base::Value' or cpp_type == 'base::DictionaryValue'): + cpp_type = 'linked_ptr<%s>' % cpp_util.PadForGenerics(cpp_type) elif is_ptr: cpp_type = 'scoped_ptr<%s>' % cpp_util.PadForGenerics(cpp_type) diff --git a/tools/json_schema_compiler/cpp_type_generator_test.py b/tools/json_schema_compiler/cpp_type_generator_test.py index 51fcfe9..69d80dc 100755 --- a/tools/json_schema_compiler/cpp_type_generator_test.py +++ b/tools/json_schema_compiler/cpp_type_generator_test.py @@ -6,6 +6,7 @@ from cpp_namespace_environment import CppNamespaceEnvironment from cpp_type_generator import CppTypeGenerator from json_schema import CachedLoad +import idl_schema import model import unittest @@ -49,6 +50,10 @@ class CppTypeGeneratorTest(unittest.TestCase): self.content_settings_json = CachedLoad('test/content_settings.json') self.content_settings = self.models['content_settings'].AddNamespace( self.content_settings_json[0], 'path/to/content_settings.json') + self.objects_movable_idl = idl_schema.Load('test/objects_movable.idl') + self.objects_movable = self.models['objects_movable'].AddNamespace( + self.objects_movable_idl[0], 'path/to/objects_movable.idl', + include_compiler_options=True) def testGenerateIncludesAndForwardDeclarations(self): m = model.Model() @@ -153,6 +158,14 @@ class CppTypeGeneratorTest(unittest.TestCase): manager.GetCppType( self.permissions.types['Permissions'].properties['origins'].type_)) + manager = CppTypeGenerator(self.models.get('objects_movable'), + _FakeSchemaLoader(None)) + self.assertEquals( + 'std::vector<MovablePod>', + manager.GetCppType( + self.objects_movable.types['MovableParent']. + properties['pods'].type_)) + def testGetCppTypeLocalRef(self): manager = CppTypeGenerator(self.models.get('tabs'), _FakeSchemaLoader(None)) self.assertEquals( diff --git a/tools/json_schema_compiler/h_generator.py b/tools/json_schema_compiler/h_generator.py index 173e549..5452885 100644 --- a/tools/json_schema_compiler/h_generator.py +++ b/tools/json_schema_compiler/h_generator.py @@ -228,6 +228,10 @@ class _Generator(object): .Append('%(classname)s();') .Append('~%(classname)s();') ) + if 'use_movable_types' in type_.namespace.compiler_options: + (c.Append('%(classname)s(%(classname)s&& rhs);') + .Append('%(classname)s& operator=(%(classname)s&& rhs);') + ) if type_.origin.from_json: (c.Append() .Comment('Populates a %s object from a base::Value. Returns' diff --git a/tools/json_schema_compiler/idl_schema.py b/tools/json_schema_compiler/idl_schema.py index 58efe28..5fda70e 100755 --- a/tools/json_schema_compiler/idl_schema.py +++ b/tools/json_schema_compiler/idl_schema.py @@ -500,6 +500,8 @@ class IDLSchema(object): compiler_options['implemented_in'] = node.value elif node.name == 'camel_case_enum_to_string': compiler_options['camel_case_enum_to_string'] = node.value + elif node.name == 'use_movable_types': + compiler_options['use_movable_types'] = node.value elif node.name == 'deprecated': deprecated = str(node.value) elif node.name == 'documentation_title': diff --git a/tools/json_schema_compiler/test/BUILD.gn b/tools/json_schema_compiler/test/BUILD.gn index 33e6219..e12493f 100644 --- a/tools/json_schema_compiler/test/BUILD.gn +++ b/tools/json_schema_compiler/test/BUILD.gn @@ -23,6 +23,7 @@ json_schema_api("api") { "idl_other_namespace.idl", "idl_other_namespace_sub_namespace.idl", "objects.json", + "objects_movable.idl", "simple_api.json", ] diff --git a/tools/json_schema_compiler/test/json_schema_compiler_tests.gyp b/tools/json_schema_compiler/test/json_schema_compiler_tests.gyp index 404e9b0..a0203dc 100644 --- a/tools/json_schema_compiler/test/json_schema_compiler_tests.gyp +++ b/tools/json_schema_compiler/test/json_schema_compiler_tests.gyp @@ -25,6 +25,7 @@ 'idl_other_namespace_sub_namespace.idl', 'idl_object_types.idl', 'objects.json', + 'objects_movable.idl', 'simple_api.json', 'error_generation.json' ], diff --git a/tools/json_schema_compiler/test/objects_movable.idl b/tools/json_schema_compiler/test/objects_movable.idl new file mode 100644 index 0000000..b09f1a8 --- /dev/null +++ b/tools/json_schema_compiler/test/objects_movable.idl @@ -0,0 +1,23 @@ +// Copyright 2016 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. + +// Movable Objects. +[use_movable_types=true] namespace objectsMovable { + enum Foo { + BAR, + BAZ + }; + + dictionary MovablePod { + Foo foo; + DOMString str; + long num; + boolean b; + }; + + dictionary MovableParent { + MovablePod[] pods; + DOMString[] strs; + }; +}; diff --git a/tools/json_schema_compiler/test/objects_unittest.cc b/tools/json_schema_compiler/test/objects_unittest.cc index 5e28386..2cb6fc6 100644 --- a/tools/json_schema_compiler/test/objects_unittest.cc +++ b/tools/json_schema_compiler/test/objects_unittest.cc @@ -2,14 +2,15 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "tools/json_schema_compiler/test/objects.h" - #include <stddef.h> #include "base/json/json_writer.h" #include "testing/gtest/include/gtest/gtest.h" +#include "tools/json_schema_compiler/test/objects.h" +#include "tools/json_schema_compiler/test/objects_movable.h" using namespace test::api::objects; +using namespace test::api::objects_movable; TEST(JsonSchemaCompilerObjectsTest, ObjectParamParamsCreate) { { @@ -71,3 +72,38 @@ TEST(JsonSchemaCompilerObjectsTest, OnObjectFiredCreate) { ASSERT_TRUE(results->GetDictionary(0, &result)); ASSERT_TRUE(result->Equals(&expected)); } +TEST(JsonSchemaCompilerMovableObjectsTest, MovableObjectsTest) { + std::vector<MovablePod> pods; + { + MovablePod pod; + pod.foo = FOO_BAR; + pod.str = "str1"; + pod.num = 42; + pod.b = true; + pods.push_back(std::move(pod)); + } + { + MovablePod pod; + pod.foo = FOO_BAZ; + pod.str = "str2"; + pod.num = 45; + pod.b = false; + pods.push_back(std::move(pod)); + } + MovableParent parent; + parent.pods = std::move(pods); + parent.strs.push_back("pstr"); + + MovableParent parent2(std::move(parent)); + ASSERT_EQ(2u, parent2.pods.size()); + EXPECT_EQ(FOO_BAR, parent2.pods[0].foo); + EXPECT_EQ("str1", parent2.pods[0].str); + EXPECT_EQ(42, parent2.pods[0].num); + EXPECT_TRUE(parent2.pods[0].b); + EXPECT_EQ(FOO_BAZ, parent2.pods[1].foo); + EXPECT_EQ("str2", parent2.pods[1].str); + EXPECT_EQ(45, parent2.pods[1].num); + EXPECT_FALSE(parent2.pods[1].b); + ASSERT_EQ(1u, parent2.strs.size()); + EXPECT_EQ("pstr", parent2.strs[0]); +} diff --git a/tools/json_schema_compiler/util.h b/tools/json_schema_compiler/util.h index 59d65d0..945f061 100644 --- a/tools/json_schema_compiler/util.h +++ b/tools/json_schema_compiler/util.h @@ -57,6 +57,19 @@ bool PopulateItem(const base::Value& from, linked_ptr<T>* out) { return true; } +// This template is used for types generated by tools/json_schema_compiler. +template <class T> +bool PopulateItem(const base::Value& from, T* out) { + const base::DictionaryValue* dict = nullptr; + if (!from.GetAsDictionary(&dict)) + return false; + T obj; + if (!T::Populate(*dict, &obj)) + return false; + *out = std::move(obj); + return true; +} + // This template is used for types generated by tools/json_schema_compiler with // error generation enabled. template <class T> @@ -82,7 +95,9 @@ bool PopulateArrayFromList(const base::ListValue& list, std::vector<T>* out) { for (const base::Value* value : list) { if (!PopulateItem(*value, &item)) return false; - out->push_back(item); + // T might not be movable, but in that case it should be copyable, and this + // will still work. + out->push_back(std::move(item)); } return true; @@ -148,12 +163,18 @@ void AddItemToList(const linked_ptr<T>& from, base::ListValue* out) { out->Append(from->ToValue().release()); } +// This template is used for types generated by tools/json_schema_compiler. +template <class T> +void AddItemToList(const T& from, base::ListValue* out) { + out->Append(from.ToValue()); +} + // Set |out| to the the contents of |from|. Requires PopulateItem to be // implemented for |T|. template <class T> void PopulateListFromArray(const std::vector<T>& from, base::ListValue* out) { out->Clear(); - for (const auto& item : from) + for (const T& item : from) AddItemToList(item, out); } |