Update Bind to match its comments and always capture by value.

webrtc/base/bind_unittest.cc

Index: webrtc/base/bind_unittest.cc
diff --git a/webrtc/base/bind_unittest.cc b/webrtc/base/bind_unittest.cc
index fa47d279f5da271005dd30b44ee02e7644cdb0b9..63a6b1ae9de46b76a54543ed4edc20a15ee8c0a0 100644
--- a/webrtc/base/bind_unittest.cc
+++ b/webrtc/base/bind_unittest.cc
@@ -25,7 +25,13 @@ struct MethodBindTester {
   int NullaryConst() const { ++call_count; return 2; }
   void UnaryVoid(int dummy) { ++call_count; }
   template <class T> T Identity(T value) { ++call_count; return value; }
-  int UnaryByRef(int& value) const { ++call_count; return ++value; }  // NOLINT
+  int UnaryByPointer(int* value) const {
+    ++call_count; return ++(*value);
+  }
+  int UnaryByRef(const int& value) const {
+    ++call_count;
+    return ++const_cast<int&>(value);
+  }
   int Multiply(int a, int b) const { ++call_count; return a * b; }
   void RefArgument(const scoped_refptr<LifeTimeCheck>& object) {
     EXPECT_TRUE(object.get() != nullptr);
@@ -64,26 +70,26 @@ int Multiply(int a, int b) { return a * b; }
 
 // Try to catch any problem with scoped_refptr type deduction in rtc::Bind at
 // compile time.
-static_assert(is_same<detail::RemoveScopedPtrRef<
+static_assert(is_same<detail::RemoveAllRef<
                           const scoped_refptr<RefCountInterface>&>::type,
-                      scoped_refptr<RefCountInterface>>::value,
+                      const scoped_refptr<RefCountInterface>>::value,
               "const scoped_refptr& should be captured by value");
 
-static_assert(is_same<detail::RemoveScopedPtrRef<const scoped_refptr<F>&>::type,
-                      scoped_refptr<F>>::value,
+static_assert(is_same<detail::RemoveAllRef<const scoped_refptr<F>&>::type,
+                      const scoped_refptr<F>>::value,
               "const scoped_refptr& should be captured by value");
 
 static_assert(
-    is_same<detail::RemoveScopedPtrRef<const int&>::type, const int&>::value,
-    "const int& should be captured as const int&");
+    is_same<detail::RemoveAllRef<const int&>::type, const int>::value,
+    "const int& should be captured as const int");
 
 static_assert(
-    is_same<detail::RemoveScopedPtrRef<const F&>::type, const F&>::value,
-    "const F& should be captured as const F&");
+    is_same<detail::RemoveAllRef<const F&>::type, const F>::value,
+    "const F& should be captured as const F");
 
 static_assert(
-    is_same<detail::RemoveScopedPtrRef<F&>::type, F&>::value,
-    "F& should be captured as F&");
+    is_same<detail::RemoveAllRef<F&>::type, F>::value,
+    "F& should be captured as F");
 
 #define EXPECT_IS_CAPTURED_AS_PTR(T)                              \
   static_assert(is_same<detail::PointerType<T>::type, T*>::value, \
@@ -129,11 +135,20 @@ TEST(BindTest, BindToMethod) {
                                &object, string_value)());
   EXPECT_EQ(6, object.call_count);
   int value = 11;
-  EXPECT_EQ(12, Bind(&MethodBindTester::UnaryByRef, &object, value)());
+  // Bind binds by value, even if the method signature is by reference, so
+  // "reference" binds require pointers.
+  EXPECT_EQ(12, Bind(&MethodBindTester::UnaryByPointer, &object, &value)());
   EXPECT_EQ(12, value);
   EXPECT_EQ(7, object.call_count);
-  EXPECT_EQ(56, Bind(&MethodBindTester::Multiply, &object, 7, 8)());
+  // It's possible to bind to a function that takes a const reference, though
+  // the capture will be a copy. See UnaryByRef hackery above where it removes
+  // the const to make sure the underlying storage is, in fact, a copy.
+  EXPECT_EQ(13, Bind(&MethodBindTester::UnaryByRef, &object, value)());
+  // But the original value is unmodified.
+  EXPECT_EQ(12, value);
   EXPECT_EQ(8, object.call_count);
+  EXPECT_EQ(56, Bind(&MethodBindTester::Multiply, &object, 7, 8)());
+  EXPECT_EQ(9, object.call_count);
 }
 
 TEST(BindTest, BindToFunction) {
@@ -210,13 +225,14 @@ const int* Ref(const int& a) { return &a; }
 
 }  // anonymous namespace
 
-// Test Bind with non-scoped_refptr<> reference argument.
+// Test Bind with non-scoped_refptr<> reference argument, which should be
+// modified to a non-reference capture.
 TEST(BindTest, RefArgument) {
   const int x = 42;
-  EXPECT_TRUE(Ref(x) == &x);
-  // Bind() should not make a copy of |x|, i.e. the pointers should be the same.
+  EXPECT_EQ(&x, Ref(x));
+  // Bind() should make a copy of |x|, i.e. the pointers should be the same.

[magjed_webrtc, 2015/10/23 05:17:01]

s/i.e. the pointers should be the same/i.e. the pointers should not be the
same/g.

[noahric, 2015/10/23 06:02:16]

Done.

   auto functor = Bind(&Ref, x);
-  EXPECT_TRUE(functor() == &x);
+  EXPECT_NE(&x, functor());
 }
 
 }  // namespace rtc