Skip to content

Commit

Permalink
Re-write many calls to WrapUnique() with MakeUnique()
Browse files Browse the repository at this point in the history
A mostly-automated change to convert instances of WrapUnique(new Foo(...)) to
MakeUnique<Foo>(...). See the thread at
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/iQgMedVA8-k
for background.

To avoid requiring too many manual fixups, the change skips some cases that are
frequently problematic. In particular, in methods named Foo::Method() it will
not try to change WrapUnique(new Foo()) to MakeUnique<Foo>(). This is because
Foo::Method() may be accessing an internal constructor of Foo.

Cases where MakeUnique<NestedClass>(...) is called within a method of
OuterClass are common but hard to detect automatically, so have been fixed-up
manually.

The only types of manual fix ups applied are:
1) Revert MakeUnique back to WrapUnique
2) Change NULL to nullptr in argument list (MakeUnique cannot forward NULL
   correctly)
3) Add base:: namespace qualifier where missing.

WrapUnique(new Foo) has not been converted to MakeUnique<Foo>() as this might
change behaviour if Foo does not have a user-defined constructor. For example,
WrapUnique(new int) creates an unitialised integer, but MakeUnique<int>()
creates an integer initialised to 0.

git cl format has been been run over the CL. Spot-checking has uncovered no
cases of mis-formatting.

Miscellaneous minor cleanups were applied.

BUG=637812

