indexeddb: Removed use of dangling ptr in writeBlobToFileOnIOThread.

content/browser/indexed_db/indexed_db_backing_store.h

 // Copyright (c) 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.
 
 #ifndef CONTENT_BROWSER_INDEXED_DB_INDEXED_DB_BACKING_STORE_H_
 #define CONTENT_BROWSER_INDEXED_DB_INDEXED_DB_BACKING_STORE_H_
 
 #include <map>
 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/sequenced_task_runner.h"

[michaeln, 2014/07/24 00:20:23]

would a fwrd decl work in here?

[cmumford, 2014/07/24 18:18:26]

If I change task_runner() to return a SequencedTaskRunner then we can forward
declare type.

 #include "base/strings/string_piece.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "content/browser/indexed_db/indexed_db.h"
 #include "content/browser/indexed_db/indexed_db_active_blob_registry.h"
 #include "content/browser/indexed_db/indexed_db_blob_info.h"
 #include "content/browser/indexed_db/indexed_db_leveldb_coding.h"
 #include "content/browser/indexed_db/indexed_db_metadata.h"
 #include "content/browser/indexed_db/leveldb/leveldb_iterator.h"
 #include "content/browser/indexed_db/leveldb/leveldb_transaction.h"
 #include "content/common/content_export.h"
 #include "content/common/indexed_db/indexed_db_key.h"
 #include "content/common/indexed_db/indexed_db_key_path.h"
 #include "content/common/indexed_db/indexed_db_key_range.h"
 #include "third_party/leveldatabase/src/include/leveldb/status.h"
 #include "url/gurl.h"
 #include "webkit/browser/blob/blob_data_handle.h"
 
-namespace base {
-class TaskRunner;
-}
-
 namespace fileapi {
 class FileWriterDelegate;
 }
 
 namespace net {
 class URLRequestContext;
 }
 
 namespace content {
 
@@ skipping 26 matching lines @@
   }
 
   static scoped_refptr<IndexedDBBackingStore> Open(
       IndexedDBFactory* indexed_db_factory,
       const GURL& origin_url,
       const base::FilePath& path_base,
       net::URLRequestContext* request_context,
       blink::WebIDBDataLoss* data_loss,
       std::string* data_loss_message,
       bool* disk_full,
-      base::TaskRunner* task_runner,
+      base::SequencedTaskRunner* task_runner,
       bool clean_journal,
       leveldb::Status* status);
   static scoped_refptr<IndexedDBBackingStore> Open(
       IndexedDBFactory* indexed_db_factory,
       const GURL& origin_url,
       const base::FilePath& path_base,
       net::URLRequestContext* request_context,
       blink::WebIDBDataLoss* data_loss,
       std::string* data_loss_message,
       bool* disk_full,
       LevelDBFactory* leveldb_factory,
-      base::TaskRunner* task_runner,
+      base::SequencedTaskRunner* task_runner,
       bool clean_journal,
       leveldb::Status* status);
   static scoped_refptr<IndexedDBBackingStore> OpenInMemory(
       const GURL& origin_url,
-      base::TaskRunner* task_runner,
+      base::SequencedTaskRunner* task_runner,
       leveldb::Status* status);
   static scoped_refptr<IndexedDBBackingStore> OpenInMemory(
       const GURL& origin_url,
       LevelDBFactory* leveldb_factory,
-      base::TaskRunner* task_runner,
+      base::SequencedTaskRunner* task_runner,
       leveldb::Status* status);
 
   void GrantChildProcessPermissions(int child_process_id);
 
   // Compact is public for testing.
   virtual void Compact();
   virtual std::vector<base::string16> GetDatabaseNames(leveldb::Status*);
   virtual leveldb::Status GetIDBDatabaseMetaData(
       const base::string16& name,
       IndexedDBDatabaseMetadata* metadata,
@@ skipping 49 matching lines @@
     std::string primary_key_;
     int64 version_;
     DISALLOW_COPY_AND_ASSIGN(RecordIdentifier);
   };
 
   class BlobWriteCallback : public base::RefCounted<BlobWriteCallback> {
    public:
     virtual void Run(bool succeeded) = 0;
 
    protected:
+    friend class base::RefCounted<BlobWriteCallback>;
     virtual ~BlobWriteCallback() {}
-    friend class base::RefCounted<BlobWriteCallback>;
   };
 
   virtual leveldb::Status GetRecord(
       IndexedDBBackingStore::Transaction* transaction,
       int64 database_id,
       int64 object_store_id,
       const IndexedDBKey& key,
       IndexedDBValue* record) WARN_UNUSED_RESULT;
   virtual leveldb::Status PutRecord(
       IndexedDBBackingStore::Transaction* transaction,
@@ skipping 262 matching lines @@
 
      private:
       bool is_file_;
       GURL url_;
       base::FilePath file_path_;
       int64_t key_;
       int64_t size_;
       base::Time last_modified_;
     };
 
