[Background Fetch] Cleanup/fix thread safety

content/browser/background_fetch/background_fetch_request_info.cc

Index: content/browser/background_fetch/background_fetch_request_info.cc
diff --git a/content/browser/background_fetch/background_fetch_request_info.cc b/content/browser/background_fetch/background_fetch_request_info.cc
index 1973d359ab0e35a4f4a655a87077c7412ef99c93..0f23af18e1a2b20e914440634a9d971b37101a7b 100644
--- a/content/browser/background_fetch/background_fetch_request_info.cc
+++ b/content/browser/background_fetch/background_fetch_request_info.cc
@@ -7,6 +7,8 @@
 #include <string>
 
 #include "base/strings/string_util.h"
+#include "base/threading/sequenced_task_runner_handle.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/download_item.h"
 #include "net/http/http_response_headers.h"
 
@@ -15,13 +17,19 @@ namespace content {
 BackgroundFetchRequestInfo::BackgroundFetchRequestInfo(
     int request_index,
     const ServiceWorkerFetchRequest& fetch_request)
-    : request_index_(request_index), fetch_request_(fetch_request) {}
+    : RefCountedDeleteOnSequence<BackgroundFetchRequestInfo>(
+          base::SequencedTaskRunnerHandle::Get()),

[Peter Beverloo, 2017/07/10 13:13:21]

Should we be more explicit and bind to
BrowserThread::GetTaskRunnerForThread(BrowserThread::IO)?

[johnme, 2017/07/10 13:41:07]

No, we don't actually care what thread we're deleted on, we just care that it's
the same thread as the one that we're accessed on.

The sequence_checker_ is automatically bound to the current sequence in the
constructor since I don't call DETACH_FROM_SEQUENCE, so it's cleanest to do the
same thing for RefCountedDeleteOnSequence.

+      request_index_(request_index),
+      fetch_request_(fetch_request) {}
 
-BackgroundFetchRequestInfo::~BackgroundFetchRequestInfo() {}
+BackgroundFetchRequestInfo::~BackgroundFetchRequestInfo() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
 
-void BackgroundFetchRequestInfo::PopulateDownloadState(
+void BackgroundFetchRequestInfo::PopulateDownloadStateOnUI(
     DownloadItem* download_item,
     DownloadInterruptReason download_interrupt_reason) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(!download_state_populated_);
 
   download_guid_ = download_item->GetGuid();
@@ -41,54 +49,68 @@ void BackgroundFetchRequestInfo::PopulateDownloadState(
     while (headers->EnumerateHeaderLines(&iter, &name, &value))
       response_headers_[base::ToLowerASCII(name)] = value;
   }
+}
 
+void BackgroundFetchRequestInfo::SetDownloadStatePopulated() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

[Peter Beverloo, 2017/07/10 13:13:21]

ugh. noise. so glad that we'll be able to get rid of all of this once the
ClientProxy thing is in place.

[johnme, 2017/07/10 13:41:07]

Acknowledged.

   download_state_populated_ = true;
 }
 
-void BackgroundFetchRequestInfo::PopulateResponseFromDownloadItem(
+void BackgroundFetchRequestInfo::PopulateResponseFromDownloadItemOnUI(
     DownloadItem* download_item) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(!response_data_populated_);
 
   url_chain_ = download_item->GetUrlChain();
   file_path_ = download_item->GetTargetFilePath();
   file_size_ = download_item->GetReceivedBytes();
   response_time_ = download_item->GetEndTime();
+}
 
+void BackgroundFetchRequestInfo::SetResponseDataPopulated() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   response_data_populated_ = true;
 }
 
 int BackgroundFetchRequestInfo::GetResponseCode() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(download_state_populated_);
   return response_code_;
 }
 
 const std::string& BackgroundFetchRequestInfo::GetResponseText() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(download_state_populated_);
   return response_text_;
 }
 
 const std::map<std::string, std::string>&
 BackgroundFetchRequestInfo::GetResponseHeaders() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(download_state_populated_);
   return response_headers_;
 }
 
 const std::vector<GURL>& BackgroundFetchRequestInfo::GetURLChain() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(response_data_populated_);
   return url_chain_;
 }
 
 const base::FilePath& BackgroundFetchRequestInfo::GetFilePath() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(response_data_populated_);
   return file_path_;
 }
 
 int64_t BackgroundFetchRequestInfo::GetFileSize() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(response_data_populated_);
   return file_size_;
 }
 
 const base::Time& BackgroundFetchRequestInfo::GetResponseTime() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(response_data_populated_);
   return response_time_;
 }