Implement generic CreateCopyResultCallback and RunAndQuit.

chrome/browser/google_apis/test_util.h

Index: chrome/browser/google_apis/test_util.h
diff --git a/chrome/browser/google_apis/test_util.h b/chrome/browser/google_apis/test_util.h
index d76f360114ee36f398b5e28d052c56381f5815bc..dcdec14496bc1b64bd076036f1f37f3e16106df9 100644
--- a/chrome/browser/google_apis/test_util.h
+++ b/chrome/browser/google_apis/test_util.h
@@ -7,7 +7,10 @@
 
 #include <string>
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/template_util.h"
 #include "chrome/browser/google_apis/gdata_errorcode.h"
 
 class GURL;
@@ -43,6 +46,9 @@ namespace test_util {
 // repeatedly.
 void RunBlockingPoolTask();
 
+// Runs the closure, and then quits the current MessageLoop.
+void RunAndQuit(const base::Closure& closure);
+
 // Removes |prefix| from |input| and stores the result in |output|. Returns
 // true if the prefix is removed.
 bool RemovePrefix(const std::string& input,
@@ -185,6 +191,135 @@ bool ParseContentRangeHeader(const std::string& value,
                              int64* end_position,
                              int64* length);
 
+// Google API related code and Drive File System code works on asynchronous
+// architecture, so we defined lots of callback to receive results of

[hashimoto, 2013/03/13 03:34:00]

"so we defined lots of callback" and the "callback" in the next sentence are
confusing.
Maybe we can remove "so we defined..." from this sentence?

[hidehiko, 2013/03/13 05:42:10]

Refined. How about this?

+// such asynchronous operations.
+// Following code implements a callback to copy such results from asynchronous
+// operation. Here is its usage:
+//
+//   class Operation;  // Testee Operation class.

[hashimoto, 2013/03/13 03:34:00]

This comment seems a bit verbose.
Can't we simplify the example like this?

ResultType1 result1;
ResultType2 result2;

PerformSomeAsynchronousTask(param1, param2, CreateCopyResultCallback(&result1,
&result2));
test_util::RunBlockingPoolTask()  // Run message loop to complete the
asynchronous task.

[hidehiko, 2013/03/13 05:42:10]

Done.

+//
+//   // Prepare result storage.
+//   ResultType1 result1;
+//   ResultType2 result2;
+//
+//   // Create an operation.
+//   Operation operation(
+//       ...(some arguments)...,
+//       CreateCopyResultCallback(&result1, &result2));
+//   operation.StartOperation();  // And start to run it.
+//   ... // Run message loop to complete the asynchronous task.
+//
+//   // Hereafter, we can write expectation with result1 and result2.
+//   EXPECT_EQ(expected_result1, result1);
+//   EXPECT_EQ(expected_result2, result2);
+//
+// Note: The max arity of the supported function is 3. The limitation comes
+//   from the max arity of base::Callback, which is 7. A created callback
+//   consumes two arguments for each input type.
+// TODO(hidehiko): Use replace CopyResultFromXxxCallback method defined above
+//   by this one. (crbug.com/180569).
+namespace internal {
+// Following helper templates are to support Chrome's move semantics.
+// Their goal is defining helper methods which are similar to:
+//   void CopyResultCallback1(T1* out1, T1&& in1)
+//   void CopyResultCallback2(T1* out1, T2* out2, T1&& in1, T2&& in2)
+//            :
+// in C++11.
+
+// Declare if the type is movable or not. Currently limited to scoped_ptr only.
+// We can add more types upon the usage.
+template<typename T> struct IsMovable : base::false_type {};
+template<typename T, typename D>
+struct IsMovable<scoped_ptr<T, D> > : base::true_type {};
+
+// The incoming type is as follows:
+// 1) If argument type |T| is non-class type or supporting Chrome's Move

[kinaba, 2013/03/13 06:34:06]

In offline chat I was convinced why it cannot be const T& for non-class types.

What the template has to compute is the _exact_ signature of callbacks
we want to create, not just types suitable for move semantics.
In our case, because we're using non-ref parameters for primitive and movable
types for callbacks declared in drive_service_interface.h, we have to respect it
here.

Could you clarify about it in the comment?

[hidehiko, 2013/03/13 08:49:19]

Done.

+//    concept, |InType| is |T| as is.
+// 2) Otherwise, const T&.
+template<bool IsNonMovableClass, typename T> struct InTypeHelper {

[hashimoto, 2013/03/13 03:34:00]

All other parts are dealing with IsMovable, only this part is dealing with the
negation, IsNonMovable.
How about something like "UseConstRef"?

[hidehiko, 2013/03/13 05:42:10]

Done.

+  typedef T InType;
+};
+template<typename T> struct InTypeHelper<true, T> {
+  typedef const T& InType;
+};
+
+// Simulate the move operation. To avoid value copy in argument, the method

[hashimoto, 2013/03/13 03:34:00]

s/Simulate/Simulates/?

"To avoid value copy" does not apply to scoped_ptr, the argument must be pointer
because scoped_ptr::Pass has side effects.
Can we remove this part from the comment?

[hidehiko, 2013/03/13 05:42:10]

OK. Refined the comment, but please let me keep mentioning about the pointer.

+// takes pointer.
+template<bool IsMovable, typename T> struct MoveHelper {
+  static const T& Move(const T* in) { return *in; }
+};
+template<typename T> struct MoveHelper<true, T> {
+  static T Move(T* in) { return in->Pass(); }
+};
+
+// Helper to handle Chrome's move semantics correctly.
+template<typename T> struct CopyResultCallbackHelper {
+  // The input type. See also comments of InTypeHelper.
+  typedef typename InTypeHelper<
+    base::is_class<T>::value && !IsMovable<T>::value, T>::InType InType;

[kinaba, 2013/03/13 04:07:44]

I don't feel it worth caring whether is_class<T> or not. Why don't we check just
IsMovable<T> and merge InTypeHelper into MoveHelper.

Then perhaps MoveHelper will be mergable to CopyResultCallbackHelper, like:
template<typename T, bool Movable=IsMovable<T>::value>
strcut CopyResultCallbackHelper { ...movable-case... };
template<typename T>
strcut CopyResultCallbackHelper<T, false> { ...non-movable-case... };

[kinaba, 2013/03/13 04:18:13]

Or even,

template<typename T>
struct CopyResultCallbackHelper { non-movable cases };
template<typename T>
struct CopyResultCallbackHelperForMovableTypes { movable cases };

struct CopyResultCallbackHelper<scoped_ptr>
 : CopyResultCallbackHelperForMovableTypes<scoped_ptr> {};

no need for true_type/false_type.

[hidehiko, 2013/03/13 05:42:10]

There are three kinds of types here.
- non-class type (e.g. primitive types, enums).
  The input type is |T|. It is not "move" supported,
  so copy the value.
- class type without chrome's move support (e.g. base::FilePath)
  The input type is |const T&|. Copy the value.
- class type with chrome's move support (e.g. scoped_ptr<U>).
  The input type is |T|. Move the value via Pass() method.

Thus it is necessary to check is_class here.
Changed the implementation to use inheritance.

+
+  // The implementation of move like operation.
+  typedef MoveHelper<IsMovable<T>::value, T> MoveHelperType;
+};
+
+// Copies the |in|'s value to |out|.
+template<typename T1>
+void CopyResultCallback1(

[hashimoto, 2013/03/13 03:34:00]

Naming functions in Name1, Name2, ... looks worse than sharing one name.
Could you refine these function names?

[hidehiko, 2013/03/13 05:42:10]

Hmm, I thought we need to static_cast to resolve function pointer of the
overloaded methods, but for templates (at least) g++ correctly resolves the
overloading by the number of template parameters. I'm trying to send this to
try-bot, but it seems stuck now, unfortunately. I'll send it later, and if it
turned out one of our compilers failed let me revert to names with the arity
suffix (instead of casting).

+    T1* out,
+    typename CopyResultCallbackHelper<T1>::InType in) {
+  *out = CopyResultCallbackHelper<T1>::MoveHelperType::Move(&in);

[hashimoto, 2013/03/13 03:34:00]

How about adding a static method Move to CopyResultCallbackHelper?

[hidehiko, 2013/03/13 05:42:10]

Inherited by MoveHelper.

+}
+
+// Copies the |in1|'s value to |out1|, and |in2|'s to |out2|.
+template<typename T1, typename T2>
+void CopyResultCallback2(
+    T1* out1,
+    T2* out2,
+    typename CopyResultCallbackHelper<T1>::InType in1,
+    typename CopyResultCallbackHelper<T2>::InType in2) {
+  *out1 = CopyResultCallbackHelper<T1>::MoveHelperType::Move(&in1);
+  *out2 = CopyResultCallbackHelper<T2>::MoveHelperType::Move(&in2);
+}
+
+// Copies the |in1|'s value to |out1|, |in2|'s to |out2|, and |in3|'s to |out3|.
+template<typename T1, typename T2, typename T3>
+void CopyResultCallback3(
+    T1* out1,
+    T2* out2,
+    T3* out3,
+    typename CopyResultCallbackHelper<T1>::InType in1,
+    typename CopyResultCallbackHelper<T2>::InType in2,
+    typename CopyResultCallbackHelper<T3>::InType in3) {
+  *out1 = CopyResultCallbackHelper<T1>::MoveHelperType::Move(&in1);
+  *out2 = CopyResultCallbackHelper<T2>::MoveHelperType::Move(&in2);
+  *out3 = CopyResultCallbackHelper<T3>::MoveHelperType::Move(&in3);
+}
+
+}  // namespace internal
+
+template<typename T1>
+base::Callback<void(typename internal::CopyResultCallbackHelper<T1>::InType)>
+CreateCopyResultCallback(T1* out1) {
+  return base::Bind(&internal::CopyResultCallback1<T1>, out1);
+}
+
+template<typename T1, typename T2>
+base::Callback<void(typename internal::CopyResultCallbackHelper<T1>::InType,
+                    typename internal::CopyResultCallbackHelper<T2>::InType)>
+CreateCopyResultCallback(T1* out1, T2* out2) {
+  return base::Bind(&internal::CopyResultCallback2<T1, T2>, out1, out2);
+}
+
+template<typename T1, typename T2, typename T3>
+base::Callback<void(typename internal::CopyResultCallbackHelper<T1>::InType,
+                    typename internal::CopyResultCallbackHelper<T2>::InType,
+                    typename internal::CopyResultCallbackHelper<T3>::InType)>
+CreateCopyResultCallback(T1* out1, T2* out2, T3* out3) {
+  return base::Bind(
+      &internal::CopyResultCallback3<T1, T2, T3>, out1, out2, out3);
+}
+
 }  // namespace test_util
 }  // namespace google_apis