Skip to content

Conversation

TaWeiTu
Copy link
Contributor

@TaWeiTu TaWeiTu commented Sep 6, 2021

Depends on #27243.

@TaWeiTu TaWeiTu added the release notes: no Indicates if PR should not be in release notes label Sep 6, 2021
@TaWeiTu TaWeiTu requested a review from sifmelcara September 6, 2021 14:17
@TaWeiTu TaWeiTu force-pushed the flow-control-data-size branch 2 times, most recently from e0484e3 to 29ea4de Compare September 7, 2021 08:46
@TaWeiTu TaWeiTu force-pushed the flow-control-data-size branch from 29ea4de to 4c7395d Compare September 7, 2021 10:24
@TaWeiTu
Copy link
Contributor Author

TaWeiTu commented Sep 8, 2021

Ping.

@TaWeiTu TaWeiTu force-pushed the flow-control-data-size branch 4 times, most recently from 9eac98a to b8b0300 Compare September 9, 2021 04:16
return AParcel_getDataSize(parcel_);
} else {
gpr_log(GPR_INFO, "[Warning] AParcel_getDataSize is not available");
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns 0 but ReadableParcelAndroid::GetDataSize return -1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Both return 0.

@@ -39,34 +41,41 @@ absl::Status FakeWritableParcel::SetDataPosition(int32_t pos) {
absl::Status FakeWritableParcel::WriteInt32(int32_t data) {
data_[data_position_] = data;
SetDataPosition(data_position_ + 1).IgnoreError();
data_size_ += 4;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably use size_of(int32_t) instead of hardcoding the value? Same for other types

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@TaWeiTu TaWeiTu force-pushed the flow-control-data-size branch 3 times, most recently from bb8a6ed to ca9119a Compare September 9, 2021 14:32
@@ -253,6 +253,7 @@ absl::Status WireReaderImpl::ProcessStreamingTransaction(
absl::Status WireReaderImpl::ProcessStreamingTransactionImpl(
transaction_code_t code, const ReadableParcel* parcel,
int* cancellation_flags) {
num_incoming_bytes_ += parcel->GetDataSize();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put it below GPR_ASSERT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@TaWeiTu TaWeiTu force-pushed the flow-control-data-size branch 5 times, most recently from 8a3bd94 to 918e4e1 Compare September 11, 2021 01:57
@TaWeiTu TaWeiTu force-pushed the flow-control-data-size branch from 918e4e1 to ca3ae75 Compare September 11, 2021 09:41
@TaWeiTu TaWeiTu merged commit 71ceae7 into grpc:master Sep 11, 2021
TaWeiTu added a commit to TaWeiTu/grpc that referenced this pull request Sep 11, 2021
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 grpc#27257 but was somehow
  missing...
TaWeiTu added a commit to TaWeiTu/grpc that referenced this pull request Sep 13, 2021
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 grpc#27257 but was somehow
  missing...
TaWeiTu added a commit to TaWeiTu/grpc that referenced this pull request Sep 13, 2021
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 grpc#27257 but was somehow
  missing...
TaWeiTu added a commit to TaWeiTu/grpc that referenced this pull request Sep 14, 2021
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 grpc#27257 but was somehow
  missing...
TaWeiTu added a commit that referenced this pull request Sep 14, 2021
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...
lidizheng pushed a commit to lidizheng/grpc that referenced this pull request Sep 23, 2021
lidizheng pushed a commit to lidizheng/grpc that referenced this pull request Sep 23, 2021
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 grpc#27257 but was somehow
  missing...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/core release notes: no Indicates if PR should not be in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants