Introduce a new framework for back-and-forth tracked/protected preferences migration.

base/critical_closure.h

Index: base/critical_closure.h
diff --git a/base/critical_closure.h b/base/critical_closure.h
index ca51ed5cef27d40923ba8e4d99de4044c63fa457..d43cba543a15030297670eab93efce7d3c3037ef 100644
--- a/base/critical_closure.h
+++ b/base/critical_closure.h
@@ -7,9 +7,43 @@
 
 #include "base/callback.h"
 
+#if defined(OS_IOS)
+#include "base/bind.h"
+#include "base/ios/scoped_critical_action.h"
+#endif
+
 namespace base {
 
-// Returns a closure that will continue to run for a period of time when the
+namespace internal {
+
+#if defined(OS_IOS)
+// This class wraps a closure so it can continue to run for a period of time
+// when the application goes to the background by using
+// |ios::ScopedCriticalAction|.
+template <typename R>
+class CriticalClosure {
+ public:
+  explicit CriticalClosure(const Callback<R(void)>& closure)
+      : closure_(closure) {}
+
+  ~CriticalClosure() {}
+
+  R Run() {
+    return closure_.Run();
+  }
+
+ private:
+  ios::ScopedCriticalAction critical_action_;
+  Callback<R(void)> closure_;
+
+  DISALLOW_COPY_AND_ASSIGN(CriticalClosure);
+};
+#endif  // defined(OS_IOS)
+
+}  // namespace internal

[Nico, 2014/05/06 21:14:50]

Why is this getting inlined? This seems worse than what we had before (since
you're removing that DCHECK, which I imagine someone added for a reason.)

(Side remark: This CL does more than one thing, it would've been good to split
it up more.)

[gab, 2014/05/07 03:59:09]

Because it's now templated and the callers must have all the code to be able to
compile+link (if we keep this in the .mm file, the templates don't get
instantiated in the library and aren't found at link time -- maybe we could
force instantiation for the two specific types we care about for now
Callback<void(void)> and Callback<bool(void)> in the .mm file though if you
prefer that?).

Another option is to leave the templates in the header, but bring back the
DCHECK via a C++ helper, e.g., IsMultiTaskingSupported() which could be
implemented in ObjC in critical_closure_internal_ios.mm.
I did that for now, let me know what you think.
I agree it's a big CL, but the new APIs only make sense in this change
(otherwise they're introduced yet no one uses them) -- given they don't make the
core logic of this CL harder I didn't think it was worth splitting them
(especially given the timing of this CL landing is tight).

+
+// Returns a closure (which may return a result, but must not require any extra
+// arguments) that will continue to run for a period of time when the
 // application goes to the background if possible on platforms where
 // applications don't execute while backgrounded, otherwise the original task is
 // returned.
@@ -23,14 +57,19 @@ namespace base {
 // background running time, |MakeCriticalClosure| should be applied on them
 // before posting.
 #if defined(OS_IOS)
-base::Closure MakeCriticalClosure(const base::Closure& closure);
-#else
-inline base::Closure MakeCriticalClosure(const base::Closure& closure) {
+template <typename R>
+Callback<R(void)> MakeCriticalClosure(const Callback<R(void)>& closure) {
+  return base::Bind(&internal::CriticalClosure<R>::Run,
+                    Owned(new internal::CriticalClosure<R>(closure)));
+}
+#else  // defined(OS_IOS)
+template <typename R>
+inline Callback<R(void)> MakeCriticalClosure(const Callback<R(void)>& closure) {
   // No-op for platforms where the application does not need to acquire
   // background time for closures to finish when it goes into the background.
   return closure;
 }
-#endif  // !defined(OS_IOS)
+#endif  // defined(OS_IOS)
 
 }  // namespace base