[binder] Clean up parcel interfaces (#27322)

Some changes:

* OnTransactCb now takes a non-const ReadableParcel* so that testing
  codes no longer have to rely on mutable.
* Remove GetReadableParcel() interface from binder since we only sent
  one-way transaction and the output (readable) parcel is never used.
* Remove GetDataPosition() / SetDataPosition() interfaces since they are
  both unused.
* Some changes that should've been made to #27257 but was somehow
  missing...
pull/27344/head
Ta-Wei Tu 3 years ago committed by GitHub
parent fc16798c70
commit 8d5b93eacc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 10
      src/core/ext/transport/binder/server/binder_server.cc
  2. 16
      src/core/ext/transport/binder/wire_format/binder.h
  3. 38
      src/core/ext/transport/binder/wire_format/binder_android.cc
  4. 24
      src/core/ext/transport/binder/wire_format/binder_android.h
  5. 16
      src/core/ext/transport/binder/wire_format/wire_reader_impl.cc
  6. 6
      src/core/ext/transport/binder/wire_format/wire_reader_impl.h
  7. 44
      test/core/transport/binder/end2end/fake_binder.cc
  8. 27
      test/core/transport/binder/end2end/fake_binder.h
  9. 57
      test/core/transport/binder/end2end/fake_binder_test.cc
  10. 2
      test/core/transport/binder/end2end/testing_channel_create.cc
  11. 2
      test/core/transport/binder/mock_objects.cc
  12. 16
      test/core/transport/binder/mock_objects.h

@ -98,10 +98,10 @@ class BinderServerListener : public Server::ListenerInterface {
void Start(Server* /*server*/, void Start(Server* /*server*/,
const std::vector<grpc_pollset*>* /*pollsets*/) override { const std::vector<grpc_pollset*>* /*pollsets*/) override {
tx_receiver_ = factory_([this](transaction_code_t code, tx_receiver_ = factory_(
const grpc_binder::ReadableParcel* parcel) { [this](transaction_code_t code, grpc_binder::ReadableParcel* parcel) {
return OnSetupTransport(code, parcel); return OnSetupTransport(code, parcel);
}); });
endpoint_binder_ = tx_receiver_->GetRawBinder(); endpoint_binder_ = tx_receiver_->GetRawBinder();
grpc_add_endpoint_binder(addr_, endpoint_binder_); grpc_add_endpoint_binder(addr_, endpoint_binder_);
} }
@ -127,7 +127,7 @@ class BinderServerListener : public Server::ListenerInterface {
private: private:
absl::Status OnSetupTransport(transaction_code_t code, absl::Status OnSetupTransport(transaction_code_t code,
const grpc_binder::ReadableParcel* parcel) { grpc_binder::ReadableParcel* parcel) {
grpc_core::ExecCtx exec_ctx; grpc_core::ExecCtx exec_ctx;
if (grpc_binder::BinderTransportTxCode(code) != if (grpc_binder::BinderTransportTxCode(code) !=
grpc_binder::BinderTransportTxCode::SETUP_TRANSPORT) { grpc_binder::BinderTransportTxCode::SETUP_TRANSPORT) {

@ -44,9 +44,7 @@ class Binder;
class WritableParcel { class WritableParcel {
public: public:
virtual ~WritableParcel() = default; virtual ~WritableParcel() = default;
virtual int32_t GetDataPosition() const = 0;
virtual int32_t GetDataSize() const = 0; virtual int32_t GetDataSize() const = 0;
virtual absl::Status SetDataPosition(int32_t pos) = 0;
virtual absl::Status WriteInt32(int32_t data) = 0; virtual absl::Status WriteInt32(int32_t data) = 0;
virtual absl::Status WriteInt64(int64_t data) = 0; virtual absl::Status WriteInt64(int64_t data) = 0;
virtual absl::Status WriteBinder(HasRawBinder* binder) = 0; virtual absl::Status WriteBinder(HasRawBinder* binder) = 0;
@ -69,18 +67,17 @@ class ReadableParcel {
public: public:
virtual ~ReadableParcel() = default; virtual ~ReadableParcel() = default;
virtual int32_t GetDataSize() const = 0; virtual int32_t GetDataSize() const = 0;
virtual absl::Status ReadInt32(int32_t* data) const = 0; virtual absl::Status ReadInt32(int32_t* data) = 0;
virtual absl::Status ReadInt64(int64_t* data) const = 0; virtual absl::Status ReadInt64(int64_t* data) = 0;
virtual absl::Status ReadBinder(std::unique_ptr<Binder>* data) const = 0; virtual absl::Status ReadBinder(std::unique_ptr<Binder>* data) = 0;
// TODO(waynetu): Provide better interfaces. virtual absl::Status ReadByteArray(std::string* data) = 0;
virtual absl::Status ReadByteArray(std::string* data) const = 0; virtual absl::Status ReadString(std::string* str) = 0;
virtual absl::Status ReadString(std::string* str) const = 0;
}; };
class TransactionReceiver : public HasRawBinder { class TransactionReceiver : public HasRawBinder {
public: public:
using OnTransactCb = using OnTransactCb =
std::function<absl::Status(transaction_code_t, const ReadableParcel*)>; std::function<absl::Status(transaction_code_t, ReadableParcel*)>;
~TransactionReceiver() override = default; ~TransactionReceiver() override = default;
}; };
@ -96,7 +93,6 @@ class Binder : public HasRawBinder {
virtual absl::Status Transact(BinderTransportTxCode tx_code) = 0; virtual absl::Status Transact(BinderTransportTxCode tx_code) = 0;
virtual WritableParcel* GetWritableParcel() const = 0; virtual WritableParcel* GetWritableParcel() const = 0;
virtual ReadableParcel* GetReadableParcel() const = 0;
// TODO(waynetu): Can we decouple the receiver from the binder? // TODO(waynetu): Can we decouple the receiver from the binder?
virtual std::unique_ptr<TransactionReceiver> ConstructTxReceiver( virtual std::unique_ptr<TransactionReceiver> ConstructTxReceiver(

@ -215,11 +215,16 @@ absl::Status BinderAndroid::PrepareTransaction() {
absl::Status BinderAndroid::Transact(BinderTransportTxCode tx_code) { absl::Status BinderAndroid::Transact(BinderTransportTxCode tx_code) {
AIBinder* binder = binder_.get(); AIBinder* binder = binder_.get();
return AIBinder_transact(binder, static_cast<transaction_code_t>(tx_code), // We only do one-way transaction and thus the output parcel is never used.
&input_parcel_->parcel_, &output_parcel_->parcel_, AParcel* unused_output_parcel;
FLAG_ONEWAY) == STATUS_OK absl::Status result =
? absl::OkStatus() (AIBinder_transact(binder, static_cast<transaction_code_t>(tx_code),
: absl::InternalError("AIBinder_transact failed"); &input_parcel_->parcel_, &unused_output_parcel,
FLAG_ONEWAY) == STATUS_OK)
? absl::OkStatus()
: absl::InternalError("AIBinder_transact failed");
AParcel_delete(unused_output_parcel);
return result;
} }
std::unique_ptr<TransactionReceiver> BinderAndroid::ConstructTxReceiver( std::unique_ptr<TransactionReceiver> BinderAndroid::ConstructTxReceiver(
@ -229,10 +234,6 @@ std::unique_ptr<TransactionReceiver> BinderAndroid::ConstructTxReceiver(
transact_cb); transact_cb);
} }
int32_t WritableParcelAndroid::GetDataPosition() const {
return AParcel_getDataPosition(parcel_);
}
int32_t WritableParcelAndroid::GetDataSize() const { int32_t WritableParcelAndroid::GetDataSize() const {
if (AParcel_getDataSize) { if (AParcel_getDataSize) {
return AParcel_getDataSize(parcel_); return AParcel_getDataSize(parcel_);
@ -242,12 +243,6 @@ int32_t WritableParcelAndroid::GetDataSize() const {
} }
} }
absl::Status WritableParcelAndroid::SetDataPosition(int32_t pos) {
return AParcel_setDataPosition(parcel_, pos) == STATUS_OK
? absl::OkStatus()
: absl::InternalError("AParcel_setDataPosition failed");
}
absl::Status WritableParcelAndroid::WriteInt32(int32_t data) { absl::Status WritableParcelAndroid::WriteInt32(int32_t data) {
return AParcel_writeInt32(parcel_, data) == STATUS_OK return AParcel_writeInt32(parcel_, data) == STATUS_OK
? absl::OkStatus() ? absl::OkStatus()
@ -286,24 +281,23 @@ int32_t ReadableParcelAndroid::GetDataSize() const {
return AParcel_getDataSize(parcel_); return AParcel_getDataSize(parcel_);
} else { } else {
gpr_log(GPR_INFO, "[Warning] AParcel_getDataSize is not available"); gpr_log(GPR_INFO, "[Warning] AParcel_getDataSize is not available");
return -1; return 0;
} }
} }
absl::Status ReadableParcelAndroid::ReadInt32(int32_t* data) const { absl::Status ReadableParcelAndroid::ReadInt32(int32_t* data) {
return AParcel_readInt32(parcel_, data) == STATUS_OK return AParcel_readInt32(parcel_, data) == STATUS_OK
? absl::OkStatus() ? absl::OkStatus()
: absl::InternalError("AParcel_readInt32 failed"); : absl::InternalError("AParcel_readInt32 failed");
} }
absl::Status ReadableParcelAndroid::ReadInt64(int64_t* data) const { absl::Status ReadableParcelAndroid::ReadInt64(int64_t* data) {
return AParcel_readInt64(parcel_, data) == STATUS_OK return AParcel_readInt64(parcel_, data) == STATUS_OK
? absl::OkStatus() ? absl::OkStatus()
: absl::InternalError("AParcel_readInt64 failed"); : absl::InternalError("AParcel_readInt64 failed");
} }
absl::Status ReadableParcelAndroid::ReadBinder( absl::Status ReadableParcelAndroid::ReadBinder(std::unique_ptr<Binder>* data) {
std::unique_ptr<Binder>* data) const {
AIBinder* binder; AIBinder* binder;
if (AParcel_readStrongBinder(parcel_, &binder) != STATUS_OK) { if (AParcel_readStrongBinder(parcel_, &binder) != STATUS_OK) {
*data = nullptr; *data = nullptr;
@ -313,7 +307,7 @@ absl::Status ReadableParcelAndroid::ReadBinder(
return absl::OkStatus(); return absl::OkStatus();
} }
absl::Status ReadableParcelAndroid::ReadByteArray(std::string* data) const { absl::Status ReadableParcelAndroid::ReadByteArray(std::string* data) {
std::vector<uint8_t> vec; std::vector<uint8_t> vec;
if (AParcelReadVector(parcel_, &vec) == STATUS_OK) { if (AParcelReadVector(parcel_, &vec) == STATUS_OK) {
data->resize(vec.size()); data->resize(vec.size());
@ -325,7 +319,7 @@ absl::Status ReadableParcelAndroid::ReadByteArray(std::string* data) const {
return absl::InternalError("AParcel_readByteArray failed"); return absl::InternalError("AParcel_readByteArray failed");
} }
absl::Status ReadableParcelAndroid::ReadString(std::string* str) const { absl::Status ReadableParcelAndroid::ReadString(std::string* str) {
return AParcelReadString(parcel_, str) == STATUS_OK return AParcelReadString(parcel_, str) == STATUS_OK
? absl::OkStatus() ? absl::OkStatus()
: absl::InternalError("AParcel_readString failed"); : absl::InternalError("AParcel_readString failed");

@ -44,9 +44,7 @@ class WritableParcelAndroid final : public WritableParcel {
explicit WritableParcelAndroid(AParcel* parcel) : parcel_(parcel) {} explicit WritableParcelAndroid(AParcel* parcel) : parcel_(parcel) {}
~WritableParcelAndroid() override = default; ~WritableParcelAndroid() override = default;
int32_t GetDataPosition() const override;
int32_t GetDataSize() const override; int32_t GetDataSize() const override;
absl::Status SetDataPosition(int32_t pos) override;
absl::Status WriteInt32(int32_t data) override; absl::Status WriteInt32(int32_t data) override;
absl::Status WriteInt64(int64_t data) override; absl::Status WriteInt64(int64_t data) override;
absl::Status WriteBinder(HasRawBinder* binder) override; absl::Status WriteBinder(HasRawBinder* binder) override;
@ -63,19 +61,18 @@ class ReadableParcelAndroid final : public ReadableParcel {
public: public:
ReadableParcelAndroid() = default; ReadableParcelAndroid() = default;
// TODO(waynetu): Get rid of the const_cast. // TODO(waynetu): Get rid of the const_cast.
explicit ReadableParcelAndroid(const AParcel* parcel) explicit ReadableParcelAndroid(const AParcel* parcel) : parcel_(parcel) {}
: parcel_(const_cast<AParcel*>(parcel)) {}
~ReadableParcelAndroid() override = default; ~ReadableParcelAndroid() override = default;
int32_t GetDataSize() const override; int32_t GetDataSize() const override;
absl::Status ReadInt32(int32_t* data) const override; absl::Status ReadInt32(int32_t* data) override;
absl::Status ReadInt64(int64_t* data) const override; absl::Status ReadInt64(int64_t* data) override;
absl::Status ReadBinder(std::unique_ptr<Binder>* data) const override; absl::Status ReadBinder(std::unique_ptr<Binder>* data) override;
absl::Status ReadByteArray(std::string* data) const override; absl::Status ReadByteArray(std::string* data) override;
absl::Status ReadString(std::string* str) const override; absl::Status ReadString(std::string* str) override;
private: private:
AParcel* parcel_ = nullptr; const AParcel* parcel_ = nullptr;
friend class BinderAndroid; friend class BinderAndroid;
}; };
@ -84,8 +81,7 @@ class BinderAndroid final : public Binder {
public: public:
explicit BinderAndroid(ndk::SpAIBinder binder) explicit BinderAndroid(ndk::SpAIBinder binder)
: binder_(binder), : binder_(binder),
input_parcel_(absl::make_unique<WritableParcelAndroid>()), input_parcel_(absl::make_unique<WritableParcelAndroid>()) {}
output_parcel_(absl::make_unique<ReadableParcelAndroid>()) {}
~BinderAndroid() override = default; ~BinderAndroid() override = default;
void* GetRawBinder() override { return binder_.get(); } void* GetRawBinder() override { return binder_.get(); }
@ -97,9 +93,6 @@ class BinderAndroid final : public Binder {
WritableParcel* GetWritableParcel() const override { WritableParcel* GetWritableParcel() const override {
return input_parcel_.get(); return input_parcel_.get();
} }
ReadableParcel* GetReadableParcel() const override {
return output_parcel_.get();
};
std::unique_ptr<TransactionReceiver> ConstructTxReceiver( std::unique_ptr<TransactionReceiver> ConstructTxReceiver(
grpc_core::RefCountedPtr<WireReader> wire_reader_ref, grpc_core::RefCountedPtr<WireReader> wire_reader_ref,
@ -108,7 +101,6 @@ class BinderAndroid final : public Binder {
private: private:
ndk::SpAIBinder binder_; ndk::SpAIBinder binder_;
std::unique_ptr<WritableParcelAndroid> input_parcel_; std::unique_ptr<WritableParcelAndroid> input_parcel_;
std::unique_ptr<ReadableParcelAndroid> output_parcel_;
}; };
class TransactionReceiverAndroid final : public TransactionReceiver { class TransactionReceiverAndroid final : public TransactionReceiver {

@ -40,7 +40,7 @@
namespace grpc_binder { namespace grpc_binder {
namespace { namespace {
absl::StatusOr<Metadata> parse_metadata(const ReadableParcel* reader) { absl::StatusOr<Metadata> parse_metadata(ReadableParcel* reader) {
int num_header; int num_header;
RETURN_IF_ERROR(reader->ReadInt32(&num_header)); RETURN_IF_ERROR(reader->ReadInt32(&num_header));
gpr_log(GPR_INFO, "num_header = %d", num_header); gpr_log(GPR_INFO, "num_header = %d", num_header);
@ -109,14 +109,9 @@ void WireReaderImpl::SendSetupTransport(Binder* binder) {
gpr_log(GPR_INFO, "prepare transaction = %d", gpr_log(GPR_INFO, "prepare transaction = %d",
binder->PrepareTransaction().ok()); binder->PrepareTransaction().ok());
WritableParcel* writable_parcel = binder->GetWritableParcel(); WritableParcel* writable_parcel = binder->GetWritableParcel();
gpr_log(GPR_INFO, "data position = %d", writable_parcel->GetDataPosition());
// gpr_log(GPR_INFO, "set data position to 0 = %d",
// writer->SetDataPosition(0));
gpr_log(GPR_INFO, "data position = %d", writable_parcel->GetDataPosition());
int32_t version = 77; int32_t version = 77;
gpr_log(GPR_INFO, "write int32 = %d", gpr_log(GPR_INFO, "write int32 = %d",
writable_parcel->WriteInt32(version).ok()); writable_parcel->WriteInt32(version).ok());
gpr_log(GPR_INFO, "data position = %d", writable_parcel->GetDataPosition());
// The lifetime of the transaction receiver is the same as the wire writer's. // The lifetime of the transaction receiver is the same as the wire writer's.
// The transaction receiver is responsible for not calling the on-transact // The transaction receiver is responsible for not calling the on-transact
// callback when it's dead. // callback when it's dead.
@ -125,7 +120,7 @@ void WireReaderImpl::SendSetupTransport(Binder* binder) {
// callback owns a Ref() when it's being invoked. // callback owns a Ref() when it's being invoked.
tx_receiver_ = binder->ConstructTxReceiver( tx_receiver_ = binder->ConstructTxReceiver(
/*wire_reader_ref=*/Ref(), /*wire_reader_ref=*/Ref(),
[this](transaction_code_t code, const ReadableParcel* readable_parcel) { [this](transaction_code_t code, ReadableParcel* readable_parcel) {
return this->ProcessTransaction(code, readable_parcel); return this->ProcessTransaction(code, readable_parcel);
}); });
@ -146,7 +141,7 @@ std::unique_ptr<Binder> WireReaderImpl::RecvSetupTransport() {
} }
absl::Status WireReaderImpl::ProcessTransaction(transaction_code_t code, absl::Status WireReaderImpl::ProcessTransaction(transaction_code_t code,
const ReadableParcel* parcel) { ReadableParcel* parcel) {
gpr_log(GPR_INFO, __func__); gpr_log(GPR_INFO, __func__);
gpr_log(GPR_INFO, "tx code = %u", code); gpr_log(GPR_INFO, "tx code = %u", code);
if (code >= static_cast<unsigned>(kFirstCallId)) { if (code >= static_cast<unsigned>(kFirstCallId)) {
@ -225,7 +220,7 @@ absl::Status WireReaderImpl::ProcessTransaction(transaction_code_t code,
} }
absl::Status WireReaderImpl::ProcessStreamingTransaction( absl::Status WireReaderImpl::ProcessStreamingTransaction(
transaction_code_t code, const ReadableParcel* parcel) { transaction_code_t code, ReadableParcel* parcel) {
grpc_core::MutexLock lock(&mu_); grpc_core::MutexLock lock(&mu_);
if (!connected_) { if (!connected_) {
return absl::InvalidArgumentError("Transports not connected yet"); return absl::InvalidArgumentError("Transports not connected yet");
@ -267,8 +262,7 @@ absl::Status WireReaderImpl::ProcessStreamingTransaction(
} }
absl::Status WireReaderImpl::ProcessStreamingTransactionImpl( absl::Status WireReaderImpl::ProcessStreamingTransactionImpl(
transaction_code_t code, const ReadableParcel* parcel, transaction_code_t code, ReadableParcel* parcel, int* cancellation_flags) {
int* cancellation_flags) {
GPR_ASSERT(cancellation_flags); GPR_ASSERT(cancellation_flags);
num_incoming_bytes_ += parcel->GetDataSize(); num_incoming_bytes_ += parcel->GetDataSize();

@ -67,7 +67,7 @@ class WireReaderImpl : public WireReader {
std::unique_ptr<Binder> binder) override; std::unique_ptr<Binder> binder) override;
absl::Status ProcessTransaction(transaction_code_t code, absl::Status ProcessTransaction(transaction_code_t code,
const ReadableParcel* parcel); ReadableParcel* parcel);
/// Send SETUP_TRANSPORT request through \p binder. /// Send SETUP_TRANSPORT request through \p binder.
/// ///
@ -93,9 +93,9 @@ class WireReaderImpl : public WireReader {
private: private:
absl::Status ProcessStreamingTransaction(transaction_code_t code, absl::Status ProcessStreamingTransaction(transaction_code_t code,
const ReadableParcel* parcel); ReadableParcel* parcel);
absl::Status ProcessStreamingTransactionImpl(transaction_code_t code, absl::Status ProcessStreamingTransactionImpl(transaction_code_t code,
const ReadableParcel* parcel, ReadableParcel* parcel,
int* cancellation_flags) int* cancellation_flags)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(mu_); ABSL_EXCLUSIVE_LOCKS_REQUIRED(mu_);

@ -24,59 +24,42 @@ namespace end2end_testing {
TransactionProcessor* g_transaction_processor = nullptr; TransactionProcessor* g_transaction_processor = nullptr;
FakeWritableParcel::FakeWritableParcel() : data_(1) {}
int32_t FakeWritableParcel::GetDataPosition() const { return data_position_; }
int32_t FakeWritableParcel::GetDataSize() const { return data_size_; } int32_t FakeWritableParcel::GetDataSize() const { return data_size_; }
absl::Status FakeWritableParcel::SetDataPosition(int32_t pos) {
if (data_.size() < static_cast<size_t>(pos) + 1) {
data_.resize(pos + 1);
}
data_position_ = pos;
return absl::OkStatus();
}
absl::Status FakeWritableParcel::WriteInt32(int32_t data) { absl::Status FakeWritableParcel::WriteInt32(int32_t data) {
data_[data_position_] = data; data_.push_back(data);
SetDataPosition(data_position_ + 1).IgnoreError(); data_size_ += sizeof(int32_t);
data_size_ += 4;
return absl::OkStatus(); return absl::OkStatus();
} }
absl::Status FakeWritableParcel::WriteInt64(int64_t data) { absl::Status FakeWritableParcel::WriteInt64(int64_t data) {
data_[data_position_] = data; data_.push_back(data);
SetDataPosition(data_position_ + 1).IgnoreError(); data_size_ += sizeof(int64_t);
data_size_ += 8;
return absl::OkStatus(); return absl::OkStatus();
} }
absl::Status FakeWritableParcel::WriteBinder(HasRawBinder* binder) { absl::Status FakeWritableParcel::WriteBinder(HasRawBinder* binder) {
data_[data_position_] = binder->GetRawBinder(); data_.push_back(binder->GetRawBinder());
SetDataPosition(data_position_ + 1).IgnoreError(); data_size_ += sizeof(void*);
data_size_ += 8;
return absl::OkStatus(); return absl::OkStatus();
} }
absl::Status FakeWritableParcel::WriteString(absl::string_view s) { absl::Status FakeWritableParcel::WriteString(absl::string_view s) {
data_[data_position_] = std::string(s); data_.push_back(std::string(s));
SetDataPosition(data_position_ + 1).IgnoreError();
data_size_ += s.size(); data_size_ += s.size();
return absl::OkStatus(); return absl::OkStatus();
} }
absl::Status FakeWritableParcel::WriteByteArray(const int8_t* buffer, absl::Status FakeWritableParcel::WriteByteArray(const int8_t* buffer,
int32_t length) { int32_t length) {
data_[data_position_] = std::vector<int8_t>(buffer, buffer + length); data_.push_back(std::vector<int8_t>(buffer, buffer + length));
SetDataPosition(data_position_ + 1).IgnoreError();
data_size_ += length; data_size_ += length;
return absl::OkStatus(); return absl::OkStatus();
} }
int32_t FakeReadableParcel::GetDataSize() const { return data_size_; } int32_t FakeReadableParcel::GetDataSize() const { return data_size_; }
absl::Status FakeReadableParcel::ReadInt32(int32_t* data) const { absl::Status FakeReadableParcel::ReadInt32(int32_t* data) {
if (data_position_ >= data_.size() || if (data_position_ >= data_.size() ||
!absl::holds_alternative<int32_t>(data_[data_position_])) { !absl::holds_alternative<int32_t>(data_[data_position_])) {
return absl::InternalError("ReadInt32 failed"); return absl::InternalError("ReadInt32 failed");
@ -85,7 +68,7 @@ absl::Status FakeReadableParcel::ReadInt32(int32_t* data) const {
return absl::OkStatus(); return absl::OkStatus();
} }
absl::Status FakeReadableParcel::ReadInt64(int64_t* data) const { absl::Status FakeReadableParcel::ReadInt64(int64_t* data) {
if (data_position_ >= data_.size() || if (data_position_ >= data_.size() ||
!absl::holds_alternative<int64_t>(data_[data_position_])) { !absl::holds_alternative<int64_t>(data_[data_position_])) {
return absl::InternalError("ReadInt64 failed"); return absl::InternalError("ReadInt64 failed");
@ -94,8 +77,7 @@ absl::Status FakeReadableParcel::ReadInt64(int64_t* data) const {
return absl::OkStatus(); return absl::OkStatus();
} }
absl::Status FakeReadableParcel::ReadBinder( absl::Status FakeReadableParcel::ReadBinder(std::unique_ptr<Binder>* data) {
std::unique_ptr<Binder>* data) const {
if (data_position_ >= data_.size() || if (data_position_ >= data_.size() ||
!absl::holds_alternative<void*>(data_[data_position_])) { !absl::holds_alternative<void*>(data_[data_position_])) {
return absl::InternalError("ReadBinder failed"); return absl::InternalError("ReadBinder failed");
@ -106,7 +88,7 @@ absl::Status FakeReadableParcel::ReadBinder(
return absl::OkStatus(); return absl::OkStatus();
} }
absl::Status FakeReadableParcel::ReadString(std::string* str) const { absl::Status FakeReadableParcel::ReadString(std::string* str) {
if (data_position_ >= data_.size() || if (data_position_ >= data_.size() ||
!absl::holds_alternative<std::string>(data_[data_position_])) { !absl::holds_alternative<std::string>(data_[data_position_])) {
return absl::InternalError("ReadString failed"); return absl::InternalError("ReadString failed");
@ -115,7 +97,7 @@ absl::Status FakeReadableParcel::ReadString(std::string* str) const {
return absl::OkStatus(); return absl::OkStatus();
} }
absl::Status FakeReadableParcel::ReadByteArray(std::string* data) const { absl::Status FakeReadableParcel::ReadByteArray(std::string* data) {
if (data_position_ >= data_.size() || if (data_position_ >= data_.size() ||
!absl::holds_alternative<std::vector<int8_t>>(data_[data_position_])) { !absl::holds_alternative<std::vector<int8_t>>(data_[data_position_])) {
return absl::InternalError("ReadByteArray failed"); return absl::InternalError("ReadByteArray failed");

@ -82,10 +82,7 @@ using FakeData = std::vector<
// MoveData(). // MoveData().
class FakeWritableParcel final : public WritableParcel { class FakeWritableParcel final : public WritableParcel {
public: public:
FakeWritableParcel();
int32_t GetDataPosition() const override;
int32_t GetDataSize() const override; int32_t GetDataSize() const override;
absl::Status SetDataPosition(int32_t pos) override;
absl::Status WriteInt32(int32_t data) override; absl::Status WriteInt32(int32_t data) override;
absl::Status WriteInt64(int64_t data) override; absl::Status WriteInt64(int64_t data) override;
absl::Status WriteBinder(HasRawBinder* binder) override; absl::Status WriteBinder(HasRawBinder* binder) override;
@ -96,7 +93,6 @@ class FakeWritableParcel final : public WritableParcel {
private: private:
FakeData data_; FakeData data_;
size_t data_position_ = 0;
int32_t data_size_ = 0; int32_t data_size_ = 0;
}; };
@ -109,11 +105,11 @@ class FakeReadableParcel final : public ReadableParcel {
explicit FakeReadableParcel(FakeData data) : data_(std::move(data)) { explicit FakeReadableParcel(FakeData data) : data_(std::move(data)) {
for (auto& d : data_) { for (auto& d : data_) {
if (absl::holds_alternative<int32_t>(d)) { if (absl::holds_alternative<int32_t>(d)) {
data_size_ += 4; data_size_ += sizeof(int32_t);
} else if (absl::holds_alternative<int64_t>(d)) { } else if (absl::holds_alternative<int64_t>(d)) {
data_size_ += 8; data_size_ += sizeof(int64_t);
} else if (absl::holds_alternative<void*>(d)) { } else if (absl::holds_alternative<void*>(d)) {
data_size_ += 8; data_size_ += sizeof(void*);
} else if (absl::holds_alternative<std::string>(d)) { } else if (absl::holds_alternative<std::string>(d)) {
data_size_ += absl::get<std::string>(d).size(); data_size_ += absl::get<std::string>(d).size();
} else { } else {
@ -123,15 +119,15 @@ class FakeReadableParcel final : public ReadableParcel {
} }
int32_t GetDataSize() const override; int32_t GetDataSize() const override;
absl::Status ReadInt32(int32_t* data) const override; absl::Status ReadInt32(int32_t* data) override;
absl::Status ReadInt64(int64_t* data) const override; absl::Status ReadInt64(int64_t* data) override;
absl::Status ReadBinder(std::unique_ptr<Binder>* data) const override; absl::Status ReadBinder(std::unique_ptr<Binder>* data) override;
absl::Status ReadByteArray(std::string* data) const override; absl::Status ReadByteArray(std::string* data) override;
absl::Status ReadString(std::string* str) const override; absl::Status ReadString(std::string* str) override;
private: private:
const FakeData data_; const FakeData data_;
mutable size_t data_position_ = 0; size_t data_position_ = 0;
int32_t data_size_ = 0; int32_t data_size_ = 0;
}; };
@ -186,8 +182,7 @@ class PersistentFakeTransactionReceiver {
TransactionReceiver::OnTransactCb cb, TransactionReceiver::OnTransactCb cb,
std::unique_ptr<FakeBinderTunnel> tunnel); std::unique_ptr<FakeBinderTunnel> tunnel);
absl::Status Receive(BinderTransportTxCode tx_code, absl::Status Receive(BinderTransportTxCode tx_code, ReadableParcel* parcel) {
const ReadableParcel* parcel) {
return callback_(static_cast<transaction_code_t>(tx_code), parcel); return callback_(static_cast<transaction_code_t>(tx_code), parcel);
} }
@ -215,7 +210,6 @@ class FakeBinder final : public Binder {
absl::Status Transact(BinderTransportTxCode tx_code) override; absl::Status Transact(BinderTransportTxCode tx_code) override;
WritableParcel* GetWritableParcel() const override { return input_.get(); } WritableParcel* GetWritableParcel() const override { return input_.get(); }
ReadableParcel* GetReadableParcel() const override { return output_.get(); }
std::unique_ptr<TransactionReceiver> ConstructTxReceiver( std::unique_ptr<TransactionReceiver> ConstructTxReceiver(
grpc_core::RefCountedPtr<WireReader> wire_reader_ref, grpc_core::RefCountedPtr<WireReader> wire_reader_ref,
@ -226,7 +220,6 @@ class FakeBinder final : public Binder {
private: private:
FakeEndpoint* endpoint_; FakeEndpoint* endpoint_;
std::unique_ptr<FakeWritableParcel> input_; std::unique_ptr<FakeWritableParcel> input_;
std::unique_ptr<FakeReadableParcel> output_;
}; };
// A transaction processor. // A transaction processor.

@ -29,29 +29,6 @@
namespace grpc_binder { namespace grpc_binder {
namespace end2end_testing { namespace end2end_testing {
TEST(FakeBinderTestWithoutTransaction, WritableParcelDataPosition) {
std::unique_ptr<WritableParcel> parcel =
absl::make_unique<FakeWritableParcel>();
EXPECT_EQ(parcel->GetDataPosition(), 0);
EXPECT_TRUE(parcel->WriteInt32(0).ok());
EXPECT_EQ(parcel->GetDataPosition(), 1);
EXPECT_TRUE(parcel->WriteInt32(1).ok());
EXPECT_TRUE(parcel->WriteInt32(2).ok());
EXPECT_EQ(parcel->GetDataPosition(), 3);
EXPECT_TRUE(parcel->WriteString("").ok());
EXPECT_EQ(parcel->GetDataPosition(), 4);
EXPECT_TRUE(parcel->SetDataPosition(0).ok());
const char kBuffer[] = "test";
EXPECT_TRUE(parcel
->WriteByteArray(reinterpret_cast<const int8_t*>(kBuffer),
strlen(kBuffer))
.ok());
EXPECT_EQ(parcel->GetDataPosition(), 1);
EXPECT_TRUE(parcel->SetDataPosition(100).ok());
EXPECT_EQ(parcel->GetDataPosition(), 100);
}
namespace { namespace {
class FakeBinderTest : public ::testing::TestWithParam<absl::Duration> { class FakeBinderTest : public ::testing::TestWithParam<absl::Duration> {
@ -70,8 +47,8 @@ TEST_P(FakeBinderTest, SendInt32) {
int called = 0; int called = 0;
std::unique_ptr<Binder> sender; std::unique_ptr<Binder> sender;
std::unique_ptr<TransactionReceiver> tx_receiver; std::unique_ptr<TransactionReceiver> tx_receiver;
std::tie(sender, tx_receiver) = NewBinderPair( std::tie(sender, tx_receiver) =
[&](transaction_code_t tx_code, const ReadableParcel* parcel) { NewBinderPair([&](transaction_code_t tx_code, ReadableParcel* parcel) {
EXPECT_EQ(tx_code, kTxCode); EXPECT_EQ(tx_code, kTxCode);
int value = 0; int value = 0;
EXPECT_TRUE(parcel->ReadInt32(&value).ok()); EXPECT_TRUE(parcel->ReadInt32(&value).ok());
@ -95,8 +72,8 @@ TEST_P(FakeBinderTest, SendString) {
int called = 0; int called = 0;
std::unique_ptr<Binder> sender; std::unique_ptr<Binder> sender;
std::unique_ptr<TransactionReceiver> tx_receiver; std::unique_ptr<TransactionReceiver> tx_receiver;
std::tie(sender, tx_receiver) = NewBinderPair( std::tie(sender, tx_receiver) =
[&](transaction_code_t tx_code, const ReadableParcel* parcel) { NewBinderPair([&](transaction_code_t tx_code, ReadableParcel* parcel) {
EXPECT_EQ(tx_code, kTxCode); EXPECT_EQ(tx_code, kTxCode);
std::string value; std::string value;
EXPECT_TRUE(parcel->ReadString(&value).ok()); EXPECT_TRUE(parcel->ReadString(&value).ok());
@ -120,8 +97,8 @@ TEST_P(FakeBinderTest, SendByteArray) {
int called = 0; int called = 0;
std::unique_ptr<Binder> sender; std::unique_ptr<Binder> sender;
std::unique_ptr<TransactionReceiver> tx_receiver; std::unique_ptr<TransactionReceiver> tx_receiver;
std::tie(sender, tx_receiver) = NewBinderPair( std::tie(sender, tx_receiver) =
[&](transaction_code_t tx_code, const ReadableParcel* parcel) { NewBinderPair([&](transaction_code_t tx_code, ReadableParcel* parcel) {
EXPECT_EQ(tx_code, kTxCode); EXPECT_EQ(tx_code, kTxCode);
std::string value; std::string value;
EXPECT_TRUE(parcel->ReadByteArray(&value).ok()); EXPECT_TRUE(parcel->ReadByteArray(&value).ok());
@ -150,8 +127,8 @@ TEST_P(FakeBinderTest, SendMultipleItems) {
int called = 0; int called = 0;
std::unique_ptr<Binder> sender; std::unique_ptr<Binder> sender;
std::unique_ptr<TransactionReceiver> tx_receiver; std::unique_ptr<TransactionReceiver> tx_receiver;
std::tie(sender, tx_receiver) = NewBinderPair( std::tie(sender, tx_receiver) =
[&](transaction_code_t tx_code, const ReadableParcel* parcel) { NewBinderPair([&](transaction_code_t tx_code, ReadableParcel* parcel) {
int value_result; int value_result;
EXPECT_EQ(tx_code, kTxCode); EXPECT_EQ(tx_code, kTxCode);
EXPECT_TRUE(parcel->ReadInt32(&value_result).ok()); EXPECT_TRUE(parcel->ReadInt32(&value_result).ok());
@ -186,8 +163,8 @@ TEST_P(FakeBinderTest, SendBinder) {
int called = 0; int called = 0;
std::unique_ptr<Binder> sender; std::unique_ptr<Binder> sender;
std::unique_ptr<TransactionReceiver> tx_receiver; std::unique_ptr<TransactionReceiver> tx_receiver;
std::tie(sender, tx_receiver) = NewBinderPair( std::tie(sender, tx_receiver) =
[&](transaction_code_t tx_code, const ReadableParcel* parcel) { NewBinderPair([&](transaction_code_t tx_code, ReadableParcel* parcel) {
EXPECT_EQ(tx_code, kTxCode); EXPECT_EQ(tx_code, kTxCode);
std::unique_ptr<Binder> binder; std::unique_ptr<Binder> binder;
EXPECT_TRUE(parcel->ReadBinder(&binder).ok()); EXPECT_TRUE(parcel->ReadBinder(&binder).ok());
@ -202,8 +179,7 @@ TEST_P(FakeBinderTest, SendBinder) {
int called2 = 0; int called2 = 0;
std::unique_ptr<TransactionReceiver> tx_receiver2 = std::unique_ptr<TransactionReceiver> tx_receiver2 =
absl::make_unique<FakeTransactionReceiver>( absl::make_unique<FakeTransactionReceiver>(
nullptr, nullptr, [&](transaction_code_t tx_code, ReadableParcel* parcel) {
[&](transaction_code_t tx_code, const ReadableParcel* parcel) {
int value; int value;
EXPECT_TRUE(parcel->ReadInt32(&value).ok()); EXPECT_TRUE(parcel->ReadInt32(&value).ok());
EXPECT_EQ(value, kValue); EXPECT_EQ(value, kValue);
@ -228,8 +204,8 @@ TEST_P(FakeBinderTest, SendTransactionAfterDestruction) {
int called = 0; int called = 0;
{ {
std::unique_ptr<TransactionReceiver> tx_receiver; std::unique_ptr<TransactionReceiver> tx_receiver;
std::tie(sender, tx_receiver) = NewBinderPair( std::tie(sender, tx_receiver) =
[&](transaction_code_t tx_code, const ReadableParcel* parcel) { NewBinderPair([&](transaction_code_t tx_code, ReadableParcel* parcel) {
EXPECT_EQ(tx_code, kTxCode); EXPECT_EQ(tx_code, kTxCode);
int value; int value;
EXPECT_TRUE(parcel->ReadInt32(&value).ok()); EXPECT_TRUE(parcel->ReadInt32(&value).ok());
@ -299,10 +275,9 @@ TEST_P(FakeBinderTest, StressTest) {
std::unique_ptr<TransactionReceiver> tx_receiver; std::unique_ptr<TransactionReceiver> tx_receiver;
int expected_tx_code = th_arg->tx_code; int expected_tx_code = th_arg->tx_code;
std::vector<std::vector<int>>* cnt = th_arg->global_cnts; std::vector<std::vector<int>>* cnt = th_arg->global_cnts;
std::tie(binder, tx_receiver) = std::tie(binder, tx_receiver) = NewBinderPair(
NewBinderPair([tid, p, cnt, expected_tx_code]( [tid, p, cnt, expected_tx_code](transaction_code_t tx_code,
transaction_code_t tx_code, ReadableParcel* parcel) mutable {
const ReadableParcel* parcel) mutable {
EXPECT_EQ(tx_code, expected_tx_code); EXPECT_EQ(tx_code, expected_tx_code);
int value; int value;
EXPECT_TRUE(parcel->ReadInt32(&value).ok()); EXPECT_TRUE(parcel->ReadInt32(&value).ok());

@ -35,7 +35,7 @@ class ServerSetupTransportHelper {
: wire_reader_(absl::make_unique<WireReaderImpl>( : wire_reader_(absl::make_unique<WireReaderImpl>(
/*transport_stream_receiver=*/nullptr, /*is_client=*/false)) { /*transport_stream_receiver=*/nullptr, /*is_client=*/false)) {
std::tie(endpoint_binder_, tx_receiver_) = NewBinderPair( std::tie(endpoint_binder_, tx_receiver_) = NewBinderPair(
[this](transaction_code_t tx_code, const ReadableParcel* parcel) { [this](transaction_code_t tx_code, ReadableParcel* parcel) {
return this->wire_reader_->ProcessTransaction(tx_code, parcel); return this->wire_reader_->ProcessTransaction(tx_code, parcel);
}); });
} }

@ -33,7 +33,6 @@ MockReadableParcel::MockReadableParcel() {
} }
MockWritableParcel::MockWritableParcel() { MockWritableParcel::MockWritableParcel() {
ON_CALL(*this, SetDataPosition).WillByDefault(Return(absl::OkStatus()));
ON_CALL(*this, WriteInt32).WillByDefault(Return(absl::OkStatus())); ON_CALL(*this, WriteInt32).WillByDefault(Return(absl::OkStatus()));
ON_CALL(*this, WriteBinder).WillByDefault(Return(absl::OkStatus())); ON_CALL(*this, WriteBinder).WillByDefault(Return(absl::OkStatus()));
ON_CALL(*this, WriteString).WillByDefault(Return(absl::OkStatus())); ON_CALL(*this, WriteString).WillByDefault(Return(absl::OkStatus()));
@ -44,7 +43,6 @@ MockBinder::MockBinder() {
ON_CALL(*this, PrepareTransaction).WillByDefault(Return(absl::OkStatus())); ON_CALL(*this, PrepareTransaction).WillByDefault(Return(absl::OkStatus()));
ON_CALL(*this, Transact).WillByDefault(Return(absl::OkStatus())); ON_CALL(*this, Transact).WillByDefault(Return(absl::OkStatus()));
ON_CALL(*this, GetWritableParcel).WillByDefault(Return(&mock_input_)); ON_CALL(*this, GetWritableParcel).WillByDefault(Return(&mock_input_));
ON_CALL(*this, GetReadableParcel).WillByDefault(Return(&mock_output_));
ON_CALL(*this, ConstructTxReceiver) ON_CALL(*this, ConstructTxReceiver)
.WillByDefault( .WillByDefault(
[this](grpc_core::RefCountedPtr<WireReader> /*wire_reader_ref*/, [this](grpc_core::RefCountedPtr<WireReader> /*wire_reader_ref*/,

@ -26,9 +26,7 @@ namespace grpc_binder {
class MockWritableParcel : public WritableParcel { class MockWritableParcel : public WritableParcel {
public: public:
MOCK_METHOD(int32_t, GetDataPosition, (), (const, override));
MOCK_METHOD(int32_t, GetDataSize, (), (const, override)); MOCK_METHOD(int32_t, GetDataSize, (), (const, override));
MOCK_METHOD(absl::Status, SetDataPosition, (int32_t), (override));
MOCK_METHOD(absl::Status, WriteInt32, (int32_t), (override)); MOCK_METHOD(absl::Status, WriteInt32, (int32_t), (override));
MOCK_METHOD(absl::Status, WriteInt64, (int64_t), (override)); MOCK_METHOD(absl::Status, WriteInt64, (int64_t), (override));
MOCK_METHOD(absl::Status, WriteBinder, (HasRawBinder*), (override)); MOCK_METHOD(absl::Status, WriteBinder, (HasRawBinder*), (override));
@ -42,12 +40,11 @@ class MockWritableParcel : public WritableParcel {
class MockReadableParcel : public ReadableParcel { class MockReadableParcel : public ReadableParcel {
public: public:
MOCK_METHOD(int32_t, GetDataSize, (), (const, override)); MOCK_METHOD(int32_t, GetDataSize, (), (const, override));
MOCK_METHOD(absl::Status, ReadInt32, (int32_t*), (const, override)); MOCK_METHOD(absl::Status, ReadInt32, (int32_t*), (override));
MOCK_METHOD(absl::Status, ReadInt64, (int64_t*), (const, override)); MOCK_METHOD(absl::Status, ReadInt64, (int64_t*), (override));
MOCK_METHOD(absl::Status, ReadBinder, (std::unique_ptr<Binder>*), MOCK_METHOD(absl::Status, ReadBinder, (std::unique_ptr<Binder>*), (override));
(const, override)); MOCK_METHOD(absl::Status, ReadByteArray, (std::string*), (override));
MOCK_METHOD(absl::Status, ReadByteArray, (std::string*), (const, override)); MOCK_METHOD(absl::Status, ReadString, (std::string*), (override));
MOCK_METHOD(absl::Status, ReadString, (std::string*), (const, override));
MockReadableParcel(); MockReadableParcel();
}; };
@ -58,7 +55,6 @@ class MockBinder : public Binder {
MOCK_METHOD(absl::Status, PrepareTransaction, (), (override)); MOCK_METHOD(absl::Status, PrepareTransaction, (), (override));
MOCK_METHOD(absl::Status, Transact, (BinderTransportTxCode), (override)); MOCK_METHOD(absl::Status, Transact, (BinderTransportTxCode), (override));
MOCK_METHOD(WritableParcel*, GetWritableParcel, (), (const, override)); MOCK_METHOD(WritableParcel*, GetWritableParcel, (), (const, override));
MOCK_METHOD(ReadableParcel*, GetReadableParcel, (), (const, override));
MOCK_METHOD(std::unique_ptr<TransactionReceiver>, ConstructTxReceiver, MOCK_METHOD(std::unique_ptr<TransactionReceiver>, ConstructTxReceiver,
(grpc_core::RefCountedPtr<WireReader>, (grpc_core::RefCountedPtr<WireReader>,
TransactionReceiver::OnTransactCb), TransactionReceiver::OnTransactCb),
@ -80,7 +76,7 @@ class MockTransactionReceiver : public TransactionReceiver {
public: public:
explicit MockTransactionReceiver(OnTransactCb transact_cb, explicit MockTransactionReceiver(OnTransactCb transact_cb,
BinderTransportTxCode code, BinderTransportTxCode code,
const ReadableParcel* output) { ReadableParcel* output) {
transact_cb(static_cast<transaction_code_t>(code), output).IgnoreError(); transact_cb(static_cast<transaction_code_t>(code), output).IgnoreError();
} }

Loading…
Cancel
Save