Cancel loading when body stream reader is detached.

content/child/shared_memory_data_consumer_handle.cc

 // Copyright 2015 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.
 
 #include "content/child/shared_memory_data_consumer_handle.h"
 
 #include <algorithm>
 #include <deque>
 
 #include "base/bind.h"
@@ skipping 31 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(DelegateThreadSafeReceivedData);
 };
 
 }  // namespace
 
 using Result = blink::WebDataConsumerHandle::Result;
 
 class SharedMemoryDataConsumerHandle::Context final
     : public base::RefCountedThreadSafe<Context> {
  public:
-  Context()
+  explicit Context(const base::Closure& on_clear)
       : result_(Ok),
         first_offset_(0),
         client_(nullptr),
+        writer_task_runner_(base::MessageLoop::current()->task_runner()),
+        on_clear_(on_clear),
+        is_on_clear_valid_(!on_clear_.is_null()),
         is_handle_active_(true),
         is_two_phase_read_in_progress_(false) {}
 
   bool IsEmpty() const { return queue_.empty(); }
   void ClearIfNecessary() {
     if (!is_handle_locked() && !is_handle_active()) {
       // No one is interested in the contents.
+      if (is_on_clear_valid_) {
+        // We post a task even in the writer thread in order to keep reentrancy.

[hiroshige, 2015/06/22 18:23:16]

how about adding "because didGetReadable() can destruct the reader, which causes
ClearIfNecessary(), and calling |on_clear_| can cause didGetReadable() call" or
so (this explanation is somehow specific to CancelBodyStreaming() though).

nit optional: s/keep/avoid/? (not sure though)

[yhirano, 2015/06/23 05:28:37]

Done.

+        writer_task_runner_->PostTask(FROM_HERE, on_clear_);
+      }
       Clear();
     }
   }
   void ClearQueue() {
     for (auto& data : queue_) {
       delete data;
     }
     queue_.clear();
     first_offset_ = 0;
   }
@@ skipping 25 matching lines @@
     auto runner = notification_task_runner_;
     if (!runner)
       return;
     // We don't re-post the task when the runner changes while waiting for
     // this task because in this case a new reader is obtained and
     // notification is already done at the reader creation time if necessary.
     runner->PostTask(FROM_HERE,
                      base::Bind(&Context::NotifyInternal, this, false));
   }
   void Notify() { NotifyInternal(true); }
+  void ResetOnClear() {
+    if (on_clear_.is_null()) {
+      DCHECK(!is_on_clear_valid_);
+      return;
+    }
+    is_on_clear_valid_ = false;
+    if (writer_task_runner_->BelongsToCurrentThread()) {
+      // We can reset the closure immediately.
+      on_clear_.Reset();
+    } else {
+      // We need to reset |on_clear_| on the right thread because it might
+      // lead to the object destruction.
+      writer_task_runner_->PostTask(FROM_HERE,
+                                    base::Bind(&Context::ResetOnClear, this));
+    }
+  }
   bool is_handle_locked() const { return notification_task_runner_; }
   bool IsReaderBoundToCurrentThread() const {
     return notification_task_runner_ &&
            notification_task_runner_->BelongsToCurrentThread();
   }
   bool is_handle_active() const { return is_handle_active_; }
   void set_is_handle_active(bool b) { is_handle_active_ = b; }
   void Consume(size_t s) {
     first_offset_ += s;
     auto top = Top();
@@ skipping 34 matching lines @@
                        base::Bind(&Context::NotifyInternal, this, false));
     }
   }
   void Clear() {
     for (auto& data : queue_) {
       delete data;
     }
     queue_.clear();
     first_offset_ = 0;
     client_ = nullptr;
+    // Note this doesn't work in the destructor if |on_clear_| is not null. We

[hiroshige, 2015/06/22 18:23:16]

This comment might be better to be placed just before ResetOnClear() definition.

[yhirano, 2015/06/23 05:28:37]

Done.

+    // have an assert in the destructor.
+    ResetOnClear();
   }
 
   friend class base::RefCountedThreadSafe<Context>;
   ~Context() {
+    DCHECK(on_clear_.is_null());
+
     // This is necessary because the queue stores raw pointers.
     Clear();
   }
 
   base::Lock lock_;
   // |result_| stores the ultimate state of this handle if it has. Otherwise,
   // |Ok| is set.
   Result result_;
   // TODO(yhirano): Use std::deque<scoped_ptr<ThreadSafeReceivedData>>
   // once it is allowed.
   std::deque<RequestPeer::ThreadSafeReceivedData*> queue_;
   size_t first_offset_;
   Client* client_;
   scoped_refptr<base::SingleThreadTaskRunner> notification_task_runner_;
+  scoped_refptr<base::SingleThreadTaskRunner> writer_task_runner_;
+  base::Closure on_clear_;
+  // We need this boolean variable to remember if |on_clear_| is callable
+  // because we need to set |on_clear_| only on the writer thread and hence

[hiroshige, 2015/06/22 18:23:16]

optional: how about "... because |on_clear_| is reset on the writer thread
asynchronously and hence ..."?

[yhirano, 2015/06/23 05:28:38]

I think it misleading the callback will be reset synchronously when reset on the
writer thread.

+  // |on_clear_.is_null()| is untrustworthy.
+  bool is_on_clear_valid_;

[hiroshige, 2015/06/22 18:23:16]

Please add a comment that |on_clear_| and |is_on_clear_valid_| is protected by
|lock_|. (except for in the destructor?)

[yhirano, 2015/06/23 05:28:37]

Other variables are also protected by the lock, so I don't see a reason to say
that only for these variables.

   bool is_handle_active_;
   bool is_two_phase_read_in_progress_;
 
   DISALLOW_COPY_AND_ASSIGN(Context);
 };
 
 SharedMemoryDataConsumerHandle::Writer::Writer(
     const scoped_refptr<Context>& context,
     BackpressureMode mode)
     : context_(context), mode_(mode) {
 }
 
 SharedMemoryDataConsumerHandle::Writer::~Writer() {
   Close();
+  base::AutoLock lock(context_->lock());
+  context_->ResetOnClear();
 }
 
 void SharedMemoryDataConsumerHandle::Writer::AddData(
     scoped_ptr<RequestPeer::ReceivedData> data) {
   if (!data->length()) {
     // We omit empty data.
     return;
   }
 
   bool needs_notification = false;
@@ skipping 23 matching lines @@
   }
 }
 
 void SharedMemoryDataConsumerHandle::Writer::Close() {
   bool needs_notification = false;
 
   {
     base::AutoLock lock(context_->lock());
     if (context_->result() == Ok) {
       context_->set_result(Done);
+      context_->ResetOnClear();
       needs_notification = context_->IsEmpty();
     }
   }
   if (needs_notification) {
     // We cannot issue the notification synchronously because this function can
     // be called in the client's callback.
     context_->PostNotify();
   }
 }
 
 void SharedMemoryDataConsumerHandle::Writer::Fail() {
   bool needs_notification = false;
   {
     base::AutoLock lock(context_->lock());
     if (context_->result() == Ok) {
       // TODO(yhirano): Use an appropriate error code other than
       // UnexpectedError.
       context_->set_result(UnexpectedError);
 
       if (context_->is_two_phase_read_in_progress()) {
         // If we are in two-phase read session, we cannot discard the data. We
         // will clear the queue at the end of the session.
       } else {
         context_->ClearQueue();
       }
 
+      context_->ResetOnClear();
       needs_notification = true;
     }
   }
   if (needs_notification) {
     // We cannot issue the notification synchronously because this function can
     // be called in the client's callback.
     context_->PostNotify();
   }
 }
 
@@ skipping 82 matching lines @@
   } else {
     context_->Consume(read_size);
   }
 
   return Ok;
 }
 
 SharedMemoryDataConsumerHandle::SharedMemoryDataConsumerHandle(
     BackpressureMode mode,
     scoped_ptr<Writer>* writer)
-    : context_(new Context) {
+    : SharedMemoryDataConsumerHandle(mode, base::Closure(), writer) {
+}
+
+SharedMemoryDataConsumerHandle::SharedMemoryDataConsumerHandle(
+    BackpressureMode mode,
+    const base::Closure& on_clear,
+    scoped_ptr<Writer>* writer)
+    : context_(new Context(on_clear)) {
   writer->reset(new Writer(context_, mode));
 }
 
 SharedMemoryDataConsumerHandle::~SharedMemoryDataConsumerHandle() {
   base::AutoLock lock(context_->lock());
   context_->set_is_handle_active(false);
   context_->ClearIfNecessary();
 }
 
 scoped_ptr<blink::WebDataConsumerHandle::Reader>
 SharedMemoryDataConsumerHandle::ObtainReader(Client* client) {
   return make_scoped_ptr(obtainReaderInternal(client));
 }
 
 SharedMemoryDataConsumerHandle::ReaderImpl*
 SharedMemoryDataConsumerHandle::obtainReaderInternal(Client* client) {
   return new ReaderImpl(context_, client);
 }
 
 const char* SharedMemoryDataConsumerHandle::debugName() const {
   return "SharedMemoryDataConsumerHandle";
 }
 
 }  // namespace content