Fix thread safety issues with //components/upload_list.

components/upload_list/upload_list.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/upload_list/upload_list.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <iterator>
@@ skipping 27 matching lines @@
     : delegate_(delegate),
       upload_log_path_(upload_log_path),
       worker_pool_(worker_pool) {}
 
 UploadList::~UploadList() {}
 
 void UploadList::LoadUploadListAsynchronously() {
   DCHECK(thread_checker_.CalledOnValidThread());
   worker_pool_->PostTask(
       FROM_HERE,
-      base::Bind(&UploadList::LoadUploadListAndInformDelegateOfCompletion,
+      base::Bind(&UploadList::PerformLoadAndNotifyDelegate,
                  this, base::ThreadTaskRunnerHandle::Get()));
 }
 
 void UploadList::ClearDelegate() {
   DCHECK(thread_checker_.CalledOnValidThread());
   delegate_ = NULL;
 }
 
-void UploadList::LoadUploadListAndInformDelegateOfCompletion(
+void UploadList::PerformLoadAndNotifyDelegate(
     const scoped_refptr<base::SequencedTaskRunner>& task_runner) {
-  LoadUploadList();
+  std::vector<UploadInfo> uploads;
+  LoadUploadList(&uploads);
   task_runner->PostTask(
       FROM_HERE,
-      base::Bind(&UploadList::InformDelegateOfCompletion, this));
+      base::Bind(&UploadList::SetUploadsAndNotifyDelegate, this,
+                 std::move(uploads)));

[Mark Mentovai, 2016/03/25 18:26:56]

#include <utility>

[Robert Sesek, 2016/03/25 18:45:56]

Done.

 }
 
-void UploadList::LoadUploadList() {
+void UploadList::LoadUploadList(std::vector<UploadInfo>* uploads) {
   if (base::PathExists(upload_log_path_)) {
     std::string contents;
     base::ReadFileToString(upload_log_path_, &contents);
     std::vector<std::string> log_entries = base::SplitString(
         contents, base::kWhitespaceASCII, base::KEEP_WHITESPACE,
         base::SPLIT_WANT_NONEMPTY);
-    ClearUploads();
-    ParseLogEntries(log_entries);
+    ParseLogEntries(log_entries, uploads);
   }
 }
 
-void UploadList::AppendUploadInfo(const UploadInfo& info) {
-  uploads_.push_back(info);
-}
-
-void UploadList::ClearUploads() {
-  uploads_.clear();
-}
-
 void UploadList::ParseLogEntries(
-    const std::vector<std::string>& log_entries) {
+    const std::vector<std::string>& log_entries,
+    std::vector<UploadInfo>* uploads) {
   std::vector<std::string>::const_reverse_iterator i;
   for (i = log_entries.rbegin(); i != log_entries.rend(); ++i) {
     std::vector<std::string> components = base::SplitString(
         *i, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
     // Skip any blank (or corrupted) lines.
     if (components.size() < 2 || components.size() > 4)
       continue;
     base::Time upload_time;
     double seconds_since_epoch;
     if (!components[0].empty()) {
       if (!base::StringToDouble(components[0], &seconds_since_epoch))
         continue;
       upload_time = base::Time::FromDoubleT(seconds_since_epoch);
     }
     UploadInfo info(components[1], upload_time);
 
     // Add local ID if present.
     if (components.size() > 2)
       info.local_id = components[2];
 
     // Add capture time if present.
     if (components.size() > 3 &&
         !components[3].empty() &&
         base::StringToDouble(components[3], &seconds_since_epoch)) {
       info.capture_time = base::Time::FromDoubleT(seconds_since_epoch);
     }
 
-    uploads_.push_back(info);
+    uploads->push_back(info);
   }
 }
 
-void UploadList::InformDelegateOfCompletion() {
+void UploadList::SetUploadsAndNotifyDelegate(std::vector<UploadInfo> uploads) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  uploads_ = std::move(uploads);
   if (delegate_)
     delegate_->OnUploadListAvailable();
 }
 
-void UploadList::GetUploads(unsigned int max_count,
+void UploadList::GetUploads(size_t max_count,

[Mark Mentovai, 2016/03/25 18:26:56]

This max_count thing seems ill-advised, certainly with the Crashpad
implementation, but whatever.

[Robert Sesek, 2016/03/25 18:45:56]

Right. There's some more cleanup I'd like to do here, but this is a good first
cut.

                             std::vector<UploadInfo>* uploads) {
   DCHECK(thread_checker_.CalledOnValidThread());
   std::copy(uploads_.begin(),
             uploads_.begin() + std::min<size_t>(uploads_.size(), max_count),

[Mark Mentovai, 2016/03/25 18:26:56]

The explicit <size_t> is no longer necessary!

[Robert Sesek, 2016/03/25 18:45:56]

Done.

             std::back_inserter(*uploads));
 }