-    class ChainedBlobWriter : public base::RefCounted<ChainedBlobWriter> {
+    class ChainedBlobWriter
+        : public base::RefCountedThreadSafe<ChainedBlobWriter> {
      public:
       virtual void set_delegate(
           scoped_ptr<fileapi::FileWriterDelegate> delegate) = 0;
 
       // TODO(ericu): Add a reason in the event of failure.
       virtual void ReportWriteCompletion(bool succeeded,
                                          int64 bytes_written) = 0;
 
       virtual void Abort() = 0;
 
      protected:
+      friend class base::RefCountedThreadSafe<ChainedBlobWriter>;
       virtual ~ChainedBlobWriter() {}
-      friend class base::RefCounted<ChainedBlobWriter>;
     };
 
     class ChainedBlobWriterImpl;
 
     typedef std::vector<WriteDescriptor> WriteDescriptorVec;
 
    private:
     class BlobWriteCallbackWrapper;
 
     leveldb::Status HandleBlobPreTransaction(
@@ skipping 10 matching lines @@
     IndexedDBBackingStore* backing_store_;
     scoped_refptr<LevelDBTransaction> transaction_;
     BlobChangeMap blob_change_map_;
     BlobChangeMap incognito_blob_map_;
     int64 database_id_;
     BlobJournalType blobs_to_remove_;
     scoped_refptr<ChainedBlobWriter> chained_blob_writer_;
   };
 
  protected:
+  friend class base::RefCounted<IndexedDBBackingStore>;
   IndexedDBBackingStore(IndexedDBFactory* indexed_db_factory,
                         const GURL& origin_url,
                         const base::FilePath& blob_path,
                         net::URLRequestContext* request_context,
                         scoped_ptr<LevelDBDatabase> db,
                         scoped_ptr<LevelDBComparator> comparator,
-                        base::TaskRunner* task_runner);
+                        base::SequencedTaskRunner* task_runner);
   virtual ~IndexedDBBackingStore();
-  friend class base::RefCounted<IndexedDBBackingStore>;
 
   bool is_incognito() const { return !indexed_db_factory_; }
 
   leveldb::Status SetUpMetadata();
 
   virtual bool WriteBlobFile(
       int64 database_id,
       const Transaction::WriteDescriptor& descriptor,
       Transaction::ChainedBlobWriter* chained_blob_writer);
   virtual bool RemoveBlobFile(int64 database_id, int64 key);
   virtual void StartJournalCleaningTimer();
   void CleanPrimaryJournalIgnoreReturn();
 
  private:
   static scoped_refptr<IndexedDBBackingStore> Create(
       IndexedDBFactory* indexed_db_factory,
       const GURL& origin_url,
       const base::FilePath& blob_path,
       net::URLRequestContext* request_context,
       scoped_ptr<LevelDBDatabase> db,
       scoped_ptr<LevelDBComparator> comparator,
-      base::TaskRunner* task_runner,
+      base::SequencedTaskRunner* task_runner,
       leveldb::Status* status);
 
   static bool ReadCorruptionInfo(const base::FilePath& path_base,
                                  const GURL& origin_url,
                                  std::string* message);
 
   leveldb::Status FindKeyInIndex(
       IndexedDBBackingStore::Transaction* transaction,
       int64 database_id,
       int64 object_store_id,
@@ skipping 14 matching lines @@
 
   // The origin identifier is a key prefix unique to the origin used in the
   // leveldb backing store to partition data by origin. It is a normalized
   // version of the origin URL with a versioning suffix appended, e.g.
   // "http_localhost_81@1" Since only one origin is stored per backing store
   // this is redundant but necessary for backwards compatibility; the suffix
   // provides for future flexibility.
   const std::string origin_identifier_;
 
   net::URLRequestContext* request_context_;
-  base::TaskRunner* task_runner_;
+  base::SequencedTaskRunner* task_runner_;

[michaeln, 2014/07/24 00:20:24]

huh... i'm surprised this is not a scoped_refptr<T>?

[cmumford, 2014/07/24 18:18:26]

Not in indexed_db_context_impl.h either.

   std::set<int> child_process_ids_granted_;
   BlobChangeMap incognito_blob_map_;
   base::OneShotTimer<IndexedDBBackingStore> journal_cleaning_timer_;
 
   scoped_ptr<LevelDBDatabase> db_;
   scoped_ptr<LevelDBComparator> comparator_;
   // Whenever blobs are registered in active_blob_registry_, indexed_db_factory_
   // will hold a reference to this backing store.
   IndexedDBActiveBlobRegistry active_blob_registry_;
   base::OneShotTimer<IndexedDBBackingStore> close_timer_;
 
   DISALLOW_COPY_AND_ASSIGN(IndexedDBBackingStore);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_INDEXED_DB_INDEXED_DB_BACKING_STORE_H_