Skip to content

Commit

Permalink
Fix base::Callback<>::IsCancelled handling on nested base::Callback
Browse files Browse the repository at this point in the history
Previous implementation of IsCancelled doesn't have a specialization for
nested OnceCallback. That implies OnceCallback::IsCancelled returns false
when it's a nested callback and the inner callback is a cancelled OnceCallback.

Review-Url: https://codereview.chromium.org/2401623002
Cr-Commit-Position: refs/heads/master@{#423796}
  • Loading branch information
tzik authored and Commit bot committed Oct 7, 2016
1 parent dbcc546 commit 44adf07
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 6 deletions.
10 changes: 7 additions & 3 deletions base/bind_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -411,11 +411,15 @@ struct CancellationChecker<
}
};

template <typename Signature, typename... BoundArgs>
struct CancellationChecker<BindState<Callback<Signature>, BoundArgs...>> {
template <typename Signature,
typename... BoundArgs,
CopyMode copy_mode,
RepeatMode repeat_mode>
struct CancellationChecker<
BindState<Callback<Signature, copy_mode, repeat_mode>, BoundArgs...>> {
static constexpr bool is_cancellable = true;
static bool Run(const BindStateBase* base) {
using Functor = Callback<Signature>;
using Functor = Callback<Signature, copy_mode, repeat_mode>;
using BindStateType = BindState<Functor, BoundArgs...>;
const BindStateType* bind_state = static_cast<const BindStateType*>(base);
return bind_state->functor_.IsCancelled();
Expand Down
27 changes: 24 additions & 3 deletions base/bind_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ using ::testing::Return;
using ::testing::StrictMock;

namespace base {

using internal::OnceCallback;
using internal::RepeatingCallback;
using internal::OnceClosure;
using internal::RepeatingClosure;
using internal::BindOnce;
using internal::BindRepeating;

namespace {

class IncompleteType;
Expand Down Expand Up @@ -1041,12 +1049,21 @@ TEST_F(BindTest, Cancellation) {
EXPECT_CALL(no_ref_, VoidMethodWithIntArg(_)).Times(2);

WeakPtrFactory<NoRef> weak_factory(&no_ref_);
base::Callback<void(int)> cb =
Bind(&NoRef::VoidMethodWithIntArg, weak_factory.GetWeakPtr());
Closure cb2 = Bind(cb, 8);
RepeatingCallback<void(int)> cb =
BindRepeating(&NoRef::VoidMethodWithIntArg, weak_factory.GetWeakPtr());
RepeatingClosure cb2 = BindRepeating(cb, 8);
OnceClosure cb3 = BindOnce(cb, 8);

OnceCallback<void(int)> cb4 =
BindOnce(&NoRef::VoidMethodWithIntArg, weak_factory.GetWeakPtr());
EXPECT_FALSE(cb4.IsCancelled());

OnceClosure cb5 = BindOnce(std::move(cb4), 8);

EXPECT_FALSE(cb.IsCancelled());
EXPECT_FALSE(cb2.IsCancelled());
EXPECT_FALSE(cb3.IsCancelled());
EXPECT_FALSE(cb5.IsCancelled());

cb.Run(6);
cb2.Run();
Expand All @@ -1055,9 +1072,13 @@ TEST_F(BindTest, Cancellation) {

EXPECT_TRUE(cb.IsCancelled());
EXPECT_TRUE(cb2.IsCancelled());
EXPECT_TRUE(cb3.IsCancelled());
EXPECT_TRUE(cb5.IsCancelled());

cb.Run(6);
cb2.Run();
std::move(cb3).Run();
std::move(cb5).Run();
}

TEST_F(BindTest, OnceCallback) {
Expand Down

0 comments on commit 44adf07

Please sign in to comment.