Skip to content

GitLab

T
tangled
Commit 1b1b5aec authored by Yuta Kitamura's avatar Yuta Kitamura Committed by Commit Bot
Browse files
Options

Revert "Add supports for Objective-C blocks to base::Bind*" 

This reverts commit 3a2babeb.

Reason for revert: Suspected cause of ASAN failure (bug 845687)
and Mac-only flakiness (bug 845760, but 845759, bug 845690).

Original change's description:
> Add supports for Objective-C blocks to base::Bind*
> 
> Add template specifialization allowing to use base::Bind* with
> Objective-C block. To avoid breaking ODR, it is necessary to
> use base::WrapBlock() when ARC support is disabled, but when
> it is enabled, the block can be directly bound by base::Bind*.
> 
> This also allow to use base::BindOnce() with blocks and thus
> pass them  move-only objects, allowing the following:
> 
>   __weak Foo* weakSelf = self;
>   std::unique_ptr<Bar> bar = ...;
>   base::OnceClosure closure = base::BindOnce(
>       ^(std::unique_ptr<Bar> bar) {
>           [weakSelf adoptBar:std::move(bar)];
>       }, std::move(bar));
> 
> The base::BindBlock and base::BindBlockArc templates are kept for
> compatibility but are now just wrapper for base::BindRepeating.
> 
> Bug: 701275
> Change-Id: I6e5d5309bcae768b2d1e8425b27ca295098e30b1
> Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
> Reviewed-on: https://chromium-review.googlesource.com/977912
> Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
> Reviewed-by: Taiju Tsuiki <tzik@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#560634}

TBR=thakis@chromium.org,sdefresne@chromium.org,tzik@chromium.org

