Add a non-blocking "OneShotEvent" class

extensions/common/one_shot_event.cc

Index: extensions/common/one_shot_event.cc
diff --git a/extensions/common/one_shot_event.cc b/extensions/common/one_shot_event.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b3c6913551cdacb1b9302950bce572e2e0b82576
--- /dev/null
+++ b/extensions/common/one_shot_event.cc
@@ -0,0 +1,66 @@
+// 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.
+
+#include "extensions/common/one_shot_event.h"
+
+#include "base/callback.h"
+#include "base/location.h"
+#include "base/message_loop/message_loop_proxy.h"
+#include "base/task_runner.h"
+
+using base::TaskRunner;
+
+namespace extensions {
+
+struct OneShotEvent::TaskInfo {
+  TaskInfo() {}
+  TaskInfo(const tracked_objects::Location& from_here,
+           const scoped_refptr<TaskRunner>& runner,
+           const base::Closure& task)
+      : from_here(from_here), runner(runner), task(task) {
+    CHECK(runner != NULL);  // Detect mistakes with a decent stack frame.

[not at google - send to devlin, 2013/05/16 15:51:08]

just CHECK(runner)?

[Jeffrey Yasskin, 2013/05/16 19:05:36]

I have no idea. Sure.

+  }
+  tracked_objects::Location from_here;
+  scoped_refptr<TaskRunner> runner;
+  base::Closure task;
+};
+
+OneShotEvent::OneShotEvent() : signaled_(false) {}
+OneShotEvent::~OneShotEvent() {}
+
+void OneShotEvent::ThenRun(const tracked_objects::Location& from_here,
+                          const base::Closure& task) const {

[not at google - send to devlin, 2013/05/16 15:51:08]

indentation +1
looks like there are a couple of other places in this file like this, presumably
because you renamed.

[Jeffrey Yasskin, 2013/05/16 19:05:36]

Oops, done.

+  ThenRun(from_here, task, base::MessageLoopProxy::current());
+}
+
+void OneShotEvent::ThenRun(const tracked_objects::Location& from_here,
+                          const base::Closure& task,
+                          const scoped_refptr<TaskRunner>& runner) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  if (is_signaled()) {

[not at google - send to devlin, 2013/05/16 15:51:08]

I actually wonder if this may at some point cause confusion - it's just as
legitimate to call CHECK(!is_signalled) here and I think that this may often be
what is safer for callers.

WDYT about having two separate methods here? One version that CHECKs, the other
that does what you're doing (... and never a version that runs the callback
synchronously, obviously). Naming... well, it's not worth thinking much about
until you think about it, but maybe like AddTask vs AddOrRunTask/AddOrPostTask
(or some variation using the ThenRun theme; ThenRunEventually?).

[Jeffrey Yasskin, 2013/05/16 19:05:36]

We talked about this offline, and agreed on the following:
* We want to encourage uses to avoid caring whether the event has already
happened, so we want the version of this function that has if(is_signaled()) to
have a shorter name.
* There are some valid uses (e.g. icon loading) that need to run inline when the
event is already signaled, and possibly run different code if they're running
inline. A separate method to support them is likely to be useful.
* We don't want to add that separate method until we have a use case or two.
* The name of this method should anticipate that the second method may
eventually exist.

Other names for this method include: AddListener(), Post(), AfterSignaled().
RunAfter(), CallAfter(), etc. aren't great because event.RunAfter(callback)
reads like the event is going to run after the callback, which is backwards.

I'm going to let the names sit until this afternoon, and then poll the team.

[Matt Perry, 2013/05/16 19:12:04]

RunWhenSignaled?

[Jeffrey Yasskin, 2013/05/16 19:14:42]

I dislike that one because it implies the task will _only_ be run at the point
the Event is signaled, when actually, if the Event has already been signaled by
the call to this function, the task will be run at the call to this function,
not when the Signal() happens.

[Jeffrey Yasskin, 2013/05/16 21:36:16]

A quick vote in the extensions room concluded that we'll be going with Post().

+    runner->PostTask(from_here, task);
+  } else {
+    tasks_.push_back(TaskInfo(from_here, runner, task));
+  }
+}
+
+void OneShotEvent::Signal() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  CHECK(!signaled_) << "Only call Signal once.";
+
+  signaled_ = true;
+  // After this point, a call to ThenRun() from one of the queued
+  // tasks could proceed immediately, but the fact that this object is
+  // single-threaded prevents that from being relevant.
+
+  // We could randomize tasks_ in debug mode in order to check that
+  // the order doesn't matter...
+  for (size_t i = 0; i < tasks_.size(); ++i) {
+    tasks_[i].runner->PostTask(tasks_[i].from_here, tasks_[i].task);
+  }
+}
+
+}  // namespace extensions