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.

  BUG=637812

Review-Url: https://codereview.chromium.org/2258713003
Cr-Commit-Position: refs/heads/master@{#418167}
  • Loading branch information
ricea authored and Commit bot committed Sep 13, 2016
1 parent b37dbc7 commit ec7c399
Show file tree
Hide file tree
Showing 28 changed files with 128 additions and 139 deletions.
8 changes: 4 additions & 4 deletions base/containers/mru_cache_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,10 @@ TEST(MRUCacheTest, AutoEvict) {
Cache cache(kMaxSize);

static const int kItem1Key = 1, kItem2Key = 2, kItem3Key = 3, kItem4Key = 4;
cache.Put(kItem1Key, WrapUnique(new CachedItem(20)));
cache.Put(kItem2Key, WrapUnique(new CachedItem(21)));
cache.Put(kItem3Key, WrapUnique(new CachedItem(22)));
cache.Put(kItem4Key, WrapUnique(new CachedItem(23)));
cache.Put(kItem1Key, MakeUnique<CachedItem>(20));
cache.Put(kItem2Key, MakeUnique<CachedItem>(21));
cache.Put(kItem3Key, MakeUnique<CachedItem>(22));
cache.Put(kItem4Key, MakeUnique<CachedItem>(23));

// The cache should only have kMaxSize items in it even though we inserted
// more.
Expand Down
7 changes: 3 additions & 4 deletions base/debug/activity_analyzer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,9 @@ std::unique_ptr<GlobalActivityAnalyzer> GlobalActivityAnalyzer::CreateWithFile(
if (!FilePersistentMemoryAllocator::IsFileAcceptable(*mmfile, true))
return nullptr;

return WrapUnique(new GlobalActivityAnalyzer(
WrapUnique(new FilePersistentMemoryAllocator(
std::move(mmfile), 0, 0, base::StringPiece(),
true))));
return WrapUnique(
new GlobalActivityAnalyzer(MakeUnique<FilePersistentMemoryAllocator>(
std::move(mmfile), 0, 0, base::StringPiece(), true)));
}
#endif // !defined(OS_NACL)

Expand Down
7 changes: 3 additions & 4 deletions base/debug/activity_analyzer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class ActivityAnalyzerTest : public testing::Test {

std::unique_ptr<ThreadActivityTracker> CreateActivityTracker() {
std::unique_ptr<char[]> memory(new char[kStackSize]);
return WrapUnique(new TestActivityTracker(std::move(memory), kStackSize));
return MakeUnique<TestActivityTracker>(std::move(memory), kStackSize);
}

static void DoNothing() {}
Expand Down Expand Up @@ -142,9 +142,8 @@ TEST_F(ActivityAnalyzerTest, GlobalAnalyzerConstruction) {

PersistentMemoryAllocator* allocator =
GlobalActivityTracker::Get()->allocator();
GlobalActivityAnalyzer analyzer(WrapUnique(
new PersistentMemoryAllocator(const_cast<void*>(allocator->data()),
allocator->size(), 0, 0, "", true)));
GlobalActivityAnalyzer analyzer(MakeUnique<PersistentMemoryAllocator>(
const_cast<void*>(allocator->data()), allocator->size(), 0, 0, "", true));

// The only thread at thois point is the test thread.
ThreadActivityAnalyzer* ta1 = analyzer.GetFirstAnalyzer();
Expand Down
7 changes: 3 additions & 4 deletions base/debug/activity_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,8 @@ void GlobalActivityTracker::CreateWithFile(const FilePath& file_path,
{0, static_cast<int64_t>(size)},
MemoryMappedFile::READ_WRITE_EXTEND);
DCHECK(success);
CreateWithAllocator(WrapUnique(new FilePersistentMemoryAllocator(
std::move(mapped_file), size, id, name, false)),
CreateWithAllocator(MakeUnique<FilePersistentMemoryAllocator>(
std::move(mapped_file), size, id, name, false),
stack_depth);
}
#endif // !defined(OS_NACL)
Expand All @@ -493,8 +493,7 @@ void GlobalActivityTracker::CreateWithLocalMemory(size_t size,
StringPiece name,
int stack_depth) {
CreateWithAllocator(
WrapUnique(new LocalPersistentMemoryAllocator(size, id, name)),
stack_depth);
MakeUnique<LocalPersistentMemoryAllocator>(size, id, name), stack_depth);
}

ThreadActivityTracker* GlobalActivityTracker::CreateTrackerForCurrentThread() {
Expand Down
2 changes: 1 addition & 1 deletion base/debug/activity_tracker_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class ActivityTrackerTest : public testing::Test {

std::unique_ptr<ThreadActivityTracker> CreateActivityTracker() {
std::unique_ptr<char[]> memory(new char[kStackSize]);
return WrapUnique(new TestActivityTracker(std::move(memory), kStackSize));
return MakeUnique<TestActivityTracker>(std::move(memory), kStackSize);
}

size_t GetGlobalActiveTrackerCount() {
Expand Down
8 changes: 4 additions & 4 deletions base/files/important_file_writer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ TEST_F(ImportantFileWriterTest, Basic) {
ImportantFileWriter writer(file_, ThreadTaskRunnerHandle::Get());
EXPECT_FALSE(PathExists(writer.path()));
EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState());
writer.WriteNow(WrapUnique(new std::string("foo")));
writer.WriteNow(MakeUnique<std::string>("foo"));
RunLoop().RunUntilIdle();

EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState());
Expand All @@ -117,7 +117,7 @@ TEST_F(ImportantFileWriterTest, BasicWithSuccessfulWriteObserver) {
EXPECT_FALSE(PathExists(writer.path()));
EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState());
successful_write_observer_.ObserveNextSuccessfulWrite(&writer);
writer.WriteNow(WrapUnique(new std::string("foo")));
writer.WriteNow(MakeUnique<std::string>("foo"));
RunLoop().RunUntilIdle();

// Confirm that the observer is invoked.
Expand All @@ -128,7 +128,7 @@ TEST_F(ImportantFileWriterTest, BasicWithSuccessfulWriteObserver) {
// Confirm that re-installing the observer works for another write.
EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState());
successful_write_observer_.ObserveNextSuccessfulWrite(&writer);
writer.WriteNow(WrapUnique(new std::string("bar")));
writer.WriteNow(MakeUnique<std::string>("bar"));
RunLoop().RunUntilIdle();

EXPECT_TRUE(successful_write_observer_.GetAndResetObservationState());
Expand All @@ -138,7 +138,7 @@ TEST_F(ImportantFileWriterTest, BasicWithSuccessfulWriteObserver) {
// Confirm that writing again without re-installing the observer doesn't
// result in a notification.
EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState());
writer.WriteNow(WrapUnique(new std::string("baz")));
writer.WriteNow(MakeUnique<std::string>("baz"));
RunLoop().RunUntilIdle();

EXPECT_FALSE(successful_write_observer_.GetAndResetObservationState());
Expand Down
6 changes: 3 additions & 3 deletions base/json/json_writer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ TEST(JSONWriterTest, NestedTypes) {
std::unique_ptr<DictionaryValue> inner_dict(new DictionaryValue());
inner_dict->SetInteger("inner int", 10);
list->Append(std::move(inner_dict));
list->Append(WrapUnique(new ListValue()));
list->Append(MakeUnique<ListValue>());
list->AppendBoolean(true);
root_dict.Set("list", std::move(list));

Expand Down Expand Up @@ -119,9 +119,9 @@ TEST(JSONWriterTest, BinaryValues) {

ListValue binary_list;
binary_list.Append(BinaryValue::CreateWithCopiedBuffer("asdf", 4));
binary_list.Append(WrapUnique(new FundamentalValue(5)));
binary_list.Append(MakeUnique<FundamentalValue>(5));
binary_list.Append(BinaryValue::CreateWithCopiedBuffer("asdf", 4));
binary_list.Append(WrapUnique(new FundamentalValue(2)));
binary_list.Append(MakeUnique<FundamentalValue>(2));
binary_list.Append(BinaryValue::CreateWithCopiedBuffer("asdf", 4));
EXPECT_FALSE(JSONWriter::Write(binary_list, &output_js));
EXPECT_TRUE(JSONWriter::WriteWithOptions(
Expand Down
3 changes: 1 addition & 2 deletions base/message_loop/message_pump_perftest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ class ScheduleWorkTest : public testing::Test {
max_batch_times_.reset(new base::TimeDelta[num_scheduling_threads]);

for (int i = 0; i < num_scheduling_threads; ++i) {
scheduling_threads.push_back(
WrapUnique(new Thread("posting thread")));
scheduling_threads.push_back(MakeUnique<Thread>("posting thread"));
scheduling_threads[i]->Start();
}

Expand Down
3 changes: 1 addition & 2 deletions base/metrics/histogram.cc
Original file line number Diff line number Diff line change
Expand Up @@ -752,8 +752,7 @@ class LinearHistogram::Factory : public Histogram::Factory {

std::unique_ptr<HistogramBase> HeapAlloc(
const BucketRanges* ranges) override {
return WrapUnique(
new LinearHistogram(name_, minimum_, maximum_, ranges));
return WrapUnique(new LinearHistogram(name_, minimum_, maximum_, ranges));
}

void FillHistogram(HistogramBase* base_histogram) override {
Expand Down
28 changes: 14 additions & 14 deletions base/metrics/persistent_histogram_allocator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ PersistentSparseHistogramDataManager::GetSampleMapRecordsWhileLocked(
return found->second.get();

std::unique_ptr<PersistentSampleMapRecords>& samples = sample_records_[id];
samples = WrapUnique(new PersistentSampleMapRecords(this, id));
samples = MakeUnique<PersistentSampleMapRecords>(this, id);
return samples.get();
}

Expand Down Expand Up @@ -670,9 +670,9 @@ void GlobalHistogramAllocator::CreateWithPersistentMemory(
size_t page_size,
uint64_t id,
StringPiece name) {
Set(WrapUnique(new GlobalHistogramAllocator(
WrapUnique(new PersistentMemoryAllocator(
base, size, page_size, id, name, false)))));
Set(WrapUnique(
new GlobalHistogramAllocator(MakeUnique<PersistentMemoryAllocator>(
base, size, page_size, id, name, false))));
}

// static
Expand All @@ -681,7 +681,7 @@ void GlobalHistogramAllocator::CreateWithLocalMemory(
uint64_t id,
StringPiece name) {
Set(WrapUnique(new GlobalHistogramAllocator(
WrapUnique(new LocalPersistentMemoryAllocator(size, id, name)))));
MakeUnique<LocalPersistentMemoryAllocator>(size, id, name))));
}

#if !defined(OS_NACL)
Expand Down Expand Up @@ -709,9 +709,9 @@ void GlobalHistogramAllocator::CreateWithFile(
return;
}

Set(WrapUnique(new GlobalHistogramAllocator(
WrapUnique(new FilePersistentMemoryAllocator(
std::move(mmfile), size, id, name, false)))));
Set(WrapUnique(
new GlobalHistogramAllocator(MakeUnique<FilePersistentMemoryAllocator>(
std::move(mmfile), size, id, name, false))));
}
#endif

Expand All @@ -728,9 +728,9 @@ void GlobalHistogramAllocator::CreateWithSharedMemory(
}

DCHECK_LE(memory->mapped_size(), size);
Set(WrapUnique(new GlobalHistogramAllocator(
WrapUnique(new SharedPersistentMemoryAllocator(
std::move(memory), 0, StringPiece(), /*readonly=*/false)))));
Set(WrapUnique(
new GlobalHistogramAllocator(MakeUnique<SharedPersistentMemoryAllocator>(
std::move(memory), 0, StringPiece(), /*readonly=*/false))));
}

// static
Expand All @@ -745,9 +745,9 @@ void GlobalHistogramAllocator::CreateWithSharedMemoryHandle(
return;
}

Set(WrapUnique(new GlobalHistogramAllocator(
WrapUnique(new SharedPersistentMemoryAllocator(
std::move(shm), 0, StringPiece(), /*readonly=*/false)))));
Set(WrapUnique(
new GlobalHistogramAllocator(MakeUnique<SharedPersistentMemoryAllocator>(
std::move(shm), 0, StringPiece(), /*readonly=*/false))));
}

// static
Expand Down
10 changes: 4 additions & 6 deletions base/metrics/persistent_histogram_allocator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,8 @@ TEST_F(PersistentHistogramAllocatorTest, CreateAndIterateTest) {

// Create a second allocator and have it access the memory of the first.
std::unique_ptr<HistogramBase> recovered;
PersistentHistogramAllocator recovery(
WrapUnique(new PersistentMemoryAllocator(
allocator_memory_.get(), kAllocatorMemorySize, 0, 0, "", false)));
PersistentHistogramAllocator recovery(MakeUnique<PersistentMemoryAllocator>(
allocator_memory_.get(), kAllocatorMemorySize, 0, 0, "", false));
PersistentHistogramAllocator::Iterator histogram_iter(&recovery);

recovered = histogram_iter.GetNext();
Expand Down Expand Up @@ -181,9 +180,8 @@ TEST_F(PersistentHistogramAllocatorTest, StatisticsRecorderTest) {

// Create a second allocator and have it access the memory of the first.
std::unique_ptr<HistogramBase> recovered;
PersistentHistogramAllocator recovery(
WrapUnique(new PersistentMemoryAllocator(
allocator_memory_.get(), kAllocatorMemorySize, 0, 0, "", false)));
PersistentHistogramAllocator recovery(MakeUnique<PersistentMemoryAllocator>(
allocator_memory_.get(), kAllocatorMemorySize, 0, 0, "", false));
PersistentHistogramAllocator::Iterator histogram_iter(&recovery);

recovered = histogram_iter.GetNext();
Expand Down
10 changes: 5 additions & 5 deletions base/metrics/persistent_sample_map_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,16 @@ namespace {

std::unique_ptr<PersistentHistogramAllocator> CreateHistogramAllocator(
size_t bytes) {
return WrapUnique(new PersistentHistogramAllocator(
WrapUnique(new LocalPersistentMemoryAllocator(bytes, 0, ""))));
return MakeUnique<PersistentHistogramAllocator>(
MakeUnique<LocalPersistentMemoryAllocator>(bytes, 0, ""));
}

std::unique_ptr<PersistentHistogramAllocator> DuplicateHistogramAllocator(
PersistentHistogramAllocator* original) {
return WrapUnique(
new PersistentHistogramAllocator(WrapUnique(new PersistentMemoryAllocator(
return MakeUnique<PersistentHistogramAllocator>(
MakeUnique<PersistentMemoryAllocator>(
const_cast<void*>(original->data()), original->length(), 0,
original->Id(), original->Name(), false))));
original->Id(), original->Name(), false));
}

TEST(PersistentSampleMapTest, AccumulateTest) {
Expand Down
16 changes: 8 additions & 8 deletions base/task_scheduler/priority_queue_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,27 +58,27 @@ class ThreadBeginningTransaction : public SimpleThread {
TEST(TaskSchedulerPriorityQueueTest, PushPopPeek) {
// Create test sequences.
scoped_refptr<Sequence> sequence_a(new Sequence);
sequence_a->PushTask(WrapUnique(new Task(
sequence_a->PushTask(MakeUnique<Task>(
FROM_HERE, Closure(),
TaskTraits().WithPriority(TaskPriority::USER_VISIBLE), TimeDelta())));
TaskTraits().WithPriority(TaskPriority::USER_VISIBLE), TimeDelta()));
SequenceSortKey sort_key_a = sequence_a->GetSortKey();

scoped_refptr<Sequence> sequence_b(new Sequence);
sequence_b->PushTask(WrapUnique(new Task(
sequence_b->PushTask(MakeUnique<Task>(
FROM_HERE, Closure(),
TaskTraits().WithPriority(TaskPriority::USER_BLOCKING), TimeDelta())));
TaskTraits().WithPriority(TaskPriority::USER_BLOCKING), TimeDelta()));
SequenceSortKey sort_key_b = sequence_b->GetSortKey();

scoped_refptr<Sequence> sequence_c(new Sequence);
sequence_c->PushTask(WrapUnique(new Task(
sequence_c->PushTask(MakeUnique<Task>(
FROM_HERE, Closure(),
TaskTraits().WithPriority(TaskPriority::USER_BLOCKING), TimeDelta())));
TaskTraits().WithPriority(TaskPriority::USER_BLOCKING), TimeDelta()));
SequenceSortKey sort_key_c = sequence_c->GetSortKey();

scoped_refptr<Sequence> sequence_d(new Sequence);
sequence_d->PushTask(WrapUnique(new Task(
sequence_d->PushTask(MakeUnique<Task>(
FROM_HERE, Closure(), TaskTraits().WithPriority(TaskPriority::BACKGROUND),
TimeDelta())));
TimeDelta()));
SequenceSortKey sort_key_d = sequence_d->GetSortKey();

// Create a PriorityQueue and a Transaction.
Expand Down
10 changes: 4 additions & 6 deletions base/task_scheduler/scheduler_service_thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,10 @@ SchedulerServiceThread::~SchedulerServiceThread() = default;
// static
std::unique_ptr<SchedulerServiceThread> SchedulerServiceThread::Create(
TaskTracker* task_tracker, DelayedTaskManager* delayed_task_manager) {
std::unique_ptr<SchedulerWorker> worker =
SchedulerWorker::Create(
ThreadPriority::NORMAL,
WrapUnique(new ServiceThreadDelegate(delayed_task_manager)),
task_tracker,
SchedulerWorker::InitialState::ALIVE);
std::unique_ptr<SchedulerWorker> worker = SchedulerWorker::Create(
ThreadPriority::NORMAL,
MakeUnique<ServiceThreadDelegate>(delayed_task_manager), task_tracker,
SchedulerWorker::InitialState::ALIVE);
if (!worker)
return nullptr;

Expand Down
6 changes: 3 additions & 3 deletions base/task_scheduler/scheduler_worker_pool_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class SchedulerParallelTaskRunner : public TaskRunner {
TimeDelta delay) override {
// Post the task as part of a one-off single-task Sequence.
return worker_pool_->PostTaskWithSequence(
WrapUnique(new Task(from_here, closure, traits_, delay)),
MakeUnique<Task>(from_here, closure, traits_, delay),
make_scoped_refptr(new Sequence), nullptr);
}

Expand Down Expand Up @@ -643,9 +643,9 @@ bool SchedulerWorkerPoolImpl::Initialize(

for (size_t i = 0; i < max_threads; ++i) {
std::unique_ptr<SchedulerWorker> worker = SchedulerWorker::Create(
priority_hint, WrapUnique(new SchedulerWorkerDelegateImpl(
priority_hint, MakeUnique<SchedulerWorkerDelegateImpl>(
this, re_enqueue_sequence_callback,
&shared_priority_queue_, static_cast<int>(i))),
&shared_priority_queue_, static_cast<int>(i)),
task_tracker_, i == 0 ? SchedulerWorker::InitialState::ALIVE
: SchedulerWorker::InitialState::DETACHED);
if (!worker)
Expand Down
Loading

0 comments on commit ec7c399

Please sign in to comment.