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

Unified Diff: content/child/shared_memory_data_consumer_handle.cc

Issue 1186053004: Cancel loading when body stream reader is detached. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@error-ipc-data-consumer
Patch Set: Created 5 years, 6 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/child/shared_memory_data_consumer_handle.cc
diff --git a/content/child/shared_memory_data_consumer_handle.cc b/content/child/shared_memory_data_consumer_handle.cc
index beb470d96d31e91d05214d1b13b4e1909ae242cc..16cee7be1df6652f3a4469ffb5aa20368c421363 100644
--- a/content/child/shared_memory_data_consumer_handle.cc
+++ b/content/child/shared_memory_data_consumer_handle.cc
@@ -49,10 +49,13 @@ using Result = blink::WebDataConsumerHandle::Result;
class SharedMemoryDataConsumerHandle::Context final
: public base::RefCountedThreadSafe<Context> {
public:
- Context()
+ explicit Context(const base::Closure& on_reader_detached)
: result_(Ok),
first_offset_(0),
client_(nullptr),
+ writer_task_runner_(base::MessageLoop::current()->task_runner()),
kinuko 2015/06/23 07:56:10 please use base::ThreadTaskRunnerHandle::Get() ins
yhirano 2015/06/23 08:17:55 Done, and replaced similar ones.
+ on_reader_detached_(on_reader_detached),
+ is_on_reader_detached_valid_(!on_reader_detached_.is_null()),
is_handle_active_(true),
is_two_phase_read_in_progress_(false) {}
@@ -60,6 +63,12 @@ class SharedMemoryDataConsumerHandle::Context final
void ClearIfNecessary() {
if (!is_handle_locked() && !is_handle_active()) {
// No one is interested in the contents.
+ if (is_on_reader_detached_valid_) {
+ // We post a task even in the writer thread in order to avoid a
+ // reentrance problem as calling |on_reader_detached_| may manipulate
+ // the context synchronously.
+ writer_task_runner_->PostTask(FROM_HERE, on_reader_detached_);
+ }
Clear();
}
}
@@ -105,6 +114,25 @@ class SharedMemoryDataConsumerHandle::Context final
base::Bind(&Context::NotifyInternal, this, false));
}
void Notify() { NotifyInternal(true); }
+ // This function doesn't work in the destructor if |on_reader_detached_| is
+ // not null.
+ void ResetOnReaderDetached() {
+ if (on_reader_detached_.is_null()) {
+ DCHECK(!is_on_reader_detached_valid_);
+ return;
+ }
+ is_on_reader_detached_valid_ = false;
+ if (writer_task_runner_->BelongsToCurrentThread()) {
+ // We can reset the closure immediately.
+ on_reader_detached_.Reset();
+ } else {
+ // We need to reset |on_reader_detached_| on the right thread because it
+ // might lead to the object destruction.
+ writer_task_runner_->PostTask(
+ FROM_HERE,
+ base::Bind(&Context::ResetOnReaderDetachedWithLock, this));
+ }
+ }
bool is_handle_locked() const { return notification_task_runner_; }
bool IsReaderBoundToCurrentThread() const {
return notification_task_runner_ &&
@@ -159,10 +187,19 @@ class SharedMemoryDataConsumerHandle::Context final
queue_.clear();
first_offset_ = 0;
client_ = nullptr;
+ // Note this doesn't work in the destructor if |on_reader_detached_| is not
+ // null. We have an assert in the destructor.
+ ResetOnReaderDetached();
+ }
+ void ResetOnReaderDetachedWithLock() {
+ base::AutoLock lock(lock_);
+ ResetOnReaderDetached();
}
friend class base::RefCountedThreadSafe<Context>;
~Context() {
+ DCHECK(on_reader_detached_.is_null());
+
// This is necessary because the queue stores raw pointers.
Clear();
}
@@ -177,6 +214,13 @@ class SharedMemoryDataConsumerHandle::Context final
size_t first_offset_;
Client* client_;
scoped_refptr<base::SingleThreadTaskRunner> notification_task_runner_;
+ scoped_refptr<base::SingleThreadTaskRunner> writer_task_runner_;
+ base::Closure on_reader_detached_;
+ // We need this boolean variable to remember if |on_reader_detached_| is
+ // callable because we need to reset |on_reader_detached_| only on the writer
+ // thread and hence |on_reader_detached_.is_null()| is untrustworthy on
+ // other threads.
+ bool is_on_reader_detached_valid_;
bool is_handle_active_;
bool is_two_phase_read_in_progress_;
@@ -191,6 +235,8 @@ SharedMemoryDataConsumerHandle::Writer::Writer(
SharedMemoryDataConsumerHandle::Writer::~Writer() {
Close();
+ base::AutoLock lock(context_->lock());
+ context_->ResetOnReaderDetached();
}
void SharedMemoryDataConsumerHandle::Writer::AddData(
@@ -234,6 +280,7 @@ void SharedMemoryDataConsumerHandle::Writer::Close() {
base::AutoLock lock(context_->lock());
if (context_->result() == Ok) {
context_->set_result(Done);
+ context_->ResetOnReaderDetached();
needs_notification = context_->IsEmpty();
}
}
@@ -260,6 +307,7 @@ void SharedMemoryDataConsumerHandle::Writer::Fail() {
context_->ClearQueue();
}
+ context_->ResetOnReaderDetached();
needs_notification = true;
}
}
@@ -362,7 +410,14 @@ Result SharedMemoryDataConsumerHandle::ReaderImpl::endRead(size_t read_size) {
SharedMemoryDataConsumerHandle::SharedMemoryDataConsumerHandle(
BackpressureMode mode,
scoped_ptr<Writer>* writer)
- : context_(new Context) {
+ : SharedMemoryDataConsumerHandle(mode, base::Closure(), writer) {
+}
+
+SharedMemoryDataConsumerHandle::SharedMemoryDataConsumerHandle(
+ BackpressureMode mode,
+ const base::Closure& on_reader_detached,
+ scoped_ptr<Writer>* writer)
+ : context_(new Context(on_reader_detached)) {
writer->reset(new Writer(context_, mode));
}
« no previous file with comments | « content/child/shared_memory_data_consumer_handle.h ('k') | content/child/shared_memory_data_consumer_handle_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698