Add move constructor in ScopedClosureRunner.

base/callback_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.
 
 // This defines helpful methods for dealing with Callbacks.  Because Callbacks
 // are implemented using templates, with a class per callback signature, adding
 // methods to Callback<> itself is unattractive (lots of extra code gets
 // generated).  Instead, consider adding methods here.
 //
 // ResetAndReturn(&cb) is like cb.Reset() but allows executing a callback (via a
 // copy) after the original callback is Reset().  This can be handy if Run()
 // reads/writes the variable holding the Callback.
 
 #ifndef BASE_CALLBACK_HELPERS_H_
 #define BASE_CALLBACK_HELPERS_H_
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 
 namespace base {
 
 template <typename Sig>
 base::Callback<Sig> ResetAndReturn(base::Callback<Sig>* cb) {
   base::Callback<Sig> ret(*cb);
   cb->Reset();
   return ret;
 }
 
-// ScopedClosureRunner is akin to scoped_ptr for Closures. It ensures that the
-// Closure is executed and deleted no matter how the current scope exits.
+// ScopedClosureRunner is akin to std::unique_ptr<> for Closures. It ensures
+// that theClosure is executed no matter how the current scope exits.

[danakj, 2016/06/23 20:19:28]

the Closure

[Sergey Ulanov, 2016/06/23 23:30:32]

Done.

 class BASE_EXPORT ScopedClosureRunner {
  public:
   ScopedClosureRunner();
+  ScopedClosureRunner(ScopedClosureRunner&& other);

[danakj, 2016/06/23 20:19:28]

Any reason you're not defining operator= along with this as the style guide
asks?

Can you order this below the "normal" constructors, and separate it with
whitespace?

[Sergey Ulanov, 2016/06/23 23:30:32]

One small concern I have with operator= is that when assigning a=b the closure
in |a| has to be called. That behavior may be confusing and unexpected.  Should
we require that |a| is empty in that case, e.g. with a DCHECK? WDYT?
Done.

[danakj, 2016/06/24 20:06:30]

I think it makes sense that it does what reset does. Can you add a comment on
the operator= and on Reset() in the header saying that it runs the currently
owned closure?

[Sergey Ulanov, 2016/06/24 23:44:01]

Done.

BTW, As far as I can tell there is only one place where Reset() is called and it
looks to be used used incorrectly there:
https://codesearch.chromium.org/chromium/src/content/browser/compositor/surfa...

   explicit ScopedClosureRunner(const Closure& closure);
   ~ScopedClosureRunner();
 
   void Reset();
   void Reset(const Closure& closure);
   Closure Release() WARN_UNUSED_RESULT;
 
  private:
   Closure closure_;
 
   DISALLOW_COPY_AND_ASSIGN(ScopedClosureRunner);
 };
 
 }  // namespace base
 
 #endif  // BASE_CALLBACK_HELPERS_H_