diff options
author | rdevlin.cronin <rdevlin.cronin@chromium.org> | 2016-03-22 15:11:00 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-03-22 22:13:13 +0000 |
commit | 63ef43c0af90ca5f23d0606e2394b0e5f25fc0b5 (patch) | |
tree | 14ff1d1b8c8f9aae47fe4456d2bce99ad8f55b60 /chrome/browser/extensions/api/developer_private | |
parent | 3a1e77f21225b290585e1e9432f3cdecbf63866d (diff) | |
download | chromium_src-63ef43c0af90ca5f23d0606e2394b0e5f25fc0b5.zip chromium_src-63ef43c0af90ca5f23d0606e2394b0e5f25fc0b5.tar.gz chromium_src-63ef43c0af90ca5f23d0606e2394b0e5f25fc0b5.tar.bz2 |
[Extensions] Update generated code to support move operations
Generated extensions code currently uses linked_ptrs for non-copyable types
stored in containers. Instead, these types should just be movable. Update the
code generator to support move operations for flagged idl files and apply it
to the developerPrivate API as a first step/POC.
BUG=595949
Review URL: https://codereview.chromium.org/1811413002
Cr-Commit-Position: refs/heads/master@{#382704}
Diffstat (limited to 'chrome/browser/extensions/api/developer_private')
10 files changed, 194 insertions, 218 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); |