Close connection when fail to connect to remote target.

- Posts a delayed task to close connection
- Adds a test to check kTimedOut status in TransferMetadata

Bug: 1085068
Change-Id: I7c74b19533a97272d0aede9940eaa3f477e47827
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359129Reviewed-by: Alex Chau <alexchau@chromium.org>
Commit-Queue: Himanshu Jaju <himanshujaju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799155}

chrome/browser/BUILD.gn

@@ -3367,6 +3367,7 @@ static_library("browser") {
       "metrics/tab_stats_tracker_delegate.h",
       "nearby_sharing/attachment_info.cc",
       "nearby_sharing/attachment_info.h",
+      "nearby_sharing/constants.h",
       "nearby_sharing/fast_initiation_manager.cc",
       "nearby_sharing/fast_initiation_manager.h",
       "nearby_sharing/incoming_frames_reader.cc",

chrome/browser/nearby_sharing/constants.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_NEARBY_SHARING_CONSTANTS_H_
+#define CHROME_BROWSER_NEARBY_SHARING_CONSTANTS_H_
+
+#include "base/time/time.h"
+
+// Timeout for reading a response frame from remote device.
+constexpr base::TimeDelta kReadResponseFrameTimeout =
+    base::TimeDelta::FromSeconds(60);
+
+// The delay before the receiver will disconnect from the sender after rejecting
+// an incoming file. The sender is expected to disconnect immediately after
+// reading the rejection frame.
+constexpr base::TimeDelta kIncomingRejectionDelay =
+    base::TimeDelta::FromSeconds(2);
+
+// Timeout for reading a frame from remote device.
+constexpr base::TimeDelta kReadFramesTimeout = base::TimeDelta::FromSeconds(15);
+
+// Time to delay running the task to invalidate send and receive surfaces.
+constexpr base::TimeDelta kInvalidateDelay =
+    base::TimeDelta::FromMilliseconds(500);
+
+#endif  // CHROME_BROWSER_NEARBY_SHARING_CONSTANTS_H_

chrome/browser/nearby_sharing/nearby_sharing_service_impl.cc

@@ -19,6 +19,7 @@
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_encrypted_metadata_key.h"
 #include "chrome/browser/nearby_sharing/client/nearby_share_client_impl.h"
 #include "chrome/browser/nearby_sharing/common/nearby_share_prefs.h"
+#include "chrome/browser/nearby_sharing/constants.h"
 #include "chrome/browser/nearby_sharing/contacts/nearby_share_contact_manager_impl.h"
 #include "chrome/browser/nearby_sharing/fast_initiation_manager.h"
 #include "chrome/browser/nearby_sharing/local_device_data/nearby_share_local_device_data_manager_impl.h"
@@ -39,15 +40,6 @@
 
 namespace {
 
-constexpr base::TimeDelta kReadFramesTimeout = base::TimeDelta::FromSeconds(15);
-constexpr base::TimeDelta kReadResponseFrameTimeout =
-    base::TimeDelta::FromSeconds(60);
-constexpr base::TimeDelta kIncomingRejectionDelay =
-    base::TimeDelta::FromSeconds(2);
-// Time to delay running the task to invalidate send and receive surfaces.
-constexpr base::TimeDelta kInvalidateDelay =
-    base::TimeDelta::FromMilliseconds(500);
-
 // Used to hash a token into a 4 digit string.
 constexpr int kHashModulo = 9973;
 constexpr int kHashBaseMultiplier = 31;
@@ -1251,7 +1243,15 @@ void NearbySharingServiceImpl::Fail(const ShareTarget& share_target,
     return;
   }
 
-  // TODO(himanshujaju) - Create alarm and cancel in SetDisconnectionListener().
+  base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE,
+      base::BindOnce(&NearbySharingServiceImpl::CloseConnection,
+                     weak_ptr_factory_.GetWeakPtr(), share_target),
+      kIncomingRejectionDelay);
+
+  connection->SetDisconnectionListener(
+      base::BindOnce(&NearbySharingServiceImpl::UnregisterShareTarget,
+                     weak_ptr_factory_.GetWeakPtr(), share_target));
 
   // Send response to remote device.
   sharing::nearby::ConnectionResponseFrame::Status response_status;
@@ -1706,10 +1706,6 @@ void NearbySharingServiceImpl::OnIncomingMutualAcceptanceTimeout(
       << ": Incoming mutual acceptance timed out, closing connection for "
       << share_target.device_name;
 
-  OnIncomingTransferUpdate(share_target,
-                           TransferMetadataBuilder()
-                               .set_status(TransferMetadata::Status::kTimedOut)
-                               .build());
   Fail(share_target, TransferMetadata::Status::kTimedOut);
 }
 

chrome/browser/nearby_sharing/nearby_sharing_service_impl_unittest.cc

@@ -25,6 +25,7 @@
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager_impl.h"
 #include "chrome/browser/nearby_sharing/certificates/test_util.h"
 #include "chrome/browser/nearby_sharing/common/nearby_share_prefs.h"
+#include "chrome/browser/nearby_sharing/constants.h"
 #include "chrome/browser/nearby_sharing/contacts/fake_nearby_share_contact_manager.h"
 #include "chrome/browser/nearby_sharing/contacts/nearby_share_contact_manager_impl.h"
 #include "chrome/browser/nearby_sharing/fake_nearby_connection.h"
@@ -173,6 +174,8 @@ class MockShareTargetDiscoveredCallback : public ShareTargetDiscoveredCallback {
 
 namespace {
 
+constexpr base::TimeDelta kDelta = base::TimeDelta::FromMilliseconds(100);
+
 const char kServiceId[] = "NearbySharing";
 const char kDeviceName[] = "test_device_name";
 const char kEndpointId[] = "test_endpoint_id";
@@ -1570,6 +1573,23 @@ TEST_F(NearbySharingServiceImplTest,
   service_->UnregisterReceiveSurface(&callback);
 }
 
+TEST_F(NearbySharingServiceImplTest, IncomingConnection_TimedOut) {
+  NiceMock<MockTransferUpdateCallback> callback;
+  ShareTarget share_target = SetUpIncomingConnection(callback);
+  EXPECT_FALSE(connection_.IsClosed());
+
+  EXPECT_CALL(callback, OnTransferUpdate(testing::_, testing::_))
+      .WillOnce(testing::Invoke(
+          [](const ShareTarget& share_target, TransferMetadata metadata) {
+            EXPECT_EQ(TransferMetadata::Status::kTimedOut, metadata.status());
+            EXPECT_TRUE(metadata.is_final_status());
+          }));
+
+  task_environment_.FastForwardBy(kReadResponseFrameTimeout +
+                                  kIncomingRejectionDelay + kDelta);
+  EXPECT_TRUE(connection_.IsClosed());
+}
+
 TEST_F(NearbySharingServiceImplTest,
        IncomingConnection_ClosedWaitingLocalConfirmation) {
   NiceMock<MockTransferUpdateCallback> callback;
@@ -1892,8 +1912,7 @@ TEST_F(NearbySharingServiceImplTest, RejectValidShareTarget) {
   ExpectConnectionResponseFrame(
       sharing::nearby::ConnectionResponseFrame::REJECT);
 
-  // TODO(himanshujaju) - Extract out to a common constant b/w impl and test.
-  task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(2));
+  task_environment_.FastForwardBy(kIncomingRejectionDelay + kDelta);
   EXPECT_TRUE(connection_.IsClosed());
 
   // To avoid UAF in OnIncomingTransferUpdate().