Add a non-blocking "OneShotEvent" class

extensions/common/async_event.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 EXTENSIONS_COMMON_ASYNC_EVENT_H_
+#define EXTENSIONS_COMMON_ASYNC_EVENT_H_
+
+#include <vector>
+
+#include "base/callback_forward.h"
+#include "base/logging.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "base/threading/thread_checker.h"
+
+namespace base {
+class TaskRunner;
+}
+
+namespace tracked_objects {
+class Location;
+}
+
+namespace extensions {
+
+// This class represents an event that's expected to happen once.  It
+// allows clients to guarantee that code is run after the event has
+// happened.  If the AsyncEvent is destroyed before it happens, the
+// delayed closures are destroyed without being run.
+//
+// This class is similar to a WaitableEvent combined with several
+// WaitableEventWatchers, but using it is simpler.

[Matt Perry, 2013/05/15 21:08:54]

Maybe there's a way to unify these concepts somehow. Or at the least, use
similar terminology (e.g. "Signal" instead of "MarkHappened", etc)?

[Jeffrey Yasskin, 2013/05/15 22:46:50]

Signal is easy enough. With "Latch" as the class, I think "is_open" makes more
sense than "is_signaled"? On the other hand, many functions returning a Latch
read better as ready().is_signaled() than ready().is_open(), so maybe I've
picked another bad class name.

[Matt Perry, 2013/05/15 22:55:40]

As a consumer of the class, I think I'd find it hard to remember whether the
"ready" state was "open" or "closed". I certainly didn't know until I looked at
the code.

[Jeffrey Yasskin, 2013/05/15 23:17:53]

Renamed to is_signaled(), and renamed the class a second time to match. :)

+//
+// This class is not thread-safe, and must be used from a single thread.
+class AsyncEvent : public base::SupportsWeakPtr<AsyncEvent> {

[Matt Perry, 2013/05/15 21:08:54]

Can you change the class name? And I don't like what you were going to change it
to, either, so please change it again after that.

[Jeffrey Yasskin, 2013/05/15 22:46:50]

Changed. We'll see if you want something else. ;)

+ public:
+  AsyncEvent();
+  ~AsyncEvent();
+
+  // True if MarkHappened has been called.
+  bool has_happened() const {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    return happened_;
+  }
+
+  // Causes has_happened() to return true and all queued tasks to be
+  // run in an arbitrary order.  This method must only be called once.
+  void MarkHappened();
+
+  // Scheduled |task| to be called on |runner| after has_happened()
+  // becomes true.  Inside |task|, if this AsyncEvent is still alive,
+  // CHECK(has_happened()) will never fail (which implies that
+  // AsyncEvent::Reset() doesn't exist).
+  //
+  // If |*this| is destroyed before happening, none of these tasks
+  // will be executed.
+  //
+  // Omitting the |runner| argument indicates that |task| should run
+  // on MessageLoopProxy::current().
+  //
+  // Tasks may be run in an arbitrary order, not just FIFO.  Tasks
+  // will never be called on the current thread before this function
+  // returns.
+  //
+  // Const because RunAfter doesn't modify the logical state of this
+  // object (which is just the happened_ bit).
+  void RunAfter(const tracked_objects::Location& from_here,

[Matt Perry, 2013/05/15 21:08:54]

Is from_here really useful? Seems that most callers will only have a single
entry point. I would just set FROM_HERE to the AsyncEvent code.

[Jeffrey Yasskin, 2013/05/15 22:46:50]

I think you're saying that any function passed to RunAfter is likely to be used
in only one place, so people trying to debug code can just look for all
references to that function instead of needing to grab the from_here
information.

Does that apply more to this function than to general TaskRunner tasks? I've
definitely used the Location to debug tasks on TaskRunners.

[Matt Perry, 2013/05/15 22:55:40]

Yes, I think it's less likely to be useful for this class, because the event
that triggered the callback is more interesting than where the callback was
registered.

[Jeffrey Yasskin, 2013/05/15 23:17:53]

I managed to convince Matt that the from_here was worthwhile by pointing out
that since the tasks are enqueued on a TaskRunner instead of being run from
MarkHappened, you lose the call stack describing the triggering event.

+                const base::Closure& task) const;
+  void RunAfter(const tracked_objects::Location& from_here,
+                const base::Closure& task,
+                const scoped_refptr<base::TaskRunner>& runner) const;
+
+ private:
+  struct TaskInfo;
+
+  base::ThreadChecker thread_checker_;
+
+  bool happened_;
+
+  // The task list is mutable because it's not part of the logical
+  // state of the object.  This lets us return const references to the
+  // AsyncEvent to clients that just want to run tasks through it
+  // without worrying that they'll mark the event as having happened.
+  //
+  // Optimization note: We could reduce the size of this class to a

[Matt Perry, 2013/05/15 21:08:54]

There won't be enough of these objects for that to be worth it, right?

[Jeffrey Yasskin, 2013/05/15 22:46:50]

There might be, but there aren't yet, which is why I haven't done it.

+  // single pointer by storing |happened_| in the low bit of a
+  // pointer, and storing the size and capacity of the array (if any)
+  // on the far end of the pointer.
+  mutable std::vector<TaskInfo> tasks_;
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_COMMON_ASYNC_EVENT_H_