Reporting of policy errors via host-offline-reason: part 3

remoting/host/mock_callback.h

Index: remoting/host/mock_callback.h
diff --git a/remoting/host/mock_callback.h b/remoting/host/mock_callback.h
index ca3feb6d06a415f1853b363e7b089f8b0a03ad7a..4eb8cb032b1bbf26afb77813aaaa6481b4241c2e 100644
--- a/remoting/host/mock_callback.h
+++ b/remoting/host/mock_callback.h
@@ -10,11 +10,33 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
+#include "base/memory/ref_counted.h"
 
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace remoting {
 
+// MockCallback<Sig> can be used to mock Callback<Sig> from base/callback.h
+//
+// Basic usage example:
+//
+//   MockCallback<void(int)> mock_callback;
+//   EXPECT_CALL(mock_callback, Run(123)).Times(2);
+//   base::Callback<void(int)> callback = mock_callback.GetCallback();
+//
+//   // Use |callback| in the remainder of the test.
+//   // GMock will enforce the expectations from above.
+//
+// HasRemainingCallbacks usage example:
+//
+//   MockCallback<int()> mock_callback;
+//   EXPECT_CALL(mock_callback, Run()).WillOnce(Return(123));
+//   HypotheticalFunctionUnderTest(mock_callback.GetCallback());
+//   EXPECT_FALSE(mock_callback.HasRemainingCallbacks())
+//      << "Verify that FunctionUnderTest didn't store the callback "
+//      << "and therefore callers of FunctionUnderTest don't have to "
+//      << "worry about lifetime of arguments bound to the callback";
+
 template <typename Sig>
 class MockCallback;
 
@@ -23,16 +45,106 @@ class MockCallback<R()> {
  public:
   MOCK_CONST_METHOD0_T(Run, R());
 
-  MockCallback() {
+  MockCallback() : mock_callback_tracker_(new MockCallbackTracker(this)) {
   }
 
   // Caller of GetCallback has to guarantee that the returned callback
   // will not be run after |this| is destroyed.
   base::Callback<R()> GetCallback() {
-    return base::Bind(&MockCallback<R()>::Run, base::Unretained(this));
+    return base::Bind(&MockCallbackTracker::Run, mock_callback_tracker_);
+  }
+
+  // Returns |true| if callbacks returned by |GetCallback| have not been
+  // destroyed yet.
+  bool HasRemainingCallbacks() const {
+    bool is_tracker_field_the_only_ref = mock_callback_tracker_->HasOneRef();
+    return !is_tracker_field_the_only_ref;
   }
 
  private:
+  // MockCallbackTracker has 2 goals:
+  // - Enable counting of callbacks for |HasRemainingCallbacks| method.
+  // - Allow stack-allocated MockCallbacks to be short-lived, while
+  //   callbacks go through a long-lived MockCallbackTracker

[Lambros, 2014/11/19 02:29:42]

MockCallbackTracker outlives MockCallback? So why is MockCallbackTracker keeping
a raw pointer to MockCallback?

[Łukasz Anforowicz, 2014/11/19 21:44:24]

Well, you proposed changing WeakPtr to base::Unretained (which I really
appreciate and wholeheartedly agree with).  We are just moving a raw pointer
from inside a callback to inside of MockCallbackTracker.  Keeping a raw pointer
inside MockCallbackTracker will let us reintroduce WeakPtr-like semantics later
on (but with us in control of what happens when calling a callback associated
with a dead MockCallback, rather with a default behavior of Callback that treats
dead WeakPtrs as no-op callbacks).  

Do you think the changelist would be better and more self-contained if I
reintroduced WeakPtr-like semantics today, rather than postponing it until later
(i.e. until we are trying to move this to base/mock_callback.h and need to
discuss desired behavior with broader Chromium audience).  If yes, then please
shout at me and I'll do it :-).  Otherwise, if I misunderstood your conrerns,
then let's talk more about this.

[Łukasz Anforowicz, 2014/11/19 21:54:12]

BTW: having MockCallbackTracker is useful even without reintroducing
WeakPtr-like semantics - it is needed in the current changelist to enable
HasRemainingCallbacks method of MockCallback.

+  //   (which in the future can enable better handling of callback-
+  //    -run-after-mock-callback-already-destroyed).
+  class MockCallbackTracker
+      : public base::RefCountedThreadSafe<MockCallbackTracker> {
+   public:
+    MockCallbackTracker(MockCallback* mock_callback)
+        : mock_callback_(mock_callback) {
+    }
+
+    R Run() {
+      // Forward to (gMock controlled) MockCallback::Run method.
+      return mock_callback_->Run();
+    }
+
+   private:
+    // Ref-counting helpers.
+    friend class base::RefCountedThreadSafe<MockCallbackTracker>;
+    ~MockCallbackTracker() {
+    }
+
+    MockCallback<R()>* mock_callback_;
+  };
+
+  scoped_refptr<MockCallbackTracker> mock_callback_tracker_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockCallback);
+};
+
+template <typename R, typename A1>
+class MockCallback<R(A1)> {
+ public:
+  MOCK_CONST_METHOD1_T(Run, R(A1));
+
+  MockCallback() : mock_callback_tracker_(new MockCallbackTracker(this)) {
+  }
+
+  // Caller of GetCallback has to guarantee that the returned callback
+  // will not be run after |this| is destroyed.
+  base::Callback<R(A1)> GetCallback() {
+    return base::Bind(&MockCallbackTracker::Run, mock_callback_tracker_);
+  }
+
+  // Returns |true| if callbacks returned by |GetCallback| have not been
+  // destroyed yet.
+  bool HasRemainingCallbacks() const {
+    bool is_tracker_field_the_only_ref = mock_callback_tracker_->HasOneRef();
+    return !is_tracker_field_the_only_ref;
+  }
+
+ private:
+  // MockCallbackTracker has 2 goals:
+  // - Enable counting of callbacks for |HasRemainingCallbacks| method.
+  // - Allow stack-allocated MockCallbacks to be short-lived, while
+  //   callbacks go through a long-lived MockCallbackTracker
+  //   (which in the future can enable better handling of callback-
+  //    -run-after-mock-callback-already-destroyed).
+  class MockCallbackTracker
+      : public base::RefCountedThreadSafe<MockCallbackTracker> {
+   public:
+    MockCallbackTracker(MockCallback* mock_callback)
+        : mock_callback_(mock_callback) {
+    }
+
+    R Run(A1 a1) {
+      // Forward to (gMock controlled) MockCallback::Run method.
+      return mock_callback_->Run(a1);
+    }
+
+   private:
+    // Ref-counting helpers.
+    friend class base::RefCountedThreadSafe<MockCallbackTracker>;
+    ~MockCallbackTracker() {
+    }
+
+    MockCallback<R(A1)>* mock_callback_;
+  };
+
+  scoped_refptr<MockCallbackTracker> mock_callback_tracker_;
+
   DISALLOW_COPY_AND_ASSIGN(MockCallback);
 };