Background Sync: Switch to thread-safe lifecycle checks.

BackgroundSyncContext's members background_sync_manager_ and services_
are documented to only be accessed on the IO thread. However, the
BackgroundSyncContext destructor accesses them when running their
destructors, even though it may be run on any thread
(BackgroundSyncContext is RefCountedThreadSafe). These accesses are not
threadsafe.

This CL solves the thread safety issues by having the class' destructor
run on the IO thread.

Change-Id: Ib40c94987ee7cdfc58d513561493d4c8155b4b2d
Reviewed-on: https://chromium-review.googlesource.com/1053404
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557644}

content/browser/background_sync/background_sync_context.cc

@@ -15,11 +15,15 @@
 
 namespace content {
 
-BackgroundSyncContext::BackgroundSyncContext() {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-}
+BackgroundSyncContext::BackgroundSyncContext()
+    : base::RefCountedDeleteOnSequence<BackgroundSyncContext>(
+          BrowserThread::GetTaskRunnerForThread(BrowserThread::IO)) {}
 
 BackgroundSyncContext::~BackgroundSyncContext() {
+  // The destructor must run on the IO thread because it implicitly accesses
+  // background_sync_manager_ and services_, when it runs their destructors.
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
   DCHECK(!background_sync_manager_);
   DCHECK(services_.empty());
 }
@@ -36,7 +40,6 @@ void BackgroundSyncContext::Init(
 
 void BackgroundSyncContext::Shutdown() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::BindOnce(&BackgroundSyncContext::ShutdownOnIO, this));
@@ -45,7 +48,6 @@ void BackgroundSyncContext::Shutdown() {
 void BackgroundSyncContext::CreateService(
     blink::mojom::BackgroundSyncServiceRequest request) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::BindOnce(&BackgroundSyncContext::CreateServiceOnIOThread, this,

content/browser/background_sync/background_sync_context.h

@@ -9,7 +9,7 @@
 #include <memory>
 
 #include "base/macros.h"
-#include "base/memory/ref_counted.h"
+#include "base/memory/ref_counted_delete_on_sequence.h"
 #include "content/common/content_export.h"
 #include "third_party/blink/public/platform/modules/background_sync/background_sync.mojom.h"
 
@@ -23,7 +23,7 @@ class ServiceWorkerContextWrapper;
 // processes/origins. Most logic is delegated to the owned BackgroundSyncManager
 // instance, which is only accessed on the IO thread.
 class CONTENT_EXPORT BackgroundSyncContext
-    : public base::RefCountedThreadSafe<BackgroundSyncContext> {
+    : public base::RefCountedDeleteOnSequence<BackgroundSyncContext> {
  public:
   BackgroundSyncContext();
 
@@ -46,7 +46,8 @@ class CONTENT_EXPORT BackgroundSyncContext
   BackgroundSyncManager* background_sync_manager() const;
 
  protected:
-  friend class base::RefCountedThreadSafe<BackgroundSyncContext>;
+  friend class base::RefCountedDeleteOnSequence<BackgroundSyncContext>;
+  friend class base::DeleteHelper<BackgroundSyncContext>;
   virtual ~BackgroundSyncContext();
 
   void set_background_sync_manager_for_testing(