Fix issue with multiple Cancellations for same target ID.

PiperOrigin-RevId: 726234081
This commit is contained in:
Francis Tsui
2025-02-12 16:08:37 -08:00
committed by Copybara-Service
parent 2587c6abad
commit 1d3b17821b
8 changed files with 19 additions and 35 deletions
-7
View File
@@ -150,13 +150,6 @@ void FakeNearbySharingService::Cancel(
status_codes_callback(StatusCodes::kOk);
}
// Returns true if the local user cancelled the transfer to remote
// |share_target|.
bool FakeNearbySharingService::DidLocalUserCancelTransfer(
int64_t share_target_id) {
return false;
}
std::string FakeNearbySharingService::Dump() const { return ""; }
NearbyShareSettings* FakeNearbySharingService::GetSettings() { return nullptr; }
-4
View File
@@ -105,10 +105,6 @@ class FakeNearbySharingService : public NearbySharingService {
std::function<void(StatusCodes status_codes)>
status_codes_callback) override;
// Returns true if the local user cancelled the transfer to remote
// |share_target|.
bool DidLocalUserCancelTransfer(int64_t share_target_id) override;
std::string Dump() const override;
NearbyShareSettings* GetSettings() override;
-4
View File
@@ -227,10 +227,6 @@ class NearbySharingService {
int64_t share_target_id,
std::function<void(StatusCodes status_codes)> status_codes_callback) = 0;
// Returns true if the local user cancelled the transfer to remote
// |share_target|.
virtual bool DidLocalUserCancelTransfer(int64_t share_target_id) = 0;
// Checks to make sure visibility setting is valid and updates the service's
// visibility if so.
virtual void SetVisibility(
+5 -14
View File
@@ -344,7 +344,6 @@ void NearbySharingServiceImpl::Cleanup() {
last_incoming_metadata_.reset();
last_outgoing_metadata_.reset();
locally_cancelled_share_target_ids_.clear();
is_scanning_ = false;
is_transferring_ = false;
@@ -890,12 +889,6 @@ void NearbySharingServiceImpl::Cancel(
[this, share_target_id,
status_codes_callback = std::move(status_codes_callback)]() {
LOG(INFO) << __func__ << ": User canceled transfer";
if (locally_cancelled_share_target_ids_.contains(share_target_id)) {
LOG(WARNING) << __func__ << ": Cancel is called again.";
status_codes_callback(StatusCodes::kOutOfOrderApiCall);
return;
}
locally_cancelled_share_target_ids_.insert(share_target_id);
DoCancel(share_target_id, std::move(status_codes_callback),
/*is_initiator_of_cancellation=*/true);
});
@@ -922,7 +915,11 @@ void NearbySharingServiceImpl::DoCancel(
// cancellation signals. Also, note that there might not be any ongoing
// payload transfer, for example, if a connection has not been established
// yet.
session->CancelPayloads();
if (!session->CancelPayloads()) {
// If session has already been cancelled, report success.
status_codes_callback(StatusCodes::kOk);
return;
}
// Inform the user that the transfer has been cancelled before disconnecting
// because subsequent disconnections might be interpreted as failure.
@@ -969,12 +966,6 @@ void NearbySharingServiceImpl::DoCancel(
std::move(status_codes_callback)(StatusCodes::kOk);
}
bool NearbySharingServiceImpl::DidLocalUserCancelTransfer(
int64_t share_target_id) {
return absl::c_linear_search(locally_cancelled_share_target_ids_,
share_target_id);
}
void NearbySharingServiceImpl::SetVisibility(
proto::DeviceVisibility visibility, absl::Duration expiration,
absl::AnyInvocable<void(StatusCodes status_code) &&> callback) {
-3
View File
@@ -157,7 +157,6 @@ class NearbySharingServiceImpl
void Cancel(int64_t share_target_id,
std::function<void(StatusCodes status_codes)>
status_codes_callback) override;
bool DidLocalUserCancelTransfer(int64_t share_target_id) override;
void SetVisibility(
proto::DeviceVisibility visibility, absl::Duration expiration,
absl::AnyInvocable<void(StatusCodes status_code) &&> callback) override;
@@ -531,8 +530,6 @@ class NearbySharingServiceImpl
// A map of Endpoint id to DiscoveryCacheEntry.
// All ShareTargets in discovery cache have received_disabled set to true.
absl::flat_hash_map<std::string, DiscoveryCacheEntry> discovery_cache_;
// The IDs of ShareTargets that we cancelled the transfer to.
absl::flat_hash_set<int64_t> locally_cancelled_share_target_ids_;
// A map from endpoint ID to endpoint info from discovered, contact-based
// advertisements that could not decrypt any available public certificates.
// During discovery, if certificates are downloaded, we revisit this map and
+7 -1
View File
@@ -211,10 +211,16 @@ void ShareSession::SetAttachmentPayloadId(int64_t attachment_id,
attachment_payload_map_[attachment_id] = payload_id;
}
void ShareSession::CancelPayloads() {
bool ShareSession::CancelPayloads() {
if (is_cancelled_) {
LOG(INFO) << __func__ << ": Share session is already cancelled.";
return false;
}
for (const auto& [attachment_id, payload_id] : attachment_payload_map_) {
connections_manager_.Cancel(payload_id);
}
is_cancelled_ = true;
return true;
}
void ShareSession::WriteFrame(const Frame& frame) {
+3 -1
View File
@@ -119,7 +119,8 @@ class ShareSession {
return attachment_container_;
}
void CancelPayloads();
// Returns false if session is already cancelled, otherwise returns true.
bool CancelPayloads();
const absl::flat_hash_map<int64_t, int64_t>& attachment_payload_map() const {
return attachment_payload_map_;
@@ -215,6 +216,7 @@ class ShareSession {
AttachmentContainer attachment_container_;
absl::flat_hash_map<int64_t, int64_t> attachment_payload_map_;
PayloadTracker::PayloadUpdateQueue* payload_updates_queue_ = nullptr;
bool is_cancelled_ = false;
};
} // namespace nearby::sharing
+4 -1
View File
@@ -264,10 +264,13 @@ TEST(ShareSessionTest, CancelPayloads) {
session.SetAttachmentPayloadId(1, 2);
session.SetAttachmentPayloadId(3, 4);
session.CancelPayloads();
EXPECT_TRUE(session.CancelPayloads());
EXPECT_TRUE(session.connections_manager().WasPayloadCanceled(2));
EXPECT_TRUE(session.connections_manager().WasPayloadCanceled(4));
// Repeated calls to CancelPayloads returns false.
EXPECT_FALSE(session.CancelPayloads());
}
TEST(ShareSessionTest, WriteResponseFrame) {