Tether MessageTransferOperation: Only wait for a response from a host for a certain amount of time …

chromeos/components/tether/message_transfer_operation.h

Index: chromeos/components/tether/message_transfer_operation.h
diff --git a/chromeos/components/tether/message_transfer_operation.h b/chromeos/components/tether/message_transfer_operation.h
index 3265214002750504c3e40f3c9e71205d82c4d08f..a69e0e84eedf40f2a896dab8f11695fb3416c0df 100644
--- a/chromeos/components/tether/message_transfer_operation.h
+++ b/chromeos/components/tether/message_transfer_operation.h
@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "base/timer/timer.h"
 #include "chromeos/components/tether/ble_connection_manager.h"
 
 namespace chromeos {
@@ -16,6 +17,7 @@ namespace chromeos {
 namespace tether {
 
 class MessageWrapper;
+class TimerFactory;
 
 // Abstract base class used for operations which send and/or receive messages
 // from remote devices.
@@ -70,6 +72,16 @@ class MessageTransferOperation : public BleConnectionManager::Observer {
   // Returns the type of message that this operation intends to send.
   virtual MessageType GetMessageTypeForConnection() = 0;
 
+  // Returns true if we should wait for a response from the host device. Note
+  // that if ShouldWaitForResponse() returns true and a response is not received
+  // within the amount of time returned by GetResponseTimeoutSeconds(), the
+  // host device will be unregistered.

[Kyle Horimoto, 2017/06/01 18:18:58]

nit: Adjust wording to make it clear that there is a timeout for each device
(currently, it says "the host device" which can be misconstrued as meaning that
there is only one host device).

[Ryan Hansberry, 2017/06/01 18:44:09]

Good catch, clarified.

+  virtual bool ShouldWaitForResponse();
+
+  // The number of seconds that we should wait for a response from the host. If
+  // ShouldWaitForResponse() returns false, this method is never used.
+  virtual uint32_t GetResponseTimeoutSeconds();
+
   std::vector<cryptauth::RemoteDevice>& remote_devices() {
     return remote_devices_;
   }
@@ -82,12 +94,30 @@ class MessageTransferOperation : public BleConnectionManager::Observer {
 
   static uint32_t kMaxConnectionAttemptsPerDevice;
 
+  // The default number of seconds the client should generally wait for a
+  // response from the host once an authenticated connection is established.
+  // Once this amount of time passes, the connection will be closed. Subclasses
+  // of MessageTransferOperation should override GetResponseTimeoutSeconds()
+  // if they desire a different duration.
+  static uint32_t kDefaultResponseTimeoutSeconds;
+
+  void SetTimerFactoryForTest(
+      std::unique_ptr<TimerFactory> timer_factory_for_test);
+  void StartResponseTimerForDevice(
+      const cryptauth::RemoteDevice& remote_device);
+  void StopResponseTimerForDevice(const cryptauth::RemoteDevice& remote_device);
+  void OnResponseTimeout(const cryptauth::RemoteDevice& remote_device);
+
   std::vector<cryptauth::RemoteDevice> remote_devices_;
   BleConnectionManager* connection_manager_;
+  std::unique_ptr<TimerFactory> timer_factory_;
 
   bool initialized_;
   std::map<cryptauth::RemoteDevice, uint32_t>
       remote_device_to_num_attempts_map_;
+  std::map<cryptauth::RemoteDevice, std::unique_ptr<base::Timer>>
+      remote_device_to_timer_map_;
+  base::WeakPtrFactory<MessageTransferOperation> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(MessageTransferOperation);
 };