Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1054)

Unified Diff: content/browser/indexed_db/indexed_db_backing_store.cc

Issue 417573004: indexeddb: Removed use of dangling ptr in writeBlobToFileOnIOThread. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Added period at end of sentence Created 6 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/indexed_db/indexed_db_backing_store.cc
diff --git a/content/browser/indexed_db/indexed_db_backing_store.cc b/content/browser/indexed_db/indexed_db_backing_store.cc
index 9c220f24533b9ba1e3d92283916316cb30f465dd..6e5efdbd7c001cdea140c4d00a800713f09d70ce 100644
--- a/content/browser/indexed_db/indexed_db_backing_store.cc
+++ b/content/browser/indexed_db/indexed_db_backing_store.cc
@@ -2253,7 +2253,7 @@ class IndexedDBBackingStore::Transaction::ChainedBlobWriterImpl
};
class LocalWriteClosure : public FileWriterDelegate::DelegateWriteCallback,
- public base::RefCounted<LocalWriteClosure> {
+ public base::RefCountedThreadSafe<LocalWriteClosure> {
michaeln1 2014/07/23 20:15:15 yikes! good catch
public:
LocalWriteClosure(IndexedDBBackingStore::Transaction::ChainedBlobWriter*
chained_blob_writer,
@@ -2282,6 +2282,14 @@ class LocalWriteClosure : public FileWriterDelegate::DelegateWriteCallback,
write_status == FileWriterDelegate::SUCCESS_COMPLETED));
}
+ static void ReleaseWriterOnIDBThread(
+ scoped_refptr<IndexedDBBackingStore::Transaction::ChainedBlobWriter>
+ chained_blob_writer) {
+ // Don't actually release the writer (the closure will do that). Merely
+ // posting the ref counted pointer over to this thread ensures that it is
+ // deleted on the correct thread.
+ }
+
void writeBlobToFileOnIOThread(const FilePath& file_path,
const GURL& blob_url,
net::URLRequestContext* request_context) {
@@ -2304,15 +2312,23 @@ class LocalWriteClosure : public FileWriterDelegate::DelegateWriteCallback,
}
private:
- virtual ~LocalWriteClosure() {}
- friend class base::RefCounted<LocalWriteClosure>;
+ virtual ~LocalWriteClosure() {
+ // Make sure the ChainedBlobWriter is derefed (and deleted) on the IDB
+ // thread since it owns a transaction which has thread affinity.
+ task_runner_->PostTask(
michaeln1 2014/07/23 20:15:15 probably should use ReleaseSoon here as coded if
+ FROM_HERE,
+ base::Bind(&LocalWriteClosure::ReleaseWriterOnIDBThread,
+ chained_blob_writer_));
+ }
+ friend class base::RefCountedThreadSafe<LocalWriteClosure>;
michaeln1 2014/07/23 20:15:15 style nit: i think we usually put friend decls pri
jsbell 2014/07/23 20:17:47 I thought so to... but oddly enough it's not in th
cmumford 2014/07/23 23:26:58 Done.
void callBlobCallbackOnIDBTaskRunner(bool succeeded) {
michaeln1 2014/07/23 20:15:15 this intermediary may not be needed, at the callsi
cmumford 2014/07/23 23:26:58 You're correct - it's not necessary. It's unrelate
DCHECK(task_runner_->RunsTasksOnCurrentThread());
chained_blob_writer_->ReportWriteCompletion(succeeded, bytes_written_);
}
- IndexedDBBackingStore::Transaction::ChainedBlobWriter* chained_blob_writer_;
+ scoped_refptr<IndexedDBBackingStore::Transaction::ChainedBlobWriter>
+ chained_blob_writer_;
base::TaskRunner* task_runner_;
int64 bytes_written_;

Powered by Google App Engine
This is Rietveld 408576698