Commit 87071b64 authored by Richard Knoll's avatar Richard Knoll Committed by Commit Bot

[Nearby] Filter status updates after final status

This will filter any further TransferMetadata updates afterr receiving a
final status one.

Bug: 1085067
Change-Id: I75fa89ad57f61cf81ed7738c6b646218f3823e32
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2368598Reviewed-by: default avatarAlex Chau <alexchau@chromium.org>
Commit-Queue: Richard Knoll <knollr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800722}
parent d4364ffe
......@@ -192,6 +192,35 @@ FileAttachment FileAttachmentFromPath(const base::FilePath path) {
/*size=*/0, path, mime_type);
}
// Wraps a call to OnTransferUpdate() to filter any updates after receiving a
// final status.
class TransferUpdateDecorator : public TransferUpdateCallback {
public:
using Callback = base::RepeatingCallback<void(const ShareTarget&,
const TransferMetadata&)>;
explicit TransferUpdateDecorator(Callback callback)
: callback_(std::move(callback)) {}
TransferUpdateDecorator(const TransferUpdateDecorator&) = delete;
TransferUpdateDecorator& operator=(const TransferUpdateDecorator&) = delete;
~TransferUpdateDecorator() override = default;
void OnTransferUpdate(const ShareTarget& share_target,
const TransferMetadata& transfer_metadata) override {
if (got_final_status_) {
// If we already got a final status, we can ignore any subsequent final
// statuses caused by race conditions.
return;
}
got_final_status_ = transfer_metadata.is_final_status();
callback_.Run(share_target, transfer_metadata);
}
private:
bool got_final_status_ = false;
Callback callback_;
};
} // namespace
NearbySharingServiceImpl::NearbySharingServiceImpl(
......@@ -500,12 +529,13 @@ void NearbySharingServiceImpl::Accept(
void NearbySharingServiceImpl::Reject(
const ShareTarget& share_target,
StatusCodesCallback status_codes_callback) {
NearbyConnection* connection = GetConnection(share_target);
if (!connection) {
ShareTargetInfo* info = GetShareTargetInfo(share_target);
if (!info || !info->connection()) {
NS_LOG(WARNING) << __func__ << ": Reject invoked for unknown share target";
std::move(status_codes_callback).Run(StatusCodes::kOutOfOrderApiCall);
return;
}
NearbyConnection* connection = info->connection();
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
......@@ -521,10 +551,12 @@ void NearbySharingServiceImpl::Reject(
NS_LOG(VERBOSE) << __func__
<< ": Successfully wrote a rejection response frame";
OnIncomingTransferUpdate(share_target,
TransferMetadataBuilder()
if (info->transfer_update_callback()) {
info->transfer_update_callback()->OnTransferUpdate(
share_target, TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kRejected)
.build());
}
std::move(status_codes_callback).Run(StatusCodes::kOk);
}
......@@ -1216,7 +1248,7 @@ NearbySharingService::StatusCodes NearbySharingServiceImpl::StopScanning() {
void NearbySharingServiceImpl::OnIncomingTransferUpdate(
const ShareTarget& share_target,
TransferMetadata metadata) {
const TransferMetadata& metadata) {
if (metadata.status() != TransferMetadata::Status::kCancelled &&
metadata.status() != TransferMetadata::Status::kRejected) {
last_incoming_metadata_ =
......@@ -1246,7 +1278,7 @@ void NearbySharingServiceImpl::OnIncomingTransferUpdate(
void NearbySharingServiceImpl::OnOutgoingTransferUpdate(
const ShareTarget& share_target,
TransferMetadata metadata) {
const TransferMetadata& metadata) {
if (metadata.is_final_status()) {
is_connecting_ = false;
OnTransferComplete();
......@@ -1314,6 +1346,14 @@ NearbySharingService::StatusCodes NearbySharingServiceImpl::ReceivePayloads(
}
NearbyConnection* connection = info->connection();
if (!info->transfer_update_callback()) {
NS_LOG(WARNING) << __func__
<< ": Accept invoked for share target without transfer "
"update callback. Disconnecting.";
connection->Close();
return StatusCodes::kOutOfOrderApiCall;
}
// TODO(himanshujaju) - Implement payload tracker.
for (int64_t attachment_id : share_target.GetAttachmentIds()) {
......@@ -1337,7 +1377,7 @@ NearbySharingService::StatusCodes NearbySharingServiceImpl::ReceivePayloads(
WriteResponse(*connection, sharing::nearby::ConnectionResponseFrame::ACCEPT);
NS_LOG(VERBOSE) << __func__ << ": Successfully wrote response frame";
OnIncomingTransferUpdate(
info->transfer_update_callback()->OnTransferUpdate(
share_target,
TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kAwaitingRemoteAcceptance)
......@@ -1367,8 +1407,14 @@ NearbySharingService::StatusCodes NearbySharingServiceImpl::SendPayloads(
NS_LOG(WARNING) << "Failed to send payload due to missing connection.";
return StatusCodes::kOutOfOrderApiCall;
}
if (!info->transfer_update_callback()) {
NS_LOG(WARNING) << "Failed to send payload due to missing transfer update "
"callback. Disconnecting.";
info->connection()->Close();
return StatusCodes::kOutOfOrderApiCall;
}
OnOutgoingTransferUpdate(
info->transfer_update_callback()->OnTransferUpdate(
share_target,
TransferMetadataBuilder()
.set_token(info->token())
......@@ -1376,8 +1422,8 @@ NearbySharingService::StatusCodes NearbySharingServiceImpl::SendPayloads(
.build());
if (!info->endpoint_id()) {
OnOutgoingTransferUpdate(share_target,
TransferMetadataBuilder()
info->transfer_update_callback()->OnTransferUpdate(
share_target, TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kFailed)
.build());
info->connection()->Close();
......@@ -1422,13 +1468,17 @@ NearbySharingService::StatusCodes NearbySharingServiceImpl::SendAttachments(
return StatusCodes::kError;
}
info->set_transfer_update_callback(std::make_unique<TransferUpdateDecorator>(
base::BindRepeating(&NearbySharingServiceImpl::OnOutgoingTransferUpdate,
weak_ptr_factory_.GetWeakPtr())));
OnTransferStarted(/*is_incoming=*/false);
is_connecting_ = true;
InvalidateSendSurfaceState();
// Send process initialized successfully, from now on status updated will be
// sent out via OnOutgoingTransferUpdate().
OnOutgoingTransferUpdate(
info->transfer_update_callback()->OnTransferUpdate(
share_target, TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kConnecting)
.build());
......@@ -1452,11 +1502,13 @@ void NearbySharingServiceImpl::OnCreatePayloads(
NS_LOG(WARNING) << __func__
<< ": Failed to send file to remote ShareTarget. Failed to "
"create payloads.";
OnOutgoingTransferUpdate(
if (info && info->transfer_update_callback()) {
info->transfer_update_callback()->OnTransferUpdate(
share_target,
TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kMediaUnavailable)
.build());
}
return;
}
......@@ -1483,10 +1535,12 @@ void NearbySharingServiceImpl::OnOutgoingConnection(
NS_LOG(WARNING) << __func__
<< ": Failed to initate connection to share target "
<< share_target.device_name;
OnOutgoingTransferUpdate(share_target,
TransferMetadataBuilder()
if (info->transfer_update_callback()) {
info->transfer_update_callback()->OnTransferUpdate(
share_target, TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kFailed)
.build());
}
return;
}
......@@ -1518,12 +1572,20 @@ void NearbySharingServiceImpl::SendIntroduction(
NS_LOG(VERBOSE) << __func__ << ": Preparing to send introduction to "
<< share_target.device_name;
NearbyConnection* connection = GetConnection(share_target);
if (!connection) {
ShareTargetInfo* info = GetShareTargetInfo(share_target);
if (!info || !info->connection()) {
NS_LOG(WARNING) << __func__ << ": No NearbyConnection tied to "
<< share_target.device_name;
return;
}
NearbyConnection* connection = info->connection();
if (!info->transfer_update_callback()) {
connection->Close();
NS_LOG(WARNING) << __func__
<< ": No transfer update callback, disconnecting.";
return;
}
if (!foreground_send_transfer_callbacks_.might_have_observers() &&
!background_send_transfer_callbacks_.might_have_observers()) {
......@@ -1572,8 +1634,8 @@ void NearbySharingServiceImpl::SendIntroduction(
introduction->text_metadata_size() == 0) {
NS_LOG(WARNING) << __func__
<< ": No payloads tied to transfer, disconnecting.";
OnOutgoingTransferUpdate(share_target,
TransferMetadataBuilder()
info->transfer_update_callback()->OnTransferUpdate(
share_target, TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kFailed)
.build());
connection->Close();
......@@ -1600,7 +1662,7 @@ void NearbySharingServiceImpl::SendIntroduction(
FROM_HERE, base::BindOnce(mutual_acceptance_timeout_alarm_.callback()),
kReadResponseFrameTimeout);
OnOutgoingTransferUpdate(
info->transfer_update_callback()->OnTransferUpdate(
share_target,
TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kAwaitingLocalConfirmation)
......@@ -1712,11 +1774,12 @@ void NearbySharingServiceImpl::WriteResponse(
void NearbySharingServiceImpl::Fail(const ShareTarget& share_target,
TransferMetadata::Status status) {
NearbyConnection* connection = GetConnection(share_target);
if (!connection) {
ShareTargetInfo* info = GetShareTargetInfo(share_target);
if (!info || !info->connection()) {
NS_LOG(WARNING) << __func__ << ": Fail invoked for unknown share target.";
return;
}
NearbyConnection* connection = info->connection();
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
......@@ -1752,8 +1815,8 @@ void NearbySharingServiceImpl::Fail(const ShareTarget& share_target,
WriteResponse(*connection, response_status);
if (incoming_share_target_info_map_.count(share_target.id)) {
OnIncomingTransferUpdate(
if (info->transfer_update_callback()) {
info->transfer_update_callback()->OnTransferUpdate(
share_target, TransferMetadataBuilder().set_status(status).build());
}
}
......@@ -1832,6 +1895,11 @@ void NearbySharingServiceImpl::OnIncomingDecryptedCertificate(
DCHECK(share_target_info);
share_target_info->set_connection(connection);
share_target_info->set_transfer_update_callback(
std::make_unique<TransferUpdateDecorator>(base::BindRepeating(
&NearbySharingServiceImpl::OnIncomingTransferUpdate,
weak_ptr_factory_.GetWeakPtr())));
connection->SetDisconnectionListener(
base::BindOnce(&NearbySharingServiceImpl::UnregisterShareTarget,
weak_ptr_factory_.GetWeakPtr(), *share_target));
......@@ -1937,6 +2005,13 @@ void NearbySharingServiceImpl::OnOutgoingConnectionKeyVerificationDone(
if (!info || !info->connection())
return;
if (!info->transfer_update_callback()) {
NS_LOG(VERBOSE) << __func__
<< ": No transfer update callback. Disconnecting.";
info->connection()->Close();
return;
}
// TODO(crbug.com/1119279): Check if we need to set this to false for
// Advanced Protection users.
bool sender_skips_confirmation = true;
......@@ -1945,7 +2020,7 @@ void NearbySharingServiceImpl::OnOutgoingConnectionKeyVerificationDone(
case PairedKeyVerificationRunner::PairedKeyVerificationResult::kFail:
NS_LOG(VERBOSE) << __func__ << ": Paired key handshake failed for target "
<< share_target.device_name << ". Disconnecting.";
OnOutgoingTransferUpdate(
info->transfer_update_callback()->OnTransferUpdate(
share_target, TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kFailed)
.build());
......@@ -1992,11 +2067,15 @@ void NearbySharingServiceImpl::OnOutgoingConnectionKeyVerificationDone(
void NearbySharingServiceImpl::RefreshUIOnDisconnection(
ShareTarget share_target) {
OnIncomingTransferUpdate(
ShareTargetInfo* info = GetShareTargetInfo(share_target);
if (info->transfer_update_callback()) {
info->transfer_update_callback()->OnTransferUpdate(
share_target,
TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kAwaitingRemoteAcceptanceFailed)
.set_status(
TransferMetadata::Status::kAwaitingRemoteAcceptanceFailed)
.build());
}
UnregisterShareTarget(share_target);
}
......@@ -2146,12 +2225,19 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse(
}
NearbyConnection* connection = info->connection();
if (!info->transfer_update_callback()) {
NS_LOG(WARNING) << __func__
<< ": No transfer update callback. Disconnecting.";
connection->Close();
return;
}
if (!frame) {
NS_LOG(WARNING)
<< __func__
<< ": Failed to read a response from the remote device. Disconnecting.";
OnOutgoingTransferUpdate(share_target,
TransferMetadataBuilder()
info->transfer_update_callback()->OnTransferUpdate(
share_target, TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kFailed)
.build());
connection->Close();
......@@ -2171,7 +2257,7 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse(
&NearbySharingServiceImpl::OnFrameRead,
weak_ptr_factory_.GetWeakPtr(), std::move(share_target)));
OnOutgoingTransferUpdate(
info->transfer_update_callback()->OnTransferUpdate(
share_target, TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kInProgress)
.build());
......@@ -2194,7 +2280,7 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse(
break;
}
case sharing::mojom::ConnectionResponseFrame::Status::kReject:
OnOutgoingTransferUpdate(
info->transfer_update_callback()->OnTransferUpdate(
share_target, TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kRejected)
.build());
......@@ -2204,7 +2290,7 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse(
<< ": The connection was rejected. The connection has been closed.";
break;
case sharing::mojom::ConnectionResponseFrame::Status::kNotEnoughSpace:
OnOutgoingTransferUpdate(
info->transfer_update_callback()->OnTransferUpdate(
share_target,
TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kNotEnoughSpace)
......@@ -2218,7 +2304,7 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse(
break;
case sharing::mojom::ConnectionResponseFrame::Status::
kUnsupportedAttachmentType:
OnOutgoingTransferUpdate(
info->transfer_update_callback()->OnTransferUpdate(
share_target,
TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kUnsupportedAttachmentType)
......@@ -2231,7 +2317,7 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse(
"connection has been closed.";
break;
case sharing::mojom::ConnectionResponseFrame::Status::kTimedOut:
OnOutgoingTransferUpdate(
info->transfer_update_callback()->OnTransferUpdate(
share_target, TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kTimedOut)
.build());
......@@ -2242,7 +2328,7 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse(
"timed out. The connection has been closed.";
break;
default:
OnOutgoingTransferUpdate(
info->transfer_update_callback()->OnTransferUpdate(
share_target, TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kFailed)
.build());
......@@ -2274,6 +2360,13 @@ void NearbySharingServiceImpl::OnStorageCheckCompleted(
}
NearbyConnection* connection = info->connection();
if (!info->transfer_update_callback()) {
connection->Close();
NS_LOG(VERBOSE) << __func__
<< ": No transfer update callback. Disconnecting.";
return;
}
mutual_acceptance_timeout_alarm_.Reset(base::BindOnce(
&NearbySharingServiceImpl::OnIncomingMutualAcceptanceTimeout,
weak_ptr_factory_.GetWeakPtr(), share_target));
......@@ -2282,7 +2375,7 @@ void NearbySharingServiceImpl::OnStorageCheckCompleted(
FROM_HERE, base::BindOnce(mutual_acceptance_timeout_alarm_.callback()),
kReadResponseFrameTimeout);
OnIncomingTransferUpdate(
info->transfer_update_callback()->OnTransferUpdate(
share_target,
TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kAwaitingLocalConfirmation)
......@@ -2362,20 +2455,27 @@ void NearbySharingServiceImpl::HandleCertificateInfoFrame(
void NearbySharingServiceImpl::OnIncomingConnectionDisconnected(
const ShareTarget& share_target) {
OnIncomingTransferUpdate(share_target,
TransferMetadataBuilder()
ShareTargetInfo* info = GetShareTargetInfo(share_target);
if (info->transfer_update_callback()) {
info->transfer_update_callback()->OnTransferUpdate(
share_target, TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kFailed)
.build());
}
UnregisterShareTarget(share_target);
}
void NearbySharingServiceImpl::OnOutgoingConnectionDisconnected(
const ShareTarget& share_target) {
OnOutgoingTransferUpdate(
ShareTargetInfo* info = GetShareTargetInfo(share_target);
if (info->transfer_update_callback()) {
info->transfer_update_callback()->OnTransferUpdate(
share_target,
TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kAwaitingRemoteAcceptanceFailed)
.set_status(
TransferMetadata::Status::kAwaitingRemoteAcceptanceFailed)
.build());
}
UnregisterShareTarget(share_target);
}
......@@ -2400,14 +2500,19 @@ void NearbySharingServiceImpl::OnOutgoingMutualAcceptanceTimeout(
<< ": Outgoing mutual acceptance timed out, closing connection for "
<< share_target.device_name;
OnOutgoingTransferUpdate(share_target,
TransferMetadataBuilder()
ShareTargetInfo* info = GetShareTargetInfo(share_target);
if (!info)
return;
if (info->transfer_update_callback()) {
info->transfer_update_callback()->OnTransferUpdate(
share_target, TransferMetadataBuilder()
.set_status(TransferMetadata::Status::kTimedOut)
.build());
}
NearbyConnection* connection = GetConnection(share_target);
if (connection)
connection->Close();
if (info->connection())
info->connection()->Close();
}
base::Optional<ShareTarget> NearbySharingServiceImpl::CreateShareTarget(
......
......@@ -193,9 +193,9 @@ class NearbySharingServiceImpl
ShareTarget placeholder_share_target,
sharing::mojom::AdvertisementPtr advertisement);
void OnIncomingTransferUpdate(const ShareTarget& share_target,
TransferMetadata metadata);
const TransferMetadata& metadata);
void OnOutgoingTransferUpdate(const ShareTarget& share_target,
TransferMetadata metadata);
const TransferMetadata& metadata);
void CloseConnection(const ShareTarget& share_target);
void OnIncomingDecryptedCertificate(
const std::string& endpoint_id,
......
......@@ -2359,11 +2359,9 @@ TEST_F(NearbySharingServiceImplTest, SendText_FailedKeyVerification) {
DiscoverShareTarget(transfer_callback, discovery_callback);
base::RunLoop run_loop;
ExpectTransferUpdates(
transfer_callback, target,
{TransferMetadata::Status::kConnecting, TransferMetadata::Status::kFailed,
// TODO(crbug.com/1085067): Filter updates after the first final one.
TransferMetadata::Status::kAwaitingRemoteAcceptanceFailed},
ExpectTransferUpdates(transfer_callback, target,
{TransferMetadata::Status::kConnecting,
TransferMetadata::Status::kFailed},
run_loop.QuitClosure());
SetUpKeyVerification(/*is_incoming=*/false,
......@@ -2429,11 +2427,7 @@ TEST_P(NearbySharingServiceImplSendFailureTest, SendText_RemoteFailure) {
// We're now waiting for the remote device to respond with the accept result.
base::RunLoop reject_run_loop;
ExpectTransferUpdates(
transfer_callback, target,
{GetParam().expected_status,
// TODO(crbug.com/1085067): Filter updates after the first final one.
TransferMetadata::Status::kAwaitingRemoteAcceptanceFailed},
ExpectTransferUpdates(transfer_callback, target, {GetParam().expected_status},
reject_run_loop.QuitClosure());
// Cancel the transfer by rejecting it.
......@@ -2472,11 +2466,7 @@ TEST_P(NearbySharingServiceImplSendFailureTest, SendFiles_RemoteFailure) {
// We're now waiting for the remote device to respond with the accept result.
base::RunLoop reject_run_loop;
ExpectTransferUpdates(
transfer_callback, target,
{GetParam().expected_status,
// TODO(crbug.com/1085067): Filter updates after the first final one.
TransferMetadata::Status::kAwaitingRemoteAcceptanceFailed},
ExpectTransferUpdates(transfer_callback, target, {GetParam().expected_status},
reject_run_loop.QuitClosure());
// Cancel the transfer by rejecting it.
......
......@@ -13,6 +13,7 @@
#include "chrome/browser/nearby_sharing/incoming_frames_reader.h"
#include "chrome/browser/nearby_sharing/paired_key_verification_runner.h"
#include "chrome/browser/nearby_sharing/payload_tracker.h"
#include "chrome/browser/nearby_sharing/transfer_update_callback.h"
class NearbyConnection;
......@@ -47,6 +48,15 @@ class ShareTargetInfo {
connection_ = connection;
}
TransferUpdateCallback* transfer_update_callback() const {
return transfer_update_callback_.get();
}
void set_transfer_update_callback(
std::unique_ptr<TransferUpdateCallback> transfer_update_callback) {
transfer_update_callback_ = std::move(transfer_update_callback);
}
const base::Optional<std::string>& token() const { return token_; }
void set_token(std::string token) { token_ = std::move(token); }
......@@ -76,6 +86,7 @@ class ShareTargetInfo {
base::Optional<std::string> endpoint_id_;
base::Optional<NearbyShareDecryptedPublicCertificate> certificate_;
NearbyConnection* connection_ = nullptr;
std::unique_ptr<TransferUpdateCallback> transfer_update_callback_;
base::Optional<std::string> token_;
std::unique_ptr<IncomingFramesReader> frames_reader_;
std::unique_ptr<PairedKeyVerificationRunner> key_verification_runner_;
......
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