Blob storage: Fix ChromeBlobStorageContext deleter to use DeleteOnIOThread

The original one-off deleter for ChromeBlobStorageContext attempts to
delete |this| directly when not called on the IO thread and the IO
thread isn't available.  This causes problems with objects it owns
since some objects require being deleted on the IO thread.

This manifested in flaky tests like EmptyBlob and Bug84933Test which
started failing in a flaky way when IndexedDBDispatcherHost (which holds
a scoped_refptr<ChromeBlobStorageContext>) was moved from the IO thread
to the IDB task runner.  The flaky crashes looked like:

[8496:100:0221/084948.427:FATAL:memory_dump_manager.cc(247)] Check failed:
(*mdp_iter)->task_runner &&
(*mdp_iter)->task_runner->RunsTasksInCurrentSequence().
MemoryDumpProvider "BlobStorageContext" attempted to unregister itself
in a racy way. Please file a crbug.
Backtrace:
	base::debug::CollectStackTrace [0x00007FF6F62CCAA2+18]
	base::debug::StackTrace::StackTrace [0x00007FF6F62CC002+18]
	logging::LogMessage::~LogMessage [0x00007FF6F62E7885+101]
	base::trace_event::MemoryDumpManager::UnregisterDumpProviderInternal [0x00007FF6F634BC2B+827]
	storage::BlobStorageContext::~BlobStorageContext [0x00007FF6F61D7362+34]
	storage::BlobStorageContext::`scalar deleting destructor' [0x00007FF6F61DA8F0+16]
	content::ChromeBlobStorageContext::`scalar deleting destructor' [0x00007FF6F46EF4F8+40]
	content::IndexedDBDispatcherHost::~IndexedDBDispatcherHost [0x00007FF6F48FC6BE+382]
	content::IndexedDBDispatcherHost::`scalar deleting destructor' [0x00007FF6F48FDFA0+16]

Remove the one-off deleter for ChromeBlobStorageContext in favor of
DeleteOnIOThread which always tries to delete the object on the IO
thread and, if the IO thread isn't available, gives up since that
indicates a leak on shutdown, which is generally okay in circumstances
like this.

Bug: 717812, 934243, 934250
Change-Id: Ibfda4b0b8f17d3eeb95773bb2a75f0e9a16e84b5
Reviewed-on: https://chromium-review.googlesource.com/c/1481597Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Chase Phillips <cmp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634328}

content/browser/blob_storage/chrome_blob_storage_context.cc

@@ -243,16 +243,7 @@ blink::mojom::BlobPtr ChromeBlobStorageContext::GetBlobPtr(
   return blob_ptr;
 }
 
-ChromeBlobStorageContext::~ChromeBlobStorageContext() {}
-
-void ChromeBlobStorageContext::DeleteOnCorrectThread() const {
-  if (BrowserThread::IsThreadInitialized(BrowserThread::IO) &&
-      !BrowserThread::CurrentlyOn(BrowserThread::IO)) {
-    BrowserThread::DeleteSoon(BrowserThread::IO, FROM_HERE, this);
-    return;
-  }
-  delete this;
-}
+ChromeBlobStorageContext::~ChromeBlobStorageContext() = default;
 
 storage::BlobStorageContext* GetBlobStorageContext(
     ChromeBlobStorageContext* blob_storage_context) {

content/browser/blob_storage/chrome_blob_storage_context.h

@@ -15,6 +15,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/sequenced_task_runner_helpers.h"
 #include "content/common/content_export.h"
+#include "content/public/browser/browser_thread.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "storage/browser/blob/blob_data_handle.h"
 #include "third_party/blink/public/mojom/blob/blob_url_store.mojom.h"
@@ -34,7 +35,6 @@ class BlobStorageContext;
 namespace content {
 class BlobHandle;
 class BrowserContext;
-struct ChromeBlobStorageContextDeleter;
 class ResourceContext;
 
 // A context class that keeps track of BlobStorageController used by the chrome.
@@ -46,7 +46,7 @@ class ResourceContext;
 // the IO thread (unless specifically called out in doc comments).
 class CONTENT_EXPORT ChromeBlobStorageContext
     : public base::RefCountedThreadSafe<ChromeBlobStorageContext,
-                                        ChromeBlobStorageContextDeleter> {
+                                        BrowserThread::DeleteOnIOThread> {
  public:
   ChromeBlobStorageContext();
 
@@ -95,22 +95,12 @@ class CONTENT_EXPORT ChromeBlobStorageContext
   virtual ~ChromeBlobStorageContext();
 
  private:
+  friend struct BrowserThread::DeleteOnThread<BrowserThread::IO>;
   friend class base::DeleteHelper<ChromeBlobStorageContext>;
-  friend class base::RefCountedThreadSafe<ChromeBlobStorageContext,
-                                          ChromeBlobStorageContextDeleter>;
-  friend struct ChromeBlobStorageContextDeleter;
-
-  void DeleteOnCorrectThread() const;
 
   std::unique_ptr<storage::BlobStorageContext> context_;
 };
 
-struct ChromeBlobStorageContextDeleter {
-  static void Destruct(const ChromeBlobStorageContext* context) {
-    context->DeleteOnCorrectThread();
-  }
-};
-
 // Returns the BlobStorageContext associated with the
 // ChromeBlobStorageContext instance passed in.
 storage::BlobStorageContext* GetBlobStorageContext(

content/browser/indexed_db/indexed_db_browsertest.cc

@@ -286,8 +286,7 @@ IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, DoesntHangTest) {
   SimpleTest(GetTestUrl("indexeddb", "transaction_not_blocked.html"));
 }
 
-// TODO(crbug.com/934250) The test is flaky (crashes) on trybots.
-IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, DISABLED_Bug84933Test) {
+IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, Bug84933Test) {
   const GURL url = GetTestUrl("indexeddb", "bug_84933.html");
 
   // Just navigate to the URL. Test will crash if it fails.
@@ -519,13 +518,7 @@ IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, CanDeleteWhenOverQuotaTest) {
   SimpleTest(GetTestUrl("indexeddb", "delete_over_quota.html"));
 }
 
-#if defined(OS_WIN)
-#define MAYBE_EmptyBlob DISABLED_EmptyBlob
-#else
-#define MAYBE_EmptyBlob EmptyBlob
-#endif
-// TODO(crbug.com/934243) The test is flaky on Windows.
-IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, MAYBE_EmptyBlob) {
+IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, EmptyBlob) {
   // First delete all IDB's for the test origin
   DeleteForOrigin(kFileOrigin);
   EXPECT_EQ(0, RequestBlobFileCount(kFileOrigin));  // Start with no blob files.