Discourage use of base::Passed().

See discussion thread: https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/8PJU6CEyGwg

No-Try: true
Bug: 812523
Change-Id: Ie021ad6803dbbaf02a4caba37c6b8a50a006b251
Reviewed-on: https://chromium-review.googlesource.com/919066
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537637}

diff --git a/base/bind.h b/base/bind.h
index d1723b1e..b57a969c 100644
--- a/base/bind.h
+++ b/base/bind.h

@@ -364,13 +364,16 @@
 // argument will CHECK() because the first invocation would have already
 // transferred ownership to the target function.
 //
+// Note that Passed() is not necessary with BindOnce(), as std::move() does the
+// same thing. Avoid Passed() in favor of std::move() with BindOnce().
+//
 // EXAMPLE OF Passed():
 //
 //   void TakesOwnership(std::unique_ptr<Foo> arg) { }
-//   std::unique_ptr<Foo> CreateFoo() { return std::unique_ptr<Foo>(new Foo());
+//   std::unique_ptr<Foo> CreateFoo() { return std::make_unique<Foo>();
 //   }
 //
-//   std::unique_ptr<Foo> f(new Foo());
+//   auto f = std::make_unique<Foo>();
 //
 //   // |cb| is given ownership of Foo(). |f| is now NULL.
 //   // You can use std::move(f) in place of &f, but it's more verbose.
@@ -388,11 +391,6 @@
 //   cb.Run();  // Foo() is now transferred to |arg| and deleted.
 //   cb.Run();  // This CHECK()s since Foo() already been used once.
 //
-// Passed() is particularly useful with PostTask() when you are transferring
-// ownership of an argument into a task, but don't necessarily know if the
-// task will always be executed. This can happen if the task is cancellable
-// or if it is posted to a TaskRunner.
-//
 // We offer 2 syntaxes for calling Passed().  The first takes an rvalue and
 // is best suited for use with the return value of a function or other temporary
 // rvalues. The second takes a pointer to the scoper and is just syntactic sugar

diff --git a/docs/callback.md b/docs/callback.md
index f3f45f5..3c0d7ad 100644
--- a/docs/callback.md
+++ b/docs/callback.md

@@ -229,10 +229,10 @@
 `const std::string&`. Like normal function dispatch, `base::Bind`, will coerce
 parameter types if possible.
 
-### Avoiding Copies with Callback Parameters
+### Avoiding Copies With Callback Parameters
 
-A parameter of `base::Bind()` is moved into its internal storage if it is passed as a
-rvalue.
+A parameter of `base::BindRepeating()` or `base::BindOnce()` is moved into its
+internal storage if it is passed as a rvalue.
 
 ```cpp
 std::vector<int> v = {1, 2, 3};
@@ -241,23 +241,37 @@
 ```
 
 ```cpp
-std::vector<int> v = {1, 2, 3};
 // The vector is moved into the internal storage without copy.
 base::Bind(&Foo, std::vector<int>({1, 2, 3}));
 ```
 
-A bound object is moved out to the target function if you use `base::Passed()`
-for the parameter. If you use `base::BindOnce()`, the bound object is moved out
-even without `base::Passed()`.
+Arguments bound with `base::BindOnce()` are always moved, if possible, to the
+target function.
+A function parameter that is passed by value and has a move constructor will be
+moved instead of copied.
+This makes it easy to use move-only types with `base::BindOnce()`.
+
+In contrast, arguments bound with `base::BindRepeating()` are only moved to the
+target function if the argument is bound with `base::Passed()`.
+
+**DANGER**:
+A `base::RepeatingCallback` can only be run once if arguments were bound with
+`base::Passed()`.
+For this reason, avoid `base::Passed()`.
+If you know a callback will only be called once, prefer to refactor code to
+work with `base::OnceCallback` instead.
+
+Avoid using `base::Passed()` with `base::BindOnce()`, as `std::move()` does the
+same thing and is more familiar.
 
 ```cpp
 void Foo(std::unique_ptr<int>) {}
-std::unique_ptr<int> p(new int(42));
+auto p = std::make_unique<int>(42);
 
 // |p| is moved into the internal storage of Bind(), and moved out to |Foo|.
 base::BindOnce(&Foo, std::move(p));
-base::BindRepeating(&Foo, base::Passed(&p));
-base::BindRepeating(&Foo, base::Passed(std::move(p)));
+base::BindRepeating(&Foo, base::Passed(&p)); // Ok, but subtle.
+base::BindRepeating(&Foo, base::Passed(std::move(p))); // Ok, but subtle.
 ```
 
 ## Quick reference for advanced binding
@@ -337,10 +351,9 @@
 
 ```cpp
 void TakesOwnership(std::unique_ptr<Foo> arg) {}
-std::unique_ptr<Foo> f(new Foo);
+auto f = std::make_unique<Foo>();
 // f becomes null during the following call.
-base::RepeatingClosure cb =
-    base::BindRepeating(&TakesOwnership, base::Passed(&f));
+base::OnceClosure cb = base::BindOnce(&TakesOwnership, std::move(f));
 ```
 
 Ownership of the parameter will be with the callback until the callback is run,