Move PostTaskAndReplyWithStatus into task_runner_helpers.h

base/task_runner_helpers.h

+// Copyright (c) 2012 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 BASE_TASK_RUNNER_HELPERS_H_
+#define BASE_TASK_RUNNER_HELPERS_H_
+#pragma once
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/logging.h"
+
+namespace base {
+
+// Helper templates to call methods and reply with the returned value.
+//
+// Typically when you have these methods:
+//   R DoWorkAndReturn();
+//   void Callback(R& result);

[darin (slow to review), 2012/02/29 00:27:48]

why does Callback take a non-const reference?  should it be a const reference
instead?

[battre, 2012/03/07 20:01:48]

Done.

+//
+// You can pass the result of DoWorkAndReturn to the Callback by:
+//
+//  R* result = new R;
+//  message_loop_proxy->PostTaskAndReply(
+//      from_here,
+//      ReturnAsParam<R>(Bind(&DoWorkAndReturn), result),
+//      RelayHelper(Bind(&Callback), Owned(result)));

[satorux1, 2012/02/28 23:58:13]

drive-by: Where is RelayHelper defined? 'git grep' only gave me only this:

% git grep RelayHelper
base/file_util_proxy.cc://      RelayHelper(Bind(&Callback), Owned(result)));

Would be nice to include RealyHelper definition in the comment?

[battre, 2012/03/07 20:01:48]

This was a copy&pasted typo (->ReplyHelper).

Done.

+//
+// Or just use PostTaskAndReplyWithStatus helper template (see the code below).
+//
+//  PostTaskAndReplyWithStatus(
+//      message_loop_proxy, from_here,
+//      Bind(&DoWorkAndReturn),
+//      Bind(&Callback),
+//      new R);
+template <typename R1, typename R2>

[willchan no longer on Chromium, 2012/03/01 02:17:24]

I don't think I like ReturnValueTranslator. Do the translation in the callback
function, why define converters here? I dislike it even more because it looks
like we only get link errors for this, not a compile error. Lame =/

[battre, 2012/03/07 20:01:48]

Done.

+struct ReturnValueTranslator {
+  static R2 Value(const R1& value);
+};
+
+// Template that may be specialized to convert the return value of the
+// DoWorkAndReturn function before being passed to the Callback function.
+template <typename R>
+struct ReturnValueTranslator<R, R> {
+  static R Value(const R& value) { return value; }
+};
+
+template <typename R1, typename R2>
+void ReturnAsParamAdapter(const Callback<R1(void)>& func, R2* result) {
+  if (!func.is_null())

[willchan no longer on Chromium, 2012/03/01 02:17:24]

I don't think we should allow func to be NULL. Why not DCHECK?

[battre, 2012/03/07 20:01:48]

Some unit tests rely on this (see the failed tests for Patch Set 2 of this CL.)
We may change them.

+    *result = ReturnValueTranslator<R1, R2>::Value(func.Run());

[willchan no longer on Chromium, 2012/03/01 02:17:24]

I think this should just be *result = fun.Run(); and only have one template
parameter.

[battre, 2012/03/07 20:01:48]

Done.

+}
+
+template <typename R1, typename R2>
+Closure ReturnAsParam(const Callback<R1(void)>& func, R2* result) {
+  DCHECK(result);
+  return Bind(&ReturnAsParamAdapter<R1, R2>, func, result);
+}
+
+template <typename R, typename A1>
+void ReturnAsParamAdapter1(const Callback<R(A1)>& func, A1 a1, R* result) {

[willchan no longer on Chromium, 2012/03/01 02:17:24]

Do we need pump to generate multiple variants of this?

[battre, 2012/03/07 20:01:48]

No, we can achieve the same effect by using Bind. I have deleted this.

Done.

+  if (!func.is_null())
+    *result = func.Run(a1);
+}
+
+template <typename R, typename A1>
+Closure ReturnAsParam(const Callback<R(A1)>& func, A1 a1, R* result) {
+  DCHECK(result);
+  return Bind(&ReturnAsParamAdapter1<R, A1>, func, a1, result);
+}
+
+template <typename R>
+void ReplyAdapter(const Callback<void(R)>& callback, R* result) {
+  DCHECK(result);
+  if (!callback.is_null())

[willchan no longer on Chromium, 2012/03/01 02:17:24]

I think we should DCHECK in PostTaskAndReplyWithStatus(), so this should be
non-NULL.

[battre, 2012/03/07 20:01:48]

See above.

+    callback.Run(*result);
+}
+
+template <typename R, typename OWNED>

[willchan no longer on Chromium, 2012/03/01 02:17:24]

I think we should only have one parameter, let's not provide functionality here
to do type conversion of values. Let people do that themselves in their
callbacks.

[battre, 2012/03/07 20:01:48]

Done.

+Closure ReplyHelper(const Callback<void(R)>& callback, OWNED result) {
+  return Bind(&ReplyAdapter<R>, callback, result);
+}
+
+template <typename R1, typename R2>
+bool PostTaskAndReplyWithStatus(

[darin (slow to review), 2012/02/29 00:27:48]

why is this function named *WithStatus?

[willchan no longer on Chromium, 2012/03/01 02:17:24]

I think this should be the only function in base. Everything else should be in
base::internal.

[battre, 2012/03/07 20:01:48]

Done.

[battre, 2012/03/07 20:01:48]

I have renamed it to PostTaskAndReplyWithResult. I think this name fits better.

Done.

+    const scoped_refptr<TaskRunner>& task_runner,
+    const tracked_objects::Location& from_here,
+    const Callback<R1(void)>& do_work_and_return,
+    const Callback<void(R2)>& callback,
+    R2* result) {
+  return task_runner->PostTaskAndReply(
+      from_here,
+      ReturnAsParam<R1>(do_work_and_return, result),
+      ReplyHelper(callback, Owned(result)));
+}
+
+}  // namespace base
+
+#endif  // BASE_TASK_RUNNER_HELPERS_H_