Change-Id: Id72a1806176f830a1887243fcd2795a927e7d638
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 701275
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/1068588Reviewed-by: default avatarYuta Kitamura <yutak@chromium.org>
Commit-Queue: Yuta Kitamura <yutak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560951}
parent b6087f26
Hide whitespace changes
Inline Side-by-side
Showing with 52 additions and 145 deletions
base/bind.h
View file @ 1b1b5aec
......@@ -8,12 +8,6 @@
#include <utility>
#include "base/bind_internal.h"
#include "base/compiler_specific.h"
#include "build/build_config.h"
#if defined(OS_MACOSX) && !HAS_FEATURE(objc_arc)
#include "base/mac/scoped_block.h"
#endif
// -----------------------------------------------------------------------------
// Usage documentation
......@@ -458,25 +452,6 @@ static inline internal::IgnoreResultHelper<T> IgnoreResult(T data) {
return internal::IgnoreResultHelper<T>(std::move(data));
}
#if defined(OS_MACOSX) && !HAS_FEATURE(objc_arc)
// RetainBlock() is used to adapt an Objective-C block when Automated Reference
// Counting (ARC) is disabled. This is unnecessary when ARC is enabled, as the
// BindOnce and BindRepeating already support blocks then.
//
// EXAMPLE OF RetainBlock():
//
// // Wrap the block and bind it to a callback.
// Callback<void(int)> cb = Bind(RetainBlock(^(int n) { NSLog(@"%d", n); }));
// cb.Run(1); // Logs "1".
template <typename R, typename... Args>
base::mac::ScopedBlock<R (^)(Args...)> RetainBlock(R (^block)(Args...)) {
return base::mac::ScopedBlock<R (^)(Args...)>(block,
base::scoped_policy::RETAIN);
}
#endif // defined(OS_MACOSX) && !HAS_FEATURE(objc_arc)
} // namespace base
#endif // BASE_BIND_H_
base/bind_internal.h
View file @ 1b1b5aec
......@@ -11,16 +11,11 @@
#include <utility>
#include "base/callback_internal.h"
#include "base/compiler_specific.h"
#include "base/memory/raw_scoped_refptr_mismatch_checker.h"
#include "base/memory/weak_ptr.h"
#include "base/template_util.h"
#include "build/build_config.h"
#if defined(OS_MACOSX) && !HAS_FEATURE(objc_arc)
#include "base/mac/scoped_block.h"
#endif
// See base/callback.h for user documentation.
//
//
......@@ -438,50 +433,6 @@ struct FunctorTraits<R(__fastcall*)(Args...)> {
#endif // defined(OS_WIN) && !defined(ARCH_CPU_X86_64)
#if defined(OS_MACOSX)
// Support for Objective-C blocks. There are two implementation depending
// on whether Automated Reference Counting (ARC) is enabled. When ARC is
// enabled, then the block itself can be bound as the compiler will ensure
// its lifetime will be correctly managed. Otherwise, require the block to
// be wrapped in a base::mac::ScopedBlock (via base::RetainBlock) that will
// correctly manage the block lifetime.
//
// The two implementation ensure that the One Definition Rule (ODR) is not
// broken (it is not possible to write a template base::RetainBlock that would
// work correctly both with ARC enabled and disabled).
#if HAS_FEATURE(objc_arc)
template <typename R, typename... Args>
struct FunctorTraits<R (^)(Args...)> {
using RunType = R(Args...);
static constexpr bool is_method = false;
static constexpr bool is_nullable = true;
template <typename... RunArgs>
static R Invoke(R (^block)(Args...), RunArgs&&... args) {
return block(std::forward<RunArgs>(args)...);
}
};
#else // HAS_FEATURE(objc_arc)
template <typename R, typename... Args>
struct FunctorTraits<base::mac::ScopedBlock<R (^)(Args...)>> {
using RunType = R(Args...);
static constexpr bool is_method = false;
static constexpr bool is_nullable = true;
template <typename BlockType, typename... RunArgs>
static R Invoke(BlockType&& block, RunArgs&&... args) {
return block.get()(std::forward<RunArgs>(args)...);
}
};
#endif // HAS_FEATURE(objc_arc)
#endif // defined(OS_MACOSX)
// For methods.
template <typename R, typename Receiver, typename... Args>
struct FunctorTraits<R (Receiver::*)(Args...)> {
......
base/mac/bind_objc_block.h
View file @ 1b1b5aec
......@@ -12,55 +12,64 @@
#include "base/compiler_specific.h"
#include "base/mac/scoped_block.h"
namespace base {
#if !HAS_FEATURE(objc_arc)
// Construct a callback from an Objective-C block when ARC is disabled.
// Deprecated, instead use base::RetainBlock() and use base::BindOnce or
// base::BindRepeating with the wrapped block.
// BindBlock builds a callback from an Objective-C block. Example usages:
//
// Closure closure = BindBlock(^{DoSomething();});
//
// For example, the following are equivalent:
// Callback<int(void)> callback = BindBlock(^{return 42;});
//
// // Deprecated.
// base::RepeatingClosure closure =
// base::BindRepeating(
// base::BindBlock(^(int n) { NSLog(@"%d"); }),
// 1);
// Callback<void(const std::string&, const std::string&)> callback =
// BindBlock(^(const std::string& arg0, const std::string& arg1) {
// ...
// });
//
// // Recommended.
// base::RepeatingClosure closure =
// base::BindRepeating(
// base::RetainBlock(^(int n) { NSLog(@"%d"); }),
// 1);
template <typename R, typename... Args, typename... BoundArgs>
base::RepeatingCallback<
MakeUnboundRunType<base::mac::ScopedBlock<R (^)(Args...)>, BoundArgs...>>
BindBlock(R (^block)(Args...), BoundArgs&&... args) {
return base::BindRepeating(base::RetainBlock(block),
std::forward<BoundArgs>(args)...);
// These variadic templates will accommodate any number of arguments, however
// the underlying templates in bind_internal.h and callback.h are limited to
// seven total arguments, and the bound block itself is used as one of these
// arguments, so functionally the templates are limited to binding blocks with
// zero through six arguments.
//
// For code compiled with ARC (automatic reference counting), use BindBlockArc.
// This is because the method has a different implementation (to avoid over-
// retaining the block) and need to have a different name not to break the ODR
// (one definition rule). Another subtle difference is that the implementation
// will call a different version of ScopedBlock constructor thus the linker must
// not merge both functions.
namespace base {
namespace internal {
// Helper function to run the block contained in the parameter.
template<typename R, typename... Args>
R RunBlock(base::mac::ScopedBlock<R(^)(Args...)> block, Args... args) {
R(^extracted_block)(Args...) = block.get();
return extracted_block(args...);
}
} // namespace internal
#if !defined(__has_feature) || !__has_feature(objc_arc)
// Construct a callback from an objective-C block with up to six arguments (see
// note above).
template<typename R, typename... Args>
base::Callback<R(Args...)> BindBlock(R(^block)(Args...)) {
return base::Bind(
&base::internal::RunBlock<R, Args...>,
base::mac::ScopedBlock<R (^)(Args...)>(
base::mac::internal::ScopedBlockTraits<R (^)(Args...)>::Retain(
block)));
}
#else
// Construct a callback from an Objective-C block when ARC is enabled.
// Deprecated, instead use base::BindOnce or base::BindRepeating directly.
//
// For example, the following are equivalent:
//
// // Deprecated.
// base::RepeatingClosure closure =
// base::BindRepeating(
// base::BindBlockArc(^(int n) { NSLog(@"%d"); }),
// 1);
//
// // Recommended.
// base::RepeatingClosure closure =
// base::BindRepeating(^(int n) { NSLog(@"%d"); }, 1);
template <typename R, typename... Args, typename... BoundArgs>
base::RepeatingCallback<MakeUnboundRunType<R (^)(Args...), BoundArgs...>>
BindBlockArc(R (^block)(Args...), BoundArgs&&... args) {
return base::BindRepeating(block, std::forward<BoundArgs>(args)...);
// Construct a callback from an objective-C block with up to six arguments (see
// note above).
template <typename R, typename... Args>
base::Callback<R(Args...)> BindBlockArc(R (^block)(Args...)) {
return base::Bind(&base::internal::RunBlock<R, Args...>,
base::mac::ScopedBlock<R (^)(Args...)>(block));
}
#endif
......
base/mac/bind_objc_block_unittest.mm
View file @ 1b1b5aec
......@@ -9,13 +9,13 @@
#include "base/bind.h"
#include "base/callback.h"
#include "base/callback_helpers.h"
#include "base/mac/scoped_nsautorelease_pool.h"
#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "testing/gtest_mac.h"
#if defined(OS_IOS)
#include "base/ios/weak_nsobject.h"
#include "base/mac/scoped_nsautorelease_pool.h"
#endif
namespace {
......@@ -103,20 +103,6 @@ TEST(BindObjcBlockTest, TestSixArguments) {
EXPECT_EQ(result2, 6);
}
TEST(BindObjcBlockTest, TestBlockMoveable) {
base::OnceClosure c;
__block BOOL invoked_block = NO;
{
base::mac::ScopedNSAutoreleasePool autorelease_pool;
c = base::BindOnce(base::RetainBlock(^(std::unique_ptr<BOOL> v) {
invoked_block = *v;
}),
std::make_unique<BOOL>(YES));
};
std::move(c).Run();
EXPECT_TRUE(invoked_block);
}
#if defined(OS_IOS)
TEST(BindObjcBlockTest, TestBlockReleased) {
......
base/mac/bind_objc_block_unittest_arc.mm
View file @ 1b1b5aec
......@@ -105,20 +105,6 @@ TEST(BindObjcBlockTestARC, TestSixArguments) {
EXPECT_EQ(result2, 6);
}
TEST(BindObjcBlockTestARC, TestBlockMoveable) {
base::OnceClosure c;
__block BOOL invoked_block = NO;
@autoreleasepool {
c = base::BindOnce(
^(std::unique_ptr<BOOL> v) {
invoked_block = *v;
},
std::make_unique<BOOL>(YES));
};
std::move(c).Run();
EXPECT_TRUE(invoked_block);
}
#if defined(OS_IOS)
TEST(BindObjcBlockTestARC, TestBlockReleased) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment