Skip to content

Commit

Permalink
Move disconnection timeout into OutgoingShareSession.
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 707655553
  • Loading branch information
@ftsui @copybara-github
ftsui authored and copybara-github committed Dec 22, 2024
1 parent 29492d3 commit 2e54912
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 63 deletions.
57 changes: 16 additions & 41 deletions sharing/nearby_sharing_service_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,6 @@ void NearbySharingServiceImpl::Cleanup() {
last_outgoing_metadata_.reset();
locally_cancelled_share_target_ids_.clear();

disconnection_timeout_alarms_.clear();

is_scanning_ = false;
is_transferring_ = false;
is_receiving_files_ = false;
Expand Down Expand Up @@ -510,14 +508,11 @@ void NearbySharingServiceImpl::RegisterSendSurface(

// Let newly registered send surface catch up with discovered share
// targets from current scanning session.
// if (is_scanning_) {
for (const auto& item : outgoing_share_target_map_) {
LOG(INFO) << "Reporting discovered target "
<< item.second.ToString()
<< " when registering send surface";
wrapped_callback.OnShareTargetDiscovered(item.second);
}
// }
for (const auto& item : outgoing_share_target_map_) {
LOG(INFO) << "Reporting discovered target " << item.second.ToString()
<< " when registering send surface";
wrapped_callback.OnShareTargetDiscovered(item.second);
}

// Set Share Start time for Foreground Send Surfaces
if (state == SendSurfaceState::kForeground) {
Expand Down Expand Up @@ -818,8 +813,8 @@ void NearbySharingServiceImpl::Accept(
GetIncomingShareSession(share_target_id);
if (incoming_session != nullptr) {
// Incoming session.
bool accept_success = incoming_session->AcceptTransfer(
absl::bind_front(
bool accept_success =
incoming_session->AcceptTransfer(absl::bind_front(
&NearbySharingServiceImpl::OnIncomingPayloadTransferUpdates,
this, share_target_id));
std::move(status_codes_callback)(
Expand Down Expand Up @@ -1747,7 +1742,6 @@ void NearbySharingServiceImpl::OnOutgoingDecryptedCertificate(
absl::string_view endpoint_id, absl::Span<const uint8_t> endpoint_info,
const Advertisement& advertisement,
std::optional<NearbyShareDecryptedPublicCertificate> certificate) {

// The certificate provides the device name, in order to create a ShareTarget
// to represent this remote device.
std::optional<ShareTarget> share_target =
Expand Down Expand Up @@ -1776,7 +1770,7 @@ void NearbySharingServiceImpl::OnOutgoingDecryptedCertificate(
}
if (FindDuplicateInOutgoingShareTargets(endpoint_id, *share_target)) {
DeduplicateInOutgoingShareTarget(*share_target, endpoint_id,
std::move(certificate));
std::move(certificate));
FinishEndpointDiscoveryEvent();
return;
}
Expand Down Expand Up @@ -2872,10 +2866,9 @@ void NearbySharingServiceImpl::OnStorageCheckCompleted(
}
// Don't need to wait for user to accept for Self share.
LOG(INFO) << __func__ << ": Auto-accepting self share.";
session.AcceptTransfer(
absl::bind_front(
&NearbySharingServiceImpl::OnIncomingPayloadTransferUpdates, this,
session.share_target().id));
session.AcceptTransfer(absl::bind_front(
&NearbySharingServiceImpl::OnIncomingPayloadTransferUpdates, this,
session.share_target().id));
OnTransferStarted(/*is_incoming=*/true);
}

Expand Down Expand Up @@ -3004,9 +2997,8 @@ void NearbySharingServiceImpl::OnIncomingPayloadTransferUpdates(
}

if (metadata->status() == TransferMetadata::Status::kIncompletePayloads) {
OnIncomingFilesMetadataUpdated(share_target_id,
std::move(*metadata),
/*success=*/false);
OnIncomingFilesMetadataUpdated(share_target_id, std::move(*metadata),
/*success=*/false);
return;
}
if (metadata->status() == TransferMetadata::Status::kComplete) {
Expand Down Expand Up @@ -3091,24 +3083,8 @@ void NearbySharingServiceImpl::OnOutgoingPayloadTransferUpdates(
<< TransferMetadata::StatusToString(metadata->status());
}

// When kComplete is received from PayloadTracker, we need to wait for
// receiver to close connection before we know that the transfer has
// succeeded. Here we delay the kComplete update until receiver disconnects.
// A 1 min timer is setup so that if we do not receive disconnect from
// receiver, we assume the transfer has failed.
if (metadata->status() == TransferMetadata::Status::kComplete) {
session->DelayCompleteMetadata(*metadata);
auto timer = std::make_unique<ThreadTimer>(
*service_thread_, "disconnection_timeout_alarm",
kOutgoingDisconnectionDelay,
[this, share_target_id = session->share_target().id]() {
OutgoingShareSession* session =
GetOutgoingShareSession(share_target_id);
if (session != nullptr) {
session->DisconnectionTimeout();
}
});
disconnection_timeout_alarms_[session->endpoint_id()] = std::move(timer);
session->DelayComplete(*metadata);
return;
}
// Make sure to call this before calling Disconnect, or we risk losing some
Expand Down Expand Up @@ -3293,8 +3269,7 @@ std::optional<ShareTarget>
NearbySharingServiceImpl::RemoveOutgoingShareTargetWithEndpointId(
absl::string_view endpoint_id) {
VLOG(1) << __func__ << ":Outgoing connection to " << endpoint_id
<< " disconnected, cancel disconnection timer";
disconnection_timeout_alarms_.erase(endpoint_id);
<< " disconnected";
auto target_node = outgoing_share_target_map_.extract(endpoint_id);
if (target_node.empty()) {
LOG(WARNING) << __func__ << ": endpoint_id=" << endpoint_id
Expand Down Expand Up @@ -3373,7 +3348,7 @@ void NearbySharingServiceImpl::MoveToDiscoveryCache(std::string endpoint_id,
auto [it, inserted] =
discovery_cache_.insert_or_assign(endpoint_id, std::move(cache_entry));
LOG(INFO) << "[Dedupped] added to discovery_cache: " << endpoint_id << " by "
<< (inserted ? "insert" : "assign");
<< (inserted ? "insert" : "assign");
}

void NearbySharingServiceImpl::CreateOutgoingShareSession(
Expand Down
5 changes: 0 additions & 5 deletions sharing/nearby_sharing_service_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -543,11 +543,6 @@ class NearbySharingServiceImpl
// unnecessary backend API call.
absl::flat_hash_set<std::string> discovered_advertisements_retried_set_;

// A map of ShareTarget id to disconnection timeout callback. Used to only
// disconnect after a timeout to keep sending any pending payloads.
absl::flat_hash_map<std::string, std::unique_ptr<ThreadTimer>>
disconnection_timeout_alarms_;

// The current advertising power level. PowerLevel::kUnknown while not
// advertising.
PowerLevel advertising_power_level_ = PowerLevel::kUnknown;
Expand Down
16 changes: 9 additions & 7 deletions sharing/outgoing_share_session.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ std::optional<Payload> OutgoingShareSession::ExtractNextPayload() {
return std::nullopt;
}

void OutgoingShareSession::DelayCompleteMetadata(
void OutgoingShareSession::DelayComplete(
const TransferMetadata& complete_metadata) {
LOG(INFO)
<< "Delay complete notification until receiver disconnects for target "
Expand All @@ -478,12 +478,14 @@ void OutgoingShareSession::DelayCompleteMetadata(
TransferMetadataBuilder::Clone(complete_metadata);
builder.set_status(TransferMetadata::Status::kInProgress);
UpdateTransferMetadata(builder.build());
}

void OutgoingShareSession::DisconnectionTimeout() {
VLOG(1) << "Disconnection delay timeed out for target " << share_target().id;
pending_complete_metadata_.reset();
Disconnect();
disconnection_timeout_ = std::make_unique<ThreadTimer>(
service_thread(), "disconnection_timeout_alarm",
kOutgoingDisconnectionDelay, [this]() {
VLOG(1) << "Disconnection delay timed out for target "
<< share_target().id;
pending_complete_metadata_.reset();
Disconnect();
});
}

void OutgoingShareSession::UpdateSessionForDedup(
Expand Down
14 changes: 9 additions & 5 deletions sharing/outgoing_share_session.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,13 @@ class OutgoingShareSession : public ShareSession {
// Called when all payloads have been sent.
void SendAttachmentsCompleted(const TransferMetadata& metadata);

// Cache the kComplete metadata in pending_complete_metadata_ and forward a
// modified copy that changes kComplete into kInProgress.
void DelayCompleteMetadata(const TransferMetadata& complete_metadata);
// Disconnect timeout expired without receiving client disconnect.
void DisconnectionTimeout();
// Wait for receiver to close connection before we notify that the transfer
// has succeeded. Here we delay the `complete_metadata` update until receiver
// disconnects and forward a modified copy that changes kComplete into
// kInProgress.
// A 1 min timer is setup so that if we do not receive disconnect from
// receiver, we assume the transfer has failed.
void DelayComplete(const TransferMetadata& complete_metadata);
// Used only for OutgoingShareSession De-duplication.
void UpdateSessionForDedup(
const ShareTarget& share_target,
Expand Down Expand Up @@ -183,6 +185,8 @@ class OutgoingShareSession : public ShareSession {
std::unique_ptr<ThreadTimer> mutual_acceptance_timeout_;
std::optional<TransferMetadata> pending_complete_metadata_;
absl::Time connection_start_time_;
// Timeout waiting for remote disconnect in order to complete transfer.
std::unique_ptr<ThreadTimer> disconnection_timeout_;
};

} // namespace nearby::sharing
Expand Down
13 changes: 8 additions & 5 deletions sharing/outgoing_share_session_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,7 @@ TEST_F(OutgoingShareSessionTest, ProcessKeyVerificationResultSuccess) {
EXPECT_THAT(session_.os_type(), Eq(OSType::WINDOWS));
}

TEST_F(OutgoingShareSessionTest, DelayCompleteMetadataReceiverDisconnect) {
TEST_F(OutgoingShareSessionTest, DelayCompleteReceiverDisconnect) {
NearbyConnectionImpl connection(device_info_);
session_.set_session_id(1234);
ConnectionSuccess(&connection);
Expand All @@ -826,14 +826,14 @@ TEST_F(OutgoingShareSessionTest, DelayCompleteMetadataReceiverDisconnect) {
EXPECT_CALL(transfer_metadata_callback_,
Call(_, HasStatus(TransferMetadata::Status::kInProgress)));

session_.DelayCompleteMetadata(complete_metadata);
session_.DelayComplete(complete_metadata);

EXPECT_CALL(transfer_metadata_callback_,
Call(_, HasStatus(TransferMetadata::Status::kComplete)));
session_.OnDisconnect();
}

TEST_F(OutgoingShareSessionTest, DelayCompleteMetadataDisconnectTimeout) {
TEST_F(OutgoingShareSessionTest, DelayCompleteDisconnectTimeout) {
NearbyConnectionImpl connection(device_info_);
session_.set_session_id(1234);
std::vector<uint8_t> endpoint_info = {1, 2, 3, 4};
Expand All @@ -854,9 +854,12 @@ TEST_F(OutgoingShareSessionTest, DelayCompleteMetadataDisconnectTimeout) {
EXPECT_CALL(transfer_metadata_callback_,
Call(_, HasStatus(TransferMetadata::Status::kInProgress)));

session_.DelayCompleteMetadata(complete_metadata);
session_.DelayComplete(complete_metadata);
// Fast forward to the disconnection timeout.
fake_clock_.FastForward(absl::Seconds(60));
fake_task_runner_.SyncWithTimeout(absl::Milliseconds(100));

session_.DisconnectionTimeout();
// session_.DisconnectionTimeout();
// Verify that connection is closed.
EXPECT_THAT(
connections_manager_.connection_endpoint_info(kEndpointId).has_value(),
Expand Down

0 comments on commit 2e54912

Please sign in to comment.