Review-Url: https://codereview.chromium.org/2341693002
Cr-Commit-Position: refs/heads/master@{#418795}
  • Loading branch information
ricea authored and Commit bot committed Sep 15, 2016
1 parent d64a5f6 commit 0c39556
Show file tree
Hide file tree
Showing 23 changed files with 77 additions and 77 deletions.
5 changes: 2 additions & 3 deletions chrome/common/chrome_content_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -720,9 +720,8 @@ bool ChromeContentClient::IsSupplementarySiteIsolationModeEnabled() {
}

content::OriginTrialPolicy* ChromeContentClient::GetOriginTrialPolicy() {
if (!origin_trial_policy_) {
origin_trial_policy_ = base::WrapUnique(new ChromeOriginTrialPolicy());
}
if (!origin_trial_policy_)
origin_trial_policy_ = base::MakeUnique<ChromeOriginTrialPolicy>();
return origin_trial_policy_.get();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,11 @@ std::unique_ptr<ActionInfo> PageActionManifestTest::LoadAction(
const ActionInfo* page_action_info =
ActionInfo::GetPageActionInfo(extension.get());
EXPECT_TRUE(page_action_info);
if (page_action_info) {
return base::WrapUnique(new ActionInfo(*page_action_info));
}
if (page_action_info)
return base::MakeUnique<ActionInfo>(*page_action_info);
ADD_FAILURE() << "Expected manifest in " << manifest_filename
<< " to include a page_action section.";
return std::unique_ptr<ActionInfo>();
return nullptr;
}

TEST_F(PageActionManifestTest, ManifestVersion2) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ TEST_F(PlatformAppsManifestTest, CertainApisRequirePlatformApps) {
permissions->AppendString(api_name);
manifest->Set("permissions", permissions);
manifests.push_back(
base::WrapUnique(new ManifestData(manifest->CreateDeepCopy(), "")));
base::MakeUnique<ManifestData>(manifest->CreateDeepCopy(), ""));
}
// First try to load without any flags. This should warn for every API.
for (const std::unique_ptr<ManifestData>& manifest : manifests) {
Expand Down
18 changes: 9 additions & 9 deletions chrome/installer/util/beacons.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,25 +54,25 @@ namespace installer_util {
std::unique_ptr<Beacon> MakeLastOsUpgradeBeacon(
bool system_install,
const AppRegistrationData& registration_data) {
return base::WrapUnique(new Beacon(L"LastOsUpgrade", Beacon::BeaconType::LAST,
Beacon::BeaconScope::PER_INSTALL,
system_install, registration_data));
return base::MakeUnique<Beacon>(L"LastOsUpgrade", Beacon::BeaconType::LAST,
Beacon::BeaconScope::PER_INSTALL,
system_install, registration_data);
}

std::unique_ptr<Beacon> MakeLastWasDefaultBeacon(
bool system_install,
const AppRegistrationData& registration_data) {
return base::WrapUnique(new Beacon(
L"LastWasDefault", Beacon::BeaconType::LAST,
Beacon::BeaconScope::PER_USER, system_install, registration_data));
return base::MakeUnique<Beacon>(L"LastWasDefault", Beacon::BeaconType::LAST,
Beacon::BeaconScope::PER_USER, system_install,
registration_data);
}

std::unique_ptr<Beacon> MakeFirstNotDefaultBeacon(
bool system_install,
const AppRegistrationData& registration_data) {
return base::WrapUnique(new Beacon(
L"FirstNotDefault", Beacon::BeaconType::FIRST,
Beacon::BeaconScope::PER_USER, system_install, registration_data));
return base::MakeUnique<Beacon>(L"FirstNotDefault", Beacon::BeaconType::FIRST,
Beacon::BeaconScope::PER_USER, system_install,
registration_data);
}

// Beacon ----------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions chrome/installer/util/browser_distribution.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ BrowserDistribution::Type GetCurrentDistributionType() {

BrowserDistribution::BrowserDistribution()
: type_(CHROME_BROWSER),
app_reg_data_(base::WrapUnique(
new NonUpdatingAppRegistrationData(L"Software\\Chromium"))) {}
app_reg_data_(base::MakeUnique<NonUpdatingAppRegistrationData>(
L"Software\\Chromium")) {}

BrowserDistribution::BrowserDistribution(
Type type,
Expand Down
2 changes: 1 addition & 1 deletion chrome/installer/util/lzma_file_allocator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ TEST_F(LzmaFileAllocatorTest, SizeIsZeroTest) {

TEST_F(LzmaFileAllocatorTest, DeleteAfterCloseTest) {
std::unique_ptr<LzmaFileAllocator> allocator =
base::WrapUnique(new LzmaFileAllocator(temp_dir_.path()));
base::MakeUnique<LzmaFileAllocator>(temp_dir_.path());
base::FilePath file_path = allocator->mapped_file_path_;
ASSERT_TRUE(base::PathExists(file_path));
allocator.reset();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class ScopedUserProtocolEntryTest : public testing::Test {

void CreateScopedUserProtocolEntryAndVerifyRegistryValue(
const base::string16& expected_entry_value) {
entry_ = base::WrapUnique(new ScopedUserProtocolEntry(L"http"));
entry_ = base::MakeUnique<ScopedUserProtocolEntry>(L"http");
ASSERT_TRUE(RegistryEntry(kProtocolEntryKeyPath, kProtocolEntryName,
expected_entry_value)
.ExistsInRegistry(RegistryEntry::LOOK_IN_HKCU));
Expand Down
2 changes: 1 addition & 1 deletion chrome/renderer/extensions/extension_localization_peer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ void ExtensionLocalizationPeer::OnCompletedRequest(

original_peer_->OnReceivedResponse(response_info_);
if (!data_.empty())
original_peer_->OnReceivedData(base::WrapUnique(new StringData(data_)));
original_peer_->OnReceivedData(base::MakeUnique<StringData>(data_));
original_peer_->OnCompletedRequest(error_code, was_ignored_by_handler,
stale_copy_in_cache, completion_time,
total_transfer_size);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,13 @@ TEST_F(ExtensionLocalizationPeerTest, OnReceivedData) {
EXPECT_TRUE(GetData().empty());

const std::string data_chunk("12345");
filter_peer_->OnReceivedData(base::WrapUnique(new content::FixedReceivedData(
data_chunk.data(), data_chunk.length(), -1, 0)));
filter_peer_->OnReceivedData(base::MakeUnique<content::FixedReceivedData>(
data_chunk.data(), data_chunk.length(), -1, 0));

EXPECT_EQ(data_chunk, GetData());

filter_peer_->OnReceivedData(base::WrapUnique(new content::FixedReceivedData(
data_chunk.data(), data_chunk.length(), -1, 0)));
filter_peer_->OnReceivedData(base::MakeUnique<content::FixedReceivedData>(
data_chunk.data(), data_chunk.length(), -1, 0));
EXPECT_EQ(data_chunk + data_chunk, GetData());
}

Expand Down
25 changes: 12 additions & 13 deletions chrome/renderer/pepper/chrome_renderer_pepper_host_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,19 @@ ChromeRendererPepperHostFactory::CreateResourceHost(
ppapi::PERMISSION_FLASH)) {
switch (message.type()) {
case PpapiHostMsg_Flash_Create::ID: {
return base::WrapUnique(
new PepperFlashRendererHost(host_, instance, resource));
return base::MakeUnique<PepperFlashRendererHost>(host_, instance,
resource);
}
case PpapiHostMsg_FlashFullscreen_Create::ID: {
return base::WrapUnique(
new PepperFlashFullscreenHost(host_, instance, resource));
return base::MakeUnique<PepperFlashFullscreenHost>(host_, instance,
resource);
}
case PpapiHostMsg_FlashMenu_Create::ID: {
ppapi::proxy::SerializedFlashMenu serialized_menu;
if (ppapi::UnpackMessage<PpapiHostMsg_FlashMenu_Create>(
message, &serialized_menu)) {
return base::WrapUnique(new PepperFlashMenuHost(
host_, instance, resource, serialized_menu));
return base::MakeUnique<PepperFlashMenuHost>(
host_, instance, resource, serialized_menu);
}
break;
}
Expand All @@ -76,23 +76,22 @@ ChromeRendererPepperHostFactory::CreateResourceHost(
PP_PrivateFontCharset charset;
if (ppapi::UnpackMessage<PpapiHostMsg_FlashFontFile_Create>(
message, &description, &charset)) {
return base::WrapUnique(new PepperFlashFontFileHost(
host_, instance, resource, description, charset));
return base::MakeUnique<PepperFlashFontFileHost>(
host_, instance, resource, description, charset);
}
break;
}
case PpapiHostMsg_FlashDRM_Create::ID:
return base::WrapUnique(
new PepperFlashDRMRendererHost(host_, instance, resource));
return base::MakeUnique<PepperFlashDRMRendererHost>(host_, instance,
resource);
}
}

if (host_->GetPpapiHost()->permissions().HasPermission(
ppapi::PERMISSION_PRIVATE)) {
switch (message.type()) {
case PpapiHostMsg_PDF_Create::ID: {
return base::WrapUnique(
new pdf::PepperPDFHost(host_, instance, resource));
return base::MakeUnique<pdf::PepperPDFHost>(host_, instance, resource);
}
}
}
Expand All @@ -103,7 +102,7 @@ ChromeRendererPepperHostFactory::CreateResourceHost(
// access to the other private interfaces.
switch (message.type()) {
case PpapiHostMsg_UMA_Create::ID: {
return base::WrapUnique(new PepperUMAHost(host_, instance, resource));
return base::MakeUnique<PepperUMAHost>(host_, instance, resource);
}
}

Expand Down
4 changes: 2 additions & 2 deletions chrome/renderer/pepper/pepper_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ void PepperHelper::DidCreatePepperPlugin(content::RendererPpapiHost* host) {
// TODO(brettw) figure out how to hook up the host factory. It needs some
// kind of filter-like system to allow dynamic additions.
host->GetPpapiHost()->AddHostFactoryFilter(
base::WrapUnique(new ChromeRendererPepperHostFactory(host)));
base::MakeUnique<ChromeRendererPepperHostFactory>(host));
host->GetPpapiHost()->AddInstanceMessageFilter(
base::WrapUnique(new PepperSharedMemoryMessageFilter(host)));
base::MakeUnique<PepperSharedMemoryMessageFilter>(host));
}

void PepperHelper::OnDestruct() {
Expand Down
16 changes: 8 additions & 8 deletions chrome/renderer/security_filter_peer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ SecurityFilterPeer::CreateSecurityFilterPeerForDeniedRequest(
if (content::IsResourceTypeFrame(resource_type))
return CreateSecurityFilterPeerForFrame(std::move(peer), os_error);
// Any other content is entirely filtered-out.
return base::WrapUnique(new ReplaceContentPeer(
std::move(peer), std::string(), std::string()));
return base::MakeUnique<ReplaceContentPeer>(std::move(peer),
std::string(), std::string());
default:
// For other errors, we use our normal error handling.
return peer;
Expand All @@ -68,8 +68,8 @@ SecurityFilterPeer::CreateSecurityFilterPeerForFrame(
"<body style='background-color:#990000;color:white;'>"
"%s</body></html>",
l10n_util::GetStringUTF8(IDS_UNSAFE_FRAME_MESSAGE).c_str());
return base::WrapUnique(
new ReplaceContentPeer(std::move(peer), "text/html", html));
return base::MakeUnique<ReplaceContentPeer>(std::move(peer), "text/html",
html);
}

void SecurityFilterPeer::OnUploadProgress(uint64_t position, uint64_t size) {
Expand Down Expand Up @@ -147,8 +147,8 @@ void BufferedPeer::OnCompletedRequest(int error_code,

original_peer_->OnReceivedResponse(response_info_);
if (!data_.empty()) {
original_peer_->OnReceivedData(base::WrapUnique(
new content::FixedReceivedData(data_.data(), data_.size(), -1, 0)));
original_peer_->OnReceivedData(base::MakeUnique<content::FixedReceivedData>(
data_.data(), data_.size(), -1, 0));
}
original_peer_->OnCompletedRequest(error_code, was_ignored_by_handler,
stale_copy_in_cache, completion_time,
Expand Down Expand Up @@ -187,8 +187,8 @@ void ReplaceContentPeer::OnCompletedRequest(
info.content_length = static_cast<int>(data_.size());
original_peer_->OnReceivedResponse(info);
if (!data_.empty()) {
original_peer_->OnReceivedData(base::WrapUnique(
new content::FixedReceivedData(data_.data(), data_.size(), -1, 0)));
original_peer_->OnReceivedData(base::MakeUnique<content::FixedReceivedData>(
data_.data(), data_.size(), -1, 0));
}
original_peer_->OnCompletedRequest(net::OK, false, stale_copy_in_cache,
completion_time, total_transfer_size);
Expand Down
5 changes: 3 additions & 2 deletions chrome/service/service_process.cc
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,9 @@ bool ServiceProcess::Initialize(base::MessageLoopForUI* message_loop,

ipc_server_.reset(new ServiceIPCServer(this /* client */, io_task_runner(),
&shutdown_event_));
ipc_server_->AddMessageHandler(base::WrapUnique(
new cloud_print::CloudPrintMessageHandler(ipc_server_.get(), this)));
ipc_server_->AddMessageHandler(
base::MakeUnique<cloud_print::CloudPrintMessageHandler>(ipc_server_.get(),
this));
ipc_server_->Init();

// After the IPC server has started we signal that the service process is
Expand Down
6 changes: 3 additions & 3 deletions chrome/service/service_process_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ std::string ServiceProcessPrefs::GetString(

void ServiceProcessPrefs::SetString(const std::string& key,
const std::string& value) {
prefs_->SetValue(key, base::WrapUnique(new base::StringValue(value)),
prefs_->SetValue(key, base::MakeUnique<base::StringValue>(value),
WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
}

Expand All @@ -55,7 +55,7 @@ bool ServiceProcessPrefs::GetBoolean(const std::string& key,
}

void ServiceProcessPrefs::SetBoolean(const std::string& key, bool value) {
prefs_->SetValue(key, base::WrapUnique(new base::FundamentalValue(value)),
prefs_->SetValue(key, base::MakeUnique<base::FundamentalValue>(value),
WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
}

Expand All @@ -70,7 +70,7 @@ int ServiceProcessPrefs::GetInt(const std::string& key,
}

void ServiceProcessPrefs::SetInt(const std::string& key, int value) {
prefs_->SetValue(key, base::WrapUnique(new base::FundamentalValue(value)),
prefs_->SetValue(key, base::MakeUnique<base::FundamentalValue>(value),
WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
}

Expand Down
5 changes: 3 additions & 2 deletions chrome/test/base/chrome_render_view_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,9 @@ void ChromeRenderViewTest::InitChromeContentRendererClient(
new ChromeExtensionsDispatcherDelegate());
ChromeExtensionsRendererClient* ext_client =
ChromeExtensionsRendererClient::GetInstance();
ext_client->SetExtensionDispatcherForTest(base::WrapUnique(
new extensions::Dispatcher(extension_dispatcher_delegate_.get())));
ext_client->SetExtensionDispatcherForTest(
base::MakeUnique<extensions::Dispatcher>(
extension_dispatcher_delegate_.get()));
#endif
#if defined(ENABLE_SPELLCHECK)
client->SetSpellcheck(new SpellCheck());
Expand Down
2 changes: 1 addition & 1 deletion chrome/test/base/test_browser_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ std::unique_ptr<Browser> CreateBrowserWithTestWindowForParams(
TestBrowserWindow* window = new TestBrowserWindow;
new TestBrowserWindowOwner(window);
params->window = window;
return base::WrapUnique(new Browser(*params));
return base::MakeUnique<Browser>(*params);
}

} // namespace chrome
Expand Down
20 changes: 10 additions & 10 deletions chrome/test/base/testing_profile.cc
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,10 @@ class TestExtensionURLRequestContextGetter

std::unique_ptr<KeyedService> BuildHistoryService(
content::BrowserContext* context) {
return base::WrapUnique(new history::HistoryService(
base::WrapUnique(new ChromeHistoryClient(
BookmarkModelFactory::GetForBrowserContext(context))),
base::WrapUnique(new history::ContentVisitDelegate(context))));
return base::MakeUnique<history::HistoryService>(
base::MakeUnique<ChromeHistoryClient>(
BookmarkModelFactory::GetForBrowserContext(context)),
base::MakeUnique<history::ContentVisitDelegate>(context));
}

std::unique_ptr<KeyedService> BuildInMemoryURLIndex(
Expand All @@ -223,8 +223,8 @@ std::unique_ptr<KeyedService> BuildBookmarkModel(
content::BrowserContext* context) {
Profile* profile = Profile::FromBrowserContext(context);
std::unique_ptr<BookmarkModel> bookmark_model(
new BookmarkModel(base::WrapUnique(new ChromeBookmarkClient(
profile, ManagedBookmarkServiceFactory::GetForProfile(profile)))));
new BookmarkModel(base::MakeUnique<ChromeBookmarkClient>(
profile, ManagedBookmarkServiceFactory::GetForProfile(profile))));
bookmark_model->Load(profile->GetPrefs(), profile->GetPath(),
profile->GetIOTaskRunner(),
content::BrowserThread::GetTaskRunnerForThread(
Expand All @@ -241,12 +241,12 @@ void TestProfileErrorCallback(WebDataServiceWrapper::ErrorType error_type,
std::unique_ptr<KeyedService> BuildWebDataService(
content::BrowserContext* context) {
const base::FilePath& context_path = context->GetPath();
return base::WrapUnique(new WebDataServiceWrapper(
return base::MakeUnique<WebDataServiceWrapper>(
context_path, g_browser_process->GetApplicationLocale(),
BrowserThread::GetTaskRunnerForThread(BrowserThread::UI),
BrowserThread::GetTaskRunnerForThread(BrowserThread::DB),
sync_start_util::GetFlareForSyncableService(context_path),
&TestProfileErrorCallback));
&TestProfileErrorCallback);
}

#if BUILDFLAG(ANDROID_JAVA_UI)
Expand Down Expand Up @@ -666,9 +666,9 @@ base::FilePath TestingProfile::GetPath() const {

std::unique_ptr<content::ZoomLevelDelegate>
TestingProfile::CreateZoomLevelDelegate(const base::FilePath& partition_path) {
return base::WrapUnique(new ChromeZoomLevelPrefs(
return base::MakeUnique<ChromeZoomLevelPrefs>(
GetPrefs(), GetPath(), partition_path,
zoom::ZoomEventManager::GetForBrowserContext(this)->GetWeakPtr()));
zoom::ZoomEventManager::GetForBrowserContext(this)->GetWeakPtr());
}

scoped_refptr<base::SequencedTaskRunner> TestingProfile::GetIOTaskRunner() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -998,7 +998,7 @@ class MockDevToolsEventListener : public DevToolsEventListener {

std::unique_ptr<SyncWebSocket> CreateMockSyncWebSocket6(
std::list<std::string>* messages) {
return base::WrapUnique(new MockSyncWebSocket6(messages));
return base::MakeUnique<MockSyncWebSocket6>(messages);
}

} // namespace
Expand Down
2 changes: 1 addition & 1 deletion chrome/test/chromedriver/commands.cc
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ void ExecuteSessionCommand(
namespace internal {

void CreateSessionOnSessionThreadForTesting(const std::string& id) {
SetThreadLocalSession(base::WrapUnique(new Session(id)));
SetThreadLocalSession(base::MakeUnique<Session>(id));
}

} // namespace internal
5 changes: 3 additions & 2 deletions chrome/test/chromedriver/net/url_request_context_getter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ net::URLRequestContext* URLRequestContextGetter::GetURLRequestContext() {
// net::HttpServer fails to parse headers if user-agent header is blank.
builder.set_user_agent("chromedriver");
builder.DisableHttpCache();
builder.set_proxy_config_service(base::WrapUnique(
new net::ProxyConfigServiceFixed(net::ProxyConfig::CreateDirect())));
builder.set_proxy_config_service(
base::MakeUnique<net::ProxyConfigServiceFixed>(
net::ProxyConfig::CreateDirect()));
url_request_context_ = builder.Build();
}
return url_request_context_.get();
Expand Down
4 changes: 2 additions & 2 deletions chrome/utility/importer/edge_database_reader_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,6 @@ EdgeDatabaseReader::OpenTableEnumerator(const base::string16& table_name) {
nullptr, 0, JET_bitTableReadOnly, &table_id)))
return nullptr;

return base::WrapUnique(
new EdgeDatabaseTableEnumerator(table_name, session_id_, table_id));
return base::MakeUnique<EdgeDatabaseTableEnumerator>(table_name, session_id_,
table_id);
}
Loading

0 comments on commit 0c39556

Please sign in to comment.