Mojo: Don't hold a watch context ref in SimpleWatcher posted task

mojo/public/cpp/system/simple_watcher.cc

 // Copyright 2017 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 "mojo/public/cpp/system/simple_watcher.h"
 
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/synchronization/lock.h"
 #include "base/trace_event/heap_profiler.h"
 #include "mojo/public/c/system/watcher.h"
 
 namespace mojo {
 
 // Thread-safe Context object used to dispatch watch notifications from a
 // arbitrary threads.
 class SimpleWatcher::Context : public base::RefCountedThreadSafe<Context> {
  public:
   // Creates a |Context| instance for a new watch on |watcher|, to watch
   // |handle| for |signals|.
   static scoped_refptr<Context> Create(
       base::WeakPtr<SimpleWatcher> watcher,
       scoped_refptr<base::SingleThreadTaskRunner> task_runner,
       WatcherHandle watcher_handle,
       Handle handle,
       MojoHandleSignals signals,
+      int watch_id,
       MojoResult* watch_result) {
-    scoped_refptr<Context> context = new Context(watcher, task_runner);
+    scoped_refptr<Context> context =
+        new Context(watcher, task_runner, watch_id);
 
     // If MojoWatch succeeds, it assumes ownership of a reference to |context|.
     // In that case, this reference is balanced in CallNotify() when |result| is
     // |MOJO_RESULT_CANCELLED|.
     context->AddRef();
 
     *watch_result = MojoWatch(watcher_handle.value(), handle.value(), signals,
                               context->value());
     if (*watch_result != MOJO_RESULT_OK) {
       // Balanced by the AddRef() above since watching failed.
@@ skipping 21 matching lines @@
 
   void DisableCancellationNotifications() {
     base::AutoLock lock(lock_);
     enable_cancellation_notifications_ = false;
   }
 
  private:
   friend class base::RefCountedThreadSafe<Context>;
 
   Context(base::WeakPtr<SimpleWatcher> weak_watcher,
-          scoped_refptr<base::SingleThreadTaskRunner> task_runner)
-      : weak_watcher_(weak_watcher), task_runner_(task_runner) {}
+          scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+          int watch_id)
+      : weak_watcher_(weak_watcher),
+        task_runner_(task_runner),
+        watch_id_(watch_id) {}
   ~Context() {}
 
   void Notify(MojoResult result,
               MojoHandleSignalsState signals_state,
               MojoWatcherNotificationFlags flags) {
     if (result == MOJO_RESULT_CANCELLED) {
       // The SimpleWatcher may have explicitly cancelled this watch, so we don't
       // bother dispatching the notification - it would be ignored anyway.
       //
       // TODO(rockot): This shouldn't really be necessary, but there are already
       // instances today where bindings object may be bound and subsequently
       // closed due to pipe error, all before the thread's TaskRunner has been
       // properly initialized.
       base::AutoLock lock(lock_);
       if (!enable_cancellation_notifications_)
         return;
     }
 
     if ((flags & MOJO_WATCHER_NOTIFICATION_FLAG_FROM_SYSTEM) &&
         task_runner_->RunsTasksOnCurrentThread() && weak_watcher_ &&
         weak_watcher_->is_default_task_runner_) {
       // System notifications will trigger from the task runner passed to
       // mojo::edk::InitIPCSupport(). In Chrome this happens to always be the
       // default task runner for the IO thread.
-      weak_watcher_->OnHandleReady(make_scoped_refptr(this), result);
+      weak_watcher_->OnHandleReady(watch_id_, result);
     } else {
       task_runner_->PostTask(
           FROM_HERE, base::Bind(&SimpleWatcher::OnHandleReady, weak_watcher_,
-                                make_scoped_refptr(this), result));
+                                watch_id_, result));
     }
   }
 
   const base::WeakPtr<SimpleWatcher> weak_watcher_;
   const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+  const int watch_id_;
 
   base::Lock lock_;
   bool enable_cancellation_notifications_ = true;
 
   DISALLOW_COPY_AND_ASSIGN(Context);
 };
 
 SimpleWatcher::SimpleWatcher(const tracked_objects::Location& from_here,
                              ArmingPolicy arming_policy,
                              scoped_refptr<base::SingleThreadTaskRunner> runner)
@@ skipping 20 matching lines @@
 
 MojoResult SimpleWatcher::Watch(Handle handle,
                                 MojoHandleSignals signals,
                                 const ReadyCallback& callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!IsWatching());
   DCHECK(!callback.is_null());
 
   callback_ = callback;
   handle_ = handle;
+  watch_id_ += 1;
 
   MojoResult watch_result = MOJO_RESULT_UNKNOWN;
-  context_ =
-      Context::Create(weak_factory_.GetWeakPtr(), task_runner_,
-                      watcher_handle_.get(), handle_, signals, &watch_result);
+  context_ = Context::Create(weak_factory_.GetWeakPtr(), task_runner_,
+                             watcher_handle_.get(), handle_, signals, watch_id_,
+                             &watch_result);
   if (!context_) {
     handle_.set_value(kInvalidHandleValue);
     callback_.Reset();
     DCHECK_EQ(MOJO_RESULT_INVALID_ARGUMENT, watch_result);
     return watch_result;
   }
 
   if (arming_policy_ == ArmingPolicy::AUTOMATIC)
     ArmOrNotify();
 
@@ skipping 57 matching lines @@
     return;
 
   MojoResult ready_result;
   MojoResult rv = Arm(&ready_result);
   if (rv == MOJO_RESULT_OK)
     return;
 
   DCHECK_EQ(MOJO_RESULT_FAILED_PRECONDITION, rv);
   task_runner_->PostTask(FROM_HERE, base::Bind(&SimpleWatcher::OnHandleReady,
                                                weak_factory_.GetWeakPtr(),
-                                               context_, ready_result));
+                                               watch_id_, ready_result));
 }
 
-void SimpleWatcher::OnHandleReady(scoped_refptr<const Context> context,
-                                  MojoResult result) {
+void SimpleWatcher::OnHandleReady(int watch_id, MojoResult result) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // This notification may be for a previously watched context, in which case
   // we just ignore it.
-  if (context != context_)
+  if (watch_id != watch_id_)
     return;
 
   ReadyCallback callback = callback_;
   if (result == MOJO_RESULT_CANCELLED) {
     // Implicit cancellation due to someone closing the watched handle. We clear
     // the SimppleWatcher's state before dispatching this.
     context_ = nullptr;
     handle_.set_value(kInvalidHandleValue);
     callback_.Reset();
   }
@@ skipping 14 matching lines @@
     // at most once in AUTOMATIC arming mode.
     if (result == MOJO_RESULT_FAILED_PRECONDITION)
       unsatisfiable_ = true;
 
     if (arming_policy_ == ArmingPolicy::AUTOMATIC && IsWatching())
       ArmOrNotify();
   }
 }
 
 }  // namespace mojo