Introduce callback mechanism for AckMessage

- ResponseMessage is passed in SendMessageToDeviceCallback
- SharingMessageHandler is requried to call done callback when done

Bug: 1016714
Change-Id: I12e5729d39c9a9046ca7e8ffdfae0047a79d1464
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1869218
Commit-Queue: Alex Chau <alexchau@chromium.org>
Reviewed-by: Richard Knoll <knollr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708973}

chrome/browser/sharing/ack_message_handler.cc

@@ -4,23 +4,27 @@
 
 #include "chrome/browser/sharing/ack_message_handler.h"
 
+#include "base/memory/ptr_util.h"
 #include "chrome/browser/sharing/proto/sharing_message.pb.h"
 
-AckMessageHandler::AckMessageHandler() = default;
+AckMessageHandler::AckMessageHandler(AckReceivedCallback ack_received_callback)
+    : ack_received_callback_(std::move(ack_received_callback)) {}
 
 AckMessageHandler::~AckMessageHandler() = default;
 
 void AckMessageHandler::OnMessage(
-    const chrome_browser_sharing::SharingMessage& message) {
-  for (AckMessageObserver& observer : observers_)
-    observer.OnAckReceived(message.ack_message().original_message_type(),
-                           message.ack_message().original_message_id());
-}
+    chrome_browser_sharing::SharingMessage message,
+    SharingMessageHandler::DoneCallback done_callback) {
+  DCHECK(message.has_ack_message());
+  chrome_browser_sharing::AckMessage* ack_message =
+      message.mutable_ack_message();
+  std::unique_ptr<chrome_browser_sharing::ResponseMessage> response;
+  if (ack_message->has_response_message())
+    response = base::WrapUnique(ack_message->release_response_message());
 
-void AckMessageHandler::AddObserver(AckMessageObserver* observer) {
-  observers_.AddObserver(observer);
-}
+  ack_received_callback_.Run(ack_message->original_message_type(),
+                             ack_message->original_message_id(),
+                             std::move(response));
 
-void AckMessageHandler::RemoveObserver(AckMessageObserver* observer) {
-  observers_.RemoveObserver(observer);
+  std::move(done_callback).Run(/*response=*/nullptr);
 }

chrome/browser/sharing/ack_message_handler.h

@@ -5,46 +5,35 @@
 #ifndef CHROME_BROWSER_SHARING_ACK_MESSAGE_HANDLER_H_
 #define CHROME_BROWSER_SHARING_ACK_MESSAGE_HANDLER_H_
 
+#include <memory>
 #include <string>
 
+#include "base/callback_forward.h"
 #include "base/macros.h"
-#include "base/observer_list.h"
 #include "chrome/browser/sharing/sharing_message_handler.h"
 
 namespace chrome_browser_sharing {
 enum MessageType : int;
+class ResponseMessage;
 }  // namespace chrome_browser_sharing
 
 // Class to managae ack message and notify observers.
 class AckMessageHandler : public SharingMessageHandler {
  public:
-  // Interface for objects observing ack message received events.
-  class AckMessageObserver : public base::CheckedObserver {
-   public:
-    // Called when an ack message is received, where |message_type| is the type
-    // of the original message, and |message_id| is the identifier of the
-    // original message.
-    virtual void OnAckReceived(chrome_browser_sharing::MessageType message_type,
-                               const std::string& message_id) = 0;
-  };
-
-  AckMessageHandler();
-  ~AckMessageHandler() override;
-
-  // Add an observer ack message received events. An observer should not be
-  // added more than once.
-  void AddObserver(AckMessageObserver* observer);
+  using AckReceivedCallback = base::RepeatingCallback<void(
+      chrome_browser_sharing::MessageType message_type,
+      std::string message_id,
+      std::unique_ptr<chrome_browser_sharing::ResponseMessage> response)>;
 
-  // Removes the given observer from ack message received events. Does nothing
-  // if this observer has not been added.
-  void RemoveObserver(AckMessageObserver* observer);
+  explicit AckMessageHandler(AckReceivedCallback ack_received_callback);
+  ~AckMessageHandler() override;
 
   // SharingMessageHandler implementation:
-  void OnMessage(
-      const chrome_browser_sharing::SharingMessage& message) override;
+  void OnMessage(chrome_browser_sharing::SharingMessage message,
+                 SharingMessageHandler::DoneCallback done_callback) override;
 
  private:
-  base::ObserverList<AckMessageObserver> observers_;
+  AckReceivedCallback ack_received_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(AckMessageHandler);
 };

chrome/browser/sharing/ack_message_handler_unittest.cc

@@ -4,41 +4,86 @@
 
 #include "chrome/browser/sharing/ack_message_handler.h"
 
+#include "base/bind_helpers.h"
+#include "base/test/mock_callback.h"
 #include "chrome/browser/sharing/proto/sharing_message.pb.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
-class TestObserver : public AckMessageHandler::AckMessageObserver {
+class TestObserver {
  public:
-  void OnAckReceived(chrome_browser_sharing::MessageType message_type,
-                     const std::string& message_id) override {
+  void OnAckReceived(
+      chrome_browser_sharing::MessageType message_type,
+      std::string message_id,
+      std::unique_ptr<chrome_browser_sharing::ResponseMessage> response) {
     received_message_type_ = message_type;
-    received_message_id_ = message_id;
+    received_message_id_ = std::move(message_id);
+    received_response_ = std::move(response);
   }
 
   chrome_browser_sharing::MessageType received_message_type() const {
     return received_message_type_;
   }
 
-  std::string received_message_id() const { return received_message_id_; }
+  const std::string& received_message_id() const {
+    return received_message_id_;
+  }
+
+  const chrome_browser_sharing::ResponseMessage* received_response() const {
+    return received_response_.get();
+  }
 
  private:
   chrome_browser_sharing::MessageType received_message_type_;
   std::string received_message_id_;
+  std::unique_ptr<chrome_browser_sharing::ResponseMessage> received_response_;
 };
 
 class AckMessageHandlerTest : public testing::Test {
  protected:
-  AckMessageHandlerTest() { ack_message_handler_.AddObserver(&test_observer_); }
+  AckMessageHandlerTest()
+      : ack_message_handler_(
+            base::BindRepeating(&TestObserver::OnAckReceived,
+                                base::Unretained(&test_observer_))) {}
 
-  AckMessageHandler ack_message_handler_;
   TestObserver test_observer_;
+  AckMessageHandler ack_message_handler_;
 };
 
+bool ProtoEquals(const google::protobuf::MessageLite& expected,
+                 const google::protobuf::MessageLite& actual) {
+  std::string expected_serialized, actual_serialized;
+  expected.SerializeToString(&expected_serialized);
+  actual.SerializeToString(&actual_serialized);
+  return expected_serialized == actual_serialized;
+}
+
 }  // namespace
 
-TEST_F(AckMessageHandlerTest, OnMessage) {
+TEST_F(AckMessageHandlerTest, OnMessageNoResponse) {
+  constexpr char kTestMessageId[] = "test_message_id";
+
+  chrome_browser_sharing::SharingMessage sharing_message;
+  sharing_message.mutable_ack_message()->set_original_message_id(
+      kTestMessageId);
+  sharing_message.mutable_ack_message()->set_original_message_type(
+      chrome_browser_sharing::CLICK_TO_CALL_MESSAGE);
+
+  base::MockCallback<SharingMessageHandler::DoneCallback> done_callback;
+  EXPECT_CALL(done_callback, Run(testing::Eq(nullptr))).Times(1);
+
+  ack_message_handler_.OnMessage(std::move(sharing_message),
+                                 done_callback.Get());
+
+  EXPECT_EQ(kTestMessageId, test_observer_.received_message_id());
+  EXPECT_EQ(chrome_browser_sharing::CLICK_TO_CALL_MESSAGE,
+            test_observer_.received_message_type());
+  EXPECT_FALSE(test_observer_.received_response());
+}
+
+TEST_F(AckMessageHandlerTest, OnMessageWithResponse) {
   constexpr char kTestMessageId[] = "test_message_id";
 
   chrome_browser_sharing::SharingMessage sharing_message;
@@ -46,10 +91,21 @@ TEST_F(AckMessageHandlerTest, OnMessage) {
       kTestMessageId);
   sharing_message.mutable_ack_message()->set_original_message_type(
       chrome_browser_sharing::CLICK_TO_CALL_MESSAGE);
+  sharing_message.mutable_ack_message()->mutable_response_message();
+
+  chrome_browser_sharing::ResponseMessage response_message_copy =
+      sharing_message.ack_message().response_message();
+
+  base::MockCallback<SharingMessageHandler::DoneCallback> done_callback;
+  EXPECT_CALL(done_callback, Run(testing::Eq(nullptr))).Times(1);
 
-  ack_message_handler_.OnMessage(sharing_message);
+  ack_message_handler_.OnMessage(std::move(sharing_message),
+                                 done_callback.Get());
 
   EXPECT_EQ(kTestMessageId, test_observer_.received_message_id());
   EXPECT_EQ(chrome_browser_sharing::CLICK_TO_CALL_MESSAGE,
             test_observer_.received_message_type());
+  ASSERT_TRUE(test_observer_.received_response());
+  EXPECT_TRUE(
+      ProtoEquals(response_message_copy, *test_observer_.received_response()));
 }

chrome/browser/sharing/click_to_call/click_to_call_context_menu_observer_unittest.cc

@@ -145,7 +145,7 @@ TEST_F(ClickToCallContextMenuObserverTest, SingleDevice_ShowMenu) {
   EXPECT_EQ(item_id, static_cast<int>(menu_.GetMenuSize()));
 
   // Emulate click on the device.
-  EXPECT_CALL(*service(), SendMessageToDevice(Eq(guid), Eq(kSharingMessageTTL),
+  EXPECT_CALL(*service(), SendMessageToDevice(Eq(guid), Eq(kSendMessageTimeout),
                                               ProtoEquals(sharing_message), _))
       .Times(1);
   menu_.ExecuteCommand(IDC_CONTENT_CONTEXT_SHARING_CLICK_TO_CALL_SINGLE_DEVICE,
@@ -189,7 +189,7 @@ TEST_F(ClickToCallContextMenuObserverTest, MultipleDevices_ShowMenu) {
   for (int i = 0; i < kMaxDevicesShown; i++) {
     if (i < device_count) {
       EXPECT_CALL(*service(),
-                  SendMessageToDevice(Eq(guids[i]), Eq(kSharingMessageTTL),
+                  SendMessageToDevice(Eq(guids[i]), Eq(kSendMessageTimeout),
                                       ProtoEquals(sharing_message), _))
           .Times(1);
     } else {
@@ -238,7 +238,7 @@ TEST_F(ClickToCallContextMenuObserverTest,
   for (int i = 0; i < device_count; i++) {
     if (i < kMaxDevicesShown) {
       EXPECT_CALL(*service(),
-                  SendMessageToDevice(Eq(guids[i]), Eq(kSharingMessageTTL),
+                  SendMessageToDevice(Eq(guids[i]), Eq(kSendMessageTimeout),
                                       ProtoEquals(sharing_message), _))
           .Times(1);
     } else {

chrome/browser/sharing/click_to_call/click_to_call_message_handler_android.cc

@@ -15,11 +15,14 @@ ClickToCallMessageHandler::ClickToCallMessageHandler() = default;
 ClickToCallMessageHandler::~ClickToCallMessageHandler() = default;
 
 void ClickToCallMessageHandler::OnMessage(
-    const chrome_browser_sharing::SharingMessage& message) {
+    chrome_browser_sharing::SharingMessage message,
+    SharingMessageHandler::DoneCallback done_callback) {
   DCHECK(message.has_click_to_call_message());
   std::string phone_number = message.click_to_call_message().phone_number();
   JNIEnv* env = base::android::AttachCurrentThread();
 
   Java_ClickToCallMessageHandler_handleMessage(
       env, base::android::ConvertUTF8ToJavaString(env, phone_number));
+
+  std::move(done_callback).Run(/*response=*/nullptr);
 }

chrome/browser/sharing/click_to_call/click_to_call_message_handler_android.h

@@ -15,8 +15,8 @@ class ClickToCallMessageHandler : public SharingMessageHandler {
   ~ClickToCallMessageHandler() override;
 
   // SharingMessageHandler implementation:
-  void OnMessage(
-      const chrome_browser_sharing::SharingMessage& message) override;
+  void OnMessage(chrome_browser_sharing::SharingMessage message,
+                 SharingMessageHandler::DoneCallback done_callback) override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ClickToCallMessageHandler);

chrome/browser/sharing/click_to_call/click_to_call_ui_controller_unittest.cc

@@ -86,7 +86,7 @@ TEST_F(ClickToCallUiControllerTest, OnDeviceChosen) {
       kExpectedPhoneNumber);
   EXPECT_CALL(*service(),
               SendMessageToDevice(testing::Eq(kReceiverGuid),
-                                  testing::Eq(kSharingMessageTTL),
+                                  testing::Eq(kSendMessageTimeout),
                                   ProtoEquals(sharing_message), testing::_));
   controller_->OnDeviceChosen(device_info);
 }

chrome/browser/sharing/mock_sharing_service.h

@@ -21,7 +21,7 @@ class MockSharingService : public SharingService {
 
   MOCK_METHOD4(SendMessageToDevice,
                void(const std::string& device_guid,
-                    base::TimeDelta time_to_live,
+                    base::TimeDelta response_timeout,
                     chrome_browser_sharing::SharingMessage message,
                     SharingService::SendMessageCallback callback));
 

chrome/browser/sharing/ping_message_handler.cc

@@ -4,11 +4,15 @@
 
 #include "chrome/browser/sharing/ping_message_handler.h"
 
+#include "chrome/browser/sharing/proto/sharing_message.pb.h"
+
 PingMessageHandler::PingMessageHandler() = default;
 
 PingMessageHandler::~PingMessageHandler() = default;
 
 void PingMessageHandler::OnMessage(
-    const chrome_browser_sharing::SharingMessage& message) {
-  // TODO
+    chrome_browser_sharing::SharingMessage message,
+    SharingMessageHandler::DoneCallback done_callback) {
+  // Delibrately empty.
+  std::move(done_callback).Run(/*response=*/nullptr);
 }

chrome/browser/sharing/ping_message_handler.h

@@ -14,8 +14,8 @@ class PingMessageHandler : public SharingMessageHandler {
   ~PingMessageHandler() override;
 
   // SharingMessageHandler implementation:
-  void OnMessage(
-      const chrome_browser_sharing::SharingMessage& message) override;
+  void OnMessage(chrome_browser_sharing::SharingMessage message,
+                 SharingMessageHandler::DoneCallback done_callback) override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(PingMessageHandler);

chrome/browser/sharing/proto/BUILD.gn

@@ -16,6 +16,6 @@ proto_library("proto") {
     "click_to_call_message.proto",
     "shared_clipboard_message.proto",
     "sharing_message.proto",
-    "sms_fetch_request.proto",
+    "sms_fetch_message.proto",
   ]
 }

chrome/browser/sharing/proto/sharing_message.proto

@@ -6,7 +6,7 @@ syntax = "proto3";
 
 import "click_to_call_message.proto";
 import "shared_clipboard_message.proto";
-import "sms_fetch_request.proto";
+import "sms_fetch_message.proto";
 
 package chrome_browser_sharing;
 
@@ -60,6 +60,15 @@ message AckMessage {
 
   // The type of message that this is acknowledging. optional.
   MessageType original_message_type = 2;
+
+  // Response of the message, optional.
+  ResponseMessage response_message = 3;
+}
+
+// Message for responding to a SharingMessage.
+message ResponseMessage {
+  // Payload of the response, contains one of the messages below. required.
+  oneof payload { SmsFetchResponse sms_fetch_response = 1; }
 }
 
 // Message for data necessary to send an AckMessage to the sender.

chrome/browser/sharing/proto/sms_fetch_request.proto → chrome/browser/sharing/proto/sms_fetch_message.proto

@@ -11,3 +11,6 @@ option optimize_for = LITE_RUNTIME;
 
 // Request message to fetch a SMS from a remote device.
 message SmsFetchRequest {}
+
+// Response message to fetch a SMS from a remote device.
+message SmsFetchResponse {}

chrome/browser/sharing/shared_clipboard/shared_clipboard_context_menu_observer_unittest.cc

@@ -141,7 +141,7 @@ TEST_F(SharedClipboardContextMenuObserverTest, SingleDevice_ShowMenu) {
             item.command_id);
 
   // Emulate click on the device.
-  EXPECT_CALL(*service(), SendMessageToDevice(Eq(guid), Eq(kSharingMessageTTL),
+  EXPECT_CALL(*service(), SendMessageToDevice(Eq(guid), Eq(kSendMessageTimeout),
                                               ProtoEquals(sharing_message), _))
       .Times(1);
   menu_.ExecuteCommand(
@@ -181,7 +181,7 @@ TEST_F(SharedClipboardContextMenuObserverTest, MultipleDevices_ShowMenu) {
   for (int i = 0; i < kMaxDevicesShown; i++) {
     if (i < device_count) {
       EXPECT_CALL(*service(),
-                  SendMessageToDevice(Eq(guids[i]), Eq(kSharingMessageTTL),
+                  SendMessageToDevice(Eq(guids[i]), Eq(kSendMessageTimeout),
                                       ProtoEquals(sharing_message), _))
           .Times(1);
     } else {
@@ -226,7 +226,7 @@ TEST_F(SharedClipboardContextMenuObserverTest,
   for (int i = 0; i < device_count; i++) {
     if (i < kMaxDevicesShown) {
       EXPECT_CALL(*service(),
-                  SendMessageToDevice(Eq(guids[i]), Eq(kSharingMessageTTL),
+                  SendMessageToDevice(Eq(guids[i]), Eq(kSendMessageTimeout),
                                       ProtoEquals(sharing_message), _))
           .Times(1);
     } else {

chrome/browser/sharing/shared_clipboard/shared_clipboard_message_handler.cc

@@ -22,7 +22,8 @@ SharedClipboardMessageHandler::SharedClipboardMessageHandler(
 SharedClipboardMessageHandler::~SharedClipboardMessageHandler() = default;
 
 void SharedClipboardMessageHandler::OnMessage(
-    const chrome_browser_sharing::SharingMessage& message) {
+    chrome_browser_sharing::SharingMessage message,
+    SharingMessageHandler::DoneCallback done_callback) {
   DCHECK(message.has_shared_clipboard_message());
 
   ui::ScopedClipboardWriter(ui::ClipboardBuffer::kCopyPaste)
@@ -33,4 +34,6 @@ void SharedClipboardMessageHandler::OnMessage(
   const std::string& device_name =
       device ? device->client_name() : message.sender_device_name();
   ShowNotification(device_name);
+
+  std::move(done_callback).Run(/*response=*/nullptr);
 }

chrome/browser/sharing/shared_clipboard/shared_clipboard_message_handler.h

@@ -19,8 +19,8 @@ class SharedClipboardMessageHandler : public SharingMessageHandler {
   ~SharedClipboardMessageHandler() override;
 
   // SharingMessageHandler implementation:
-  void OnMessage(
-      const chrome_browser_sharing::SharingMessage& message) override;
+  void OnMessage(chrome_browser_sharing::SharingMessage message,
+                 SharingMessageHandler::DoneCallback done_callback) override;
 
  protected:
   // Called after the message has been copied to the clipboard. Implementers

chrome/browser/sharing/shared_clipboard/shared_clipboard_message_handler_desktop_unittest.cc

@@ -8,6 +8,7 @@
 
 #include "base/guid.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/mock_callback.h"
 #include "chrome/browser/notifications/stub_notification_display_service.h"
 #include "chrome/browser/sharing/mock_sharing_service.h"
 #include "chrome/browser/sharing/proto/shared_clipboard_message.pb.h"
@@ -94,7 +95,10 @@ TEST_F(SharedClipboardMessageHandlerTest, NotificationWithoutDeviceName) {
             [](const std::string& guid) -> std::unique_ptr<syncer::DeviceInfo> {
               return nullptr;
             });
-    message_handler_->OnMessage(CreateMessage(guid, kEmptyDeviceName));
+    base::MockCallback<SharingMessageHandler::DoneCallback> done_callback;
+    EXPECT_CALL(done_callback, Run(testing::Eq(nullptr))).Times(1);
+    message_handler_->OnMessage(CreateMessage(guid, kEmptyDeviceName),
+                                done_callback.Get());
   }
   EXPECT_EQ(GetClipboardText(), kText);
 
@@ -122,7 +126,10 @@ TEST_F(SharedClipboardMessageHandlerTest,
                   /*send_tab_to_self_receiving_enabled=*/false,
                   /*sharing_info=*/base::nullopt);
             });
-    message_handler_->OnMessage(CreateMessage(guid, kEmptyDeviceName));
+    base::MockCallback<SharingMessageHandler::DoneCallback> done_callback;
+    EXPECT_CALL(done_callback, Run(testing::Eq(nullptr))).Times(1);
+    message_handler_->OnMessage(CreateMessage(guid, kEmptyDeviceName),
+                                done_callback.Get());
   }
   message_center::Notification notification = GetNotification();
   EXPECT_EQ(l10n_util::GetStringFUTF16(
@@ -140,7 +147,10 @@ TEST_F(SharedClipboardMessageHandlerTest,
             [](const std::string& guid) -> std::unique_ptr<syncer::DeviceInfo> {
               return nullptr;
             });
-    message_handler_->OnMessage(CreateMessage(guid, kDeviceNameInMessage));
+    base::MockCallback<SharingMessageHandler::DoneCallback> done_callback;
+    EXPECT_CALL(done_callback, Run(testing::Eq(nullptr))).Times(1);
+    message_handler_->OnMessage(CreateMessage(guid, kDeviceNameInMessage),
+                                done_callback.Get());
   }
   EXPECT_EQ(GetClipboardText(), kText);
 

chrome/browser/sharing/shared_clipboard/shared_clipboard_ui_controller_unittest.cc

@@ -90,7 +90,7 @@ TEST_F(SharedClipboardUiControllerTest, OnDeviceChosen) {
   sharing_message.mutable_shared_clipboard_message()->set_text(kExpectedText);
   EXPECT_CALL(*service(),
               SendMessageToDevice(testing::Eq(kReceiverGuid),
-                                  testing::Eq(kSharingMessageTTL),
+                                  testing::Eq(kSendMessageTimeout),
                                   ProtoEquals(sharing_message), testing::_));
   controller_->OnDeviceChosen(device_info);
 }

chrome/browser/sharing/sharing_constants.cc

@@ -8,8 +8,14 @@ const char kFCMScope[] = "GCM";
 
 const char kSharingFCMAppID[] = "com.google.chrome.sharing.fcm";
 
+// Based on Stable + Beta metrics on 23-Oct-2019, 95th percentile of round trip
+// time (Sharing.MessageAckTime) is ~16 seconds. Message timeout is set as round
+// trip expected time, and ack TTL is set as half of the value.
+const constexpr base::TimeDelta kSendMessageTimeout =
+    base::TimeDelta::FromSeconds(16);
+
 const constexpr base::TimeDelta kAckTimeToLive =
-    base::TimeDelta::FromMinutes(30);
+    base::TimeDelta::FromSeconds(8);
 
 const constexpr base::TimeDelta kDeviceExpiration =
     base::TimeDelta::FromDays(2);
@@ -17,9 +23,6 @@ const constexpr base::TimeDelta kDeviceExpiration =
 const constexpr base::TimeDelta kRegistrationExpiration =
     base::TimeDelta::FromDays(1);
 
-const constexpr base::TimeDelta kSendMessageTimeout =
-    base::TimeDelta::FromSeconds(15);
-
 const constexpr net::BackoffEntry::Policy kRetryBackoffPolicy = {
     // Number of initial errors (in sequence) to ignore before applying
     // exponential back-off rules.
@@ -52,8 +55,6 @@ const constexpr net::BackoffEntry::Policy kRetryBackoffPolicy = {
     false,
 };
 
-constexpr base::TimeDelta kSharingMessageTTL = base::TimeDelta::FromSeconds(10);
-
 constexpr int kMaxDevicesShown = 10;
 
 constexpr int kSubMenuFirstDeviceCommandId = 2150;

chrome/browser/sharing/sharing_constants.h

@@ -14,6 +14,9 @@ extern const char kFCMScope[];
 // Sender ID linked to GCM messages for Sharing.
 extern const char kSharingFCMAppID[];
 
+// Amount of time before a message is considered timeout if no ack is received.
+extern const base::TimeDelta kSendMessageTimeout;
+
 // Amount of time before an ack message is expired.
 extern const base::TimeDelta kAckTimeToLive;
 
@@ -23,15 +26,9 @@ extern const base::TimeDelta kDeviceExpiration;
 // Amount of time before FCM registration should happen again.
 extern const base::TimeDelta kRegistrationExpiration;
 
-// Amount of time before a message is considered timeout if no ack is received.
-extern const base::TimeDelta kSendMessageTimeout;
-
 // Backoff policy for registration retry.
 extern const net::BackoffEntry::Policy kRetryBackoffPolicy;
 
-// Time limit for message expiration.
-extern const base::TimeDelta kSharingMessageTTL;
-
 // Maximum number of devices to be shown in dialog and context menu.
 extern const int kMaxDevicesShown;
 

chrome/browser/sharing/sharing_fcm_handler.cc

@@ -122,10 +122,14 @@ void SharingFCMHandler::OnMessage(const std::string& app_id,
     LOG(ERROR) << "No handler found for payload : "
                << sharing_message.payload_case();
   } else {
-    it->second->OnMessage(sharing_message);
-
-    if (sharing_message.payload_case() != SharingMessage::kAckMessage)
-      SendAckMessage(sharing_message, message_id);
+    SharingMessageHandler::DoneCallback done_callback = base::DoNothing();
+    if (sharing_message.payload_case() != SharingMessage::kAckMessage) {
+      done_callback = base::BindOnce(
+          &SharingFCMHandler::SendAckMessage, weak_ptr_factory_.GetWeakPtr(),
+          std::move(message_id), message_type, GetSharingInfo(sharing_message));
+    }
+
+    it->second->OnMessage(std::move(sharing_message), std::move(done_callback));
   }
 }
 
@@ -157,18 +161,11 @@ SharingFCMHandler::GetSharingInfo(const SharingMessage& original_message) {
   return sync_preference_->GetSharingInfo(original_message.sender_guid());
 }
 
-void SharingFCMHandler::SendAckMessage(const SharingMessage& original_message,
-                                       const std::string& original_message_id) {
-  SharingMessage ack_message;
-  ack_message.mutable_ack_message()->set_original_message_id(
-      original_message_id);
-  chrome_browser_sharing::MessageType original_message_type =
-      SharingPayloadCaseToMessageType(original_message.payload_case());
-  ack_message.mutable_ack_message()->set_original_message_type(
-      original_message_type);
-
-  base::Optional<syncer::DeviceInfo::SharingInfo> sharing_info =
-      GetSharingInfo(original_message);
+void SharingFCMHandler::SendAckMessage(
+    std::string original_message_id,
+    chrome_browser_sharing::MessageType original_message_type,
+    base::Optional<syncer::DeviceInfo::SharingInfo> sharing_info,
+    std::unique_ptr<chrome_browser_sharing::ResponseMessage> response) {
   if (!sharing_info) {
     LOG(ERROR) << "Unable to find sharing info";
     LogSendSharingAckMessageResult(original_message_type,
@@ -176,16 +173,24 @@ void SharingFCMHandler::SendAckMessage(const SharingMessage& original_message,
     return;
   }
 
+  SharingMessage sharing_message;
+  chrome_browser_sharing::AckMessage* ack_message =
+      sharing_message.mutable_ack_message();
+  ack_message->set_original_message_id(original_message_id);
+  ack_message->set_original_message_type(original_message_type);
+  if (response)
+    ack_message->set_allocated_response_message(response.release());
+
   sharing_fcm_sender_->SendMessageToDevice(
-      std::move(*sharing_info), kAckTimeToLive, std::move(ack_message),
+      std::move(*sharing_info), kAckTimeToLive, std::move(sharing_message),
       /*sender_device_info=*/nullptr,
       base::BindOnce(&SharingFCMHandler::OnAckMessageSent,
-                     weak_ptr_factory_.GetWeakPtr(), original_message_id,
-                     original_message_type));
+                     weak_ptr_factory_.GetWeakPtr(),
+                     std::move(original_message_id), original_message_type));
 }
 
 void SharingFCMHandler::OnAckMessageSent(
-    const std::string& original_message_id,
+    std::string original_message_id,
     chrome_browser_sharing::MessageType original_message_type,
     SharingSendMessageResult result,
     base::Optional<std::string> message_id) {

chrome/browser/sharing/sharing_fcm_handler.h

@@ -73,11 +73,14 @@ class SharingFCMHandler : public gcm::GCMAppHandler {
       const SharingMessage& original_message);
 
   // Ack message sent back to the original sender of message.
-  void SendAckMessage(const SharingMessage& original_message,
-                      const std::string& original_message_id);
+  void SendAckMessage(
+      std::string original_message_id,
+      chrome_browser_sharing::MessageType original_message_type,
+      base::Optional<syncer::DeviceInfo::SharingInfo> sharing_info,
+      std::unique_ptr<chrome_browser_sharing::ResponseMessage> response);
 
   void OnAckMessageSent(
-      const std::string& original_message_id,
+      std::string original_message_id,
       chrome_browser_sharing::MessageType original_message_type,
       SharingSendMessageResult result,
       base::Optional<std::string> message_id);

chrome/browser/sharing/sharing_fcm_handler_unittest.cc

@@ -38,7 +38,9 @@ class MockSharingMessageHandler : public SharingMessageHandler {
   ~MockSharingMessageHandler() override = default;
 
   // SharingMessageHandler implementation:
-  MOCK_METHOD1(OnMessage, void(const SharingMessage& message));
+  MOCK_METHOD2(OnMessage,
+               void(SharingMessage message,
+                    SharingMessageHandler::DoneCallback done_callback));
 };
 
 class MockSharingFCMSender : public SharingFCMSender {
@@ -123,7 +125,7 @@ TEST_F(SharingFCMHandlerTest, AckMessageHandler) {
       CreateGCMIncomingMessage(kTestMessageId, sharing_message);
 
   EXPECT_CALL(mock_sharing_message_handler_,
-              OnMessage(ProtoEquals(sharing_message)));
+              OnMessage(ProtoEquals(sharing_message), _));
   EXPECT_CALL(mock_sharing_fcm_sender_, SendMessageToDevice(_, _, _, _, _))
       .Times(0);
   sharing_fcm_handler_->AddSharingHandler(SharingMessage::kAckMessage,
@@ -149,14 +151,20 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandler) {
       chrome_browser_sharing::PING_MESSAGE);
 
   // Tests OnMessage flow in SharingFCMHandler when no handler is registered.
-  EXPECT_CALL(mock_sharing_message_handler_, OnMessage(_)).Times(0);
+  EXPECT_CALL(mock_sharing_message_handler_, OnMessage(_, _)).Times(0);
   EXPECT_CALL(mock_sharing_fcm_sender_, SendMessageToDevice(_, _, _, _, _))
       .Times(0);
   sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
 
   // Tests OnMessage flow in SharingFCMHandler after handler is added.
-  EXPECT_CALL(mock_sharing_message_handler_,
-              OnMessage(ProtoEquals(sharing_message)));
+  ON_CALL(mock_sharing_message_handler_,
+          OnMessage(ProtoEquals(sharing_message), _))
+      .WillByDefault(testing::Invoke(
+          [](const SharingMessage& message,
+             SharingMessageHandler::DoneCallback done_callback) {
+            std::move(done_callback).Run(/*response=*/nullptr);
+          }));
+  EXPECT_CALL(mock_sharing_message_handler_, OnMessage(_, _));
   EXPECT_CALL(mock_sharing_fcm_sender_,
               SendMessageToDevice(DeviceMatcher(), testing::Eq(kAckTimeToLive),
                                   ProtoEquals(sharing_ack_message),
@@ -167,13 +175,49 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandler) {
 
   // Tests OnMessage flow in SharingFCMHandler after registered handler is
   // removed.
-  EXPECT_CALL(mock_sharing_message_handler_, OnMessage(_)).Times(0);
+  EXPECT_CALL(mock_sharing_message_handler_, OnMessage(_, _)).Times(0);
   EXPECT_CALL(mock_sharing_fcm_sender_, SendMessageToDevice(_, _, _, _, _))
       .Times(0);
   sharing_fcm_handler_->RemoveSharingHandler(SharingMessage::kPingMessage);
   sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
 }
 
+TEST_F(SharingFCMHandlerTest, PingMessageHandlerWithResponse) {
+  fake_device_info_sync_service_.GetDeviceInfoTracker()->Add(
+      fake_device_info_.get());
+
+  SharingMessage sharing_message;
+  sharing_message.set_sender_guid(kSenderGuid);
+  sharing_message.mutable_ping_message();
+  gcm::IncomingMessage incoming_message =
+      CreateGCMIncomingMessage(kTestMessageId, sharing_message);
+
+  SharingMessage sharing_ack_message;
+  sharing_ack_message.mutable_ack_message()->set_original_message_id(
+      kTestMessageId);
+  sharing_ack_message.mutable_ack_message()->set_original_message_type(
+      chrome_browser_sharing::PING_MESSAGE);
+  sharing_ack_message.mutable_ack_message()->mutable_response_message();
+
+  // Tests OnMessage flow in SharingFCMHandler after handler is added.
+  ON_CALL(mock_sharing_message_handler_,
+          OnMessage(ProtoEquals(sharing_message), _))
+      .WillByDefault(testing::Invoke([](const SharingMessage& message,
+                                        SharingMessageHandler::DoneCallback
+                                            done_callback) {
+        std::move(done_callback)
+            .Run(std::make_unique<chrome_browser_sharing::ResponseMessage>());
+      }));
+  EXPECT_CALL(mock_sharing_message_handler_, OnMessage(_, _));
+  EXPECT_CALL(mock_sharing_fcm_sender_,
+              SendMessageToDevice(DeviceMatcher(), testing::Eq(kAckTimeToLive),
+                                  ProtoEquals(sharing_ack_message),
+                                  testing::Eq(nullptr), _));
+  sharing_fcm_handler_->AddSharingHandler(SharingMessage::kPingMessage,
+                                          &mock_sharing_message_handler_);
+  sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
+}
+
 // Test for handling of SharingMessage payload other than AckMessage for
 // secondary users in Android.
 TEST_F(SharingFCMHandlerTest, PingMessageHandlerSecondaryUser) {
@@ -193,8 +237,13 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandlerSecondaryUser) {
       chrome_browser_sharing::PING_MESSAGE);
 
   // Tests OnMessage flow in SharingFCMHandler after handler is added.
-  EXPECT_CALL(mock_sharing_message_handler_,
-              OnMessage(ProtoEquals(sharing_message)));
+  ON_CALL(mock_sharing_message_handler_,
+          OnMessage(ProtoEquals(sharing_message), _))
+      .WillByDefault(testing::Invoke(
+          [](const SharingMessage& message,
+             SharingMessageHandler::DoneCallback done_callback) {
+            std::move(done_callback).Run(/*response=*/nullptr);
+          }));
   EXPECT_CALL(mock_sharing_fcm_sender_,
               SendMessageToDevice(DeviceMatcher(), testing::Eq(kAckTimeToLive),
                                   ProtoEquals(sharing_ack_message),
@@ -224,8 +273,13 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandlerWithRecipientInfo) {
   sharing_ack_message.mutable_ack_message()->set_original_message_type(
       chrome_browser_sharing::PING_MESSAGE);
 
-  EXPECT_CALL(mock_sharing_message_handler_,
-              OnMessage(ProtoEquals(sharing_message)));
+  ON_CALL(mock_sharing_message_handler_,
+          OnMessage(ProtoEquals(sharing_message), _))
+      .WillByDefault(testing::Invoke(
+          [](const SharingMessage& message,
+             SharingMessageHandler::DoneCallback done_callback) {
+            std::move(done_callback).Run(/*response=*/nullptr);
+          }));
   EXPECT_CALL(mock_sharing_fcm_sender_,
               SendMessageToDevice(DeviceMatcher(), testing::Eq(kAckTimeToLive),
                                   ProtoEquals(sharing_ack_message),

chrome/browser/sharing/sharing_message_handler.h

@@ -5,18 +5,27 @@
 #ifndef CHROME_BROWSER_SHARING_SHARING_MESSAGE_HANDLER_H_
 #define CHROME_BROWSER_SHARING_SHARING_MESSAGE_HANDLER_H_
 
+#include <memory>
+
+#include "base/callback.h"
+
 namespace chrome_browser_sharing {
 class SharingMessage;
+class ResponseMessage;
 }  // namespace chrome_browser_sharing
 
 // Interface for handling incoming SharingMessage.
 class SharingMessageHandler {
  public:
+  using DoneCallback = base::OnceCallback<void(
+      std::unique_ptr<chrome_browser_sharing::ResponseMessage>)>;
+
   virtual ~SharingMessageHandler() = default;
 
-  // Called when a SharingMessage has been received.
-  virtual void OnMessage(
-      const chrome_browser_sharing::SharingMessage& message) = 0;
+  // Called when a SharingMessage has been received. |done_callback| must be
+  // invoked after work to determine response is done.
+  virtual void OnMessage(chrome_browser_sharing::SharingMessage message,
+                         DoneCallback done_callback) = 0;
 };
 
 #endif  // CHROME_BROWSER_SHARING_SHARING_MESSAGE_HANDLER_H_

chrome/browser/sharing/sharing_metrics.cc

@@ -168,11 +168,27 @@ void LogSharingSelectedAppIndex(SharingFeatureName feature,
 
 void LogSharingMessageAckTime(chrome_browser_sharing::MessageType message_type,
                               base::TimeDelta time) {
-  base::UmaHistogramMediumTimes("Sharing.MessageAckTime", time);
-  base::UmaHistogramMediumTimes(
-      base::StrCat({"Sharing.MessageAckTime.",
-                    MessageTypeToMessageSuffix(message_type)}),
-      time);
+  std::string suffixed_name = base::StrCat(
+      {"Sharing.MessageAckTime.", MessageTypeToMessageSuffix(message_type)});
+  switch (message_type) {
+    case chrome_browser_sharing::MessageType::UNKNOWN_MESSAGE:
+    case chrome_browser_sharing::MessageType::PING_MESSAGE:
+    case chrome_browser_sharing::MessageType::CLICK_TO_CALL_MESSAGE:
+    case chrome_browser_sharing::MessageType::SHARED_CLIPBOARD_MESSAGE:
+      base::UmaHistogramMediumTimes(suffixed_name, time);
+      break;
+    case chrome_browser_sharing::MessageType::SMS_FETCH_REQUEST:
+      base::UmaHistogramCustomTimes(
+          suffixed_name, time, /*min=*/base::TimeDelta::FromMilliseconds(1),
+          /*max=*/base::TimeDelta::FromMinutes(10), /*buckets=*/50);
+      break;
+    case chrome_browser_sharing::MessageType::ACK_MESSAGE:
+    default:
+      // For proto3 enums unrecognized enum values are kept, so message_type may
+      // not fall into any switch case. However, as an ack message, original
+      // message type should always be known.
+      NOTREACHED();
+  }
 }
 
 void LogSharingDialogShown(SharingFeatureName feature, SharingDialogType type) {

chrome/browser/sharing/sharing_service.cc

@@ -172,10 +172,12 @@ SharingService::SharingService(
       chrome_browser_sharing::SharingMessage::kPingMessage,
       &ping_message_handler_);
 
+  ack_message_handler_ =
+      std::make_unique<AckMessageHandler>(base::BindRepeating(
+          &SharingService::OnAckReceived, weak_ptr_factory_.GetWeakPtr()));
   fcm_handler_->AddSharingHandler(
       chrome_browser_sharing::SharingMessage::kAckMessage,
-      &ack_message_handler_);
-  ack_message_handler_.AddObserver(this);
+      ack_message_handler_.get());
 
 #if defined(OS_ANDROID)
   // Note: IsClickToCallSupported() is not used as it requires JNI call.
@@ -224,8 +226,6 @@ SharingService::SharingService(
 }
 
 SharingService::~SharingService() {
-  ack_message_handler_.RemoveObserver(this);
-
   if (sync_service_ && sync_service_->HasObserver(this))
     sync_service_->RemoveObserver(this);
 }
@@ -285,7 +285,7 @@ void SharingService::AddDeviceCandidatesInitializedObserver(
 
 void SharingService::SendMessageToDevice(
     const std::string& device_guid,
-    base::TimeDelta time_to_live,
+    base::TimeDelta response_timeout,
     chrome_browser_sharing::SharingMessage message,
     SendMessageCallback callback) {
   std::string message_guid = base::GenerateGUID();
@@ -297,8 +297,9 @@ void SharingService::SendMessageToDevice(
       FROM_HERE, {base::TaskPriority::USER_VISIBLE, content::BrowserThread::UI},
       base::BindOnce(&SharingService::InvokeSendMessageCallback,
                      weak_ptr_factory_.GetWeakPtr(), message_guid, message_type,
-                     SharingSendMessageResult::kAckTimeout),
-      kSendMessageTimeout);
+                     SharingSendMessageResult::kAckTimeout,
+                     /*response=*/nullptr),
+      response_timeout);
 
   // TODO(crbug/1015411): Here we assume caller gets |device_guid| from
   // GetDeviceCandidates, so both DeviceInfoTracker and LocalDeviceInfoProvider
@@ -308,7 +309,8 @@ void SharingService::SendMessageToDevice(
       sync_prefs_->GetSharingInfo(device_guid);
   if (!target_sharing_info) {
     InvokeSendMessageCallback(message_guid, message_type,
-                              SharingSendMessageResult::kDeviceNotFound);
+                              SharingSendMessageResult::kDeviceNotFound,
+                              /*response=*/nullptr);
     return;
   }
 
@@ -316,7 +318,8 @@ void SharingService::SendMessageToDevice(
       local_device_info_provider_->GetLocalDeviceInfo();
   if (!local_device_info) {
     InvokeSendMessageCallback(message_guid, message_type,
-                              SharingSendMessageResult::kInternalError);
+                              SharingSendMessageResult::kInternalError,
+                              /*response=*/nullptr);
     return;
   }
 
@@ -327,13 +330,15 @@ void SharingService::SendMessageToDevice(
 
   if (!sender_device_info->sharing_info()) {
     InvokeSendMessageCallback(message_guid, message_type,
-                              SharingSendMessageResult::kInternalError);
+                              SharingSendMessageResult::kInternalError,
+                              /*response=*/nullptr);
     return;
   }
 
+  DCHECK_GE(response_timeout, kAckTimeToLive);
   fcm_sender_->SendMessageToDevice(
-      std::move(*target_sharing_info), time_to_live, std::move(message),
-      std::move(sender_device_info),
+      std::move(*target_sharing_info), response_timeout - kAckTimeToLive,
+      std::move(message), std::move(sender_device_info),
       base::BindOnce(&SharingService::OnMessageSent,
                      weak_ptr_factory_.GetWeakPtr(), base::TimeTicks::Now(),
                      message_guid, message_type));
@@ -359,7 +364,8 @@ void SharingService::OnMessageSent(
     SharingSendMessageResult result,
     base::Optional<std::string> message_id) {
   if (result != SharingSendMessageResult::kSuccessful) {
-    InvokeSendMessageCallback(message_guid, message_type, result);
+    InvokeSendMessageCallback(message_guid, message_type, result,
+                              /*response=*/nullptr);
     return;
   }
 
@@ -369,7 +375,8 @@ void SharingService::OnMessageSent(
 
 void SharingService::OnAckReceived(
     chrome_browser_sharing::MessageType message_type,
-    const std::string& message_id) {
+    std::string message_id,
+    std::unique_ptr<chrome_browser_sharing::ResponseMessage> response) {
   auto times_iter = send_message_times_.find(message_id);
   if (times_iter != send_message_times_.end()) {
     LogSharingMessageAckTime(message_type,
@@ -384,20 +391,22 @@ void SharingService::OnAckReceived(
   std::string message_guid = std::move(iter->second);
   message_guids_.erase(iter);
   InvokeSendMessageCallback(message_guid, message_type,
-                            SharingSendMessageResult::kSuccessful);
+                            SharingSendMessageResult::kSuccessful,
+                            std::move(response));
 }
 
 void SharingService::InvokeSendMessageCallback(
     const std::string& message_guid,
     chrome_browser_sharing::MessageType message_type,
-    SharingSendMessageResult result) {
+    SharingSendMessageResult result,
+    std::unique_ptr<chrome_browser_sharing::ResponseMessage> response) {
   auto iter = send_message_callbacks_.find(message_guid);
   if (iter == send_message_callbacks_.end())
     return;
 
   SendMessageCallback callback = std::move(iter->second);
   send_message_callbacks_.erase(iter);
-  std::move(callback).Run(result);
+  std::move(callback).Run(result, std::move(response));
   LogSendSharingMessageResult(message_type, result);
 }
 

chrome/browser/sharing/sharing_service.h

@@ -55,11 +55,11 @@ enum class SharingDeviceRegistrationResult;
 // sharing messages to other devices.
 class SharingService : public KeyedService,
                        syncer::SyncServiceObserver,
-                       AckMessageHandler::AckMessageObserver,
                        syncer::DeviceInfoTracker::Observer {
  public:
-  using SendMessageCallback =
-      base::OnceCallback<void(SharingSendMessageResult)>;
+  using SendMessageCallback = base::OnceCallback<void(
+      SharingSendMessageResult,
+      std::unique_ptr<chrome_browser_sharing::ResponseMessage>)>;
   using SharingDeviceList = std::vector<std::unique_ptr<syncer::DeviceInfo>>;
 
   enum class State {
@@ -99,12 +99,16 @@ class SharingService : public KeyedService,
   // GetDeviceCandidates are ready.
   void AddDeviceCandidatesInitializedObserver(base::OnceClosure callback);
 
-  // Sends a message to the device specified by GUID.
-  // |callback| will be invoked with message_id if synchronous operation
-  // succeeded, or base::nullopt if operation failed.
+  // Sends a Sharing message to remote device.
+  // |device_guid|: Sync GUID of receiver device.
+  // |response_timeout|: Maximum amount of time waiting for a response before
+  // invoking |callback| with kAckTimeout.
+  // |message|: Message to be sent.
+  // |callback| will be invoked once a response has received from remote device,
+  // or if operation has failed or timed out.
   virtual void SendMessageToDevice(
       const std::string& device_guid,
-      base::TimeDelta time_to_live,
+      base::TimeDelta response_timeout,
       chrome_browser_sharing::SharingMessage message,
       SendMessageCallback callback);
 
@@ -128,9 +132,10 @@ class SharingService : public KeyedService,
   void OnStateChanged(syncer::SyncService* sync) override;
   void OnSyncCycleCompleted(syncer::SyncService* sync) override;
 
-  // AckMessageHandler::AckMessageObserver override.
-  void OnAckReceived(chrome_browser_sharing::MessageType message_type,
-                     const std::string& message_id) override;
+  void OnAckReceived(
+      chrome_browser_sharing::MessageType message_type,
+      std::string message_id,
+      std::unique_ptr<chrome_browser_sharing::ResponseMessage> response);
 
   // syncer::DeviceInfoTracker::Observer.
   void OnDeviceInfoChange() override;
@@ -150,7 +155,8 @@ class SharingService : public KeyedService,
   void InvokeSendMessageCallback(
       const std::string& message_guid,
       chrome_browser_sharing::MessageType message_type,
-      SharingSendMessageResult result);
+      SharingSendMessageResult result,
+      std::unique_ptr<chrome_browser_sharing::ResponseMessage> response);
 
   // Returns true if required sync feature is enabled.
   bool IsSyncEnabled() const;
@@ -207,7 +213,7 @@ class SharingService : public KeyedService,
 #endif  // defined(OS_ANDROID)
 
   PingMessageHandler ping_message_handler_;
-  AckMessageHandler ack_message_handler_;
+  std::unique_ptr<AckMessageHandler> ack_message_handler_;
 #if defined(OS_ANDROID)
   ClickToCallMessageHandler click_to_call_message_handler_;
   std::unique_ptr<SmsFetchRequestHandler> sms_fetch_request_handler_;

chrome/browser/sharing/sharing_service_proxy_android.cc

@@ -56,7 +56,9 @@ void SharingServiceProxyAndroid::SendSharedClipboardMessage(
       guid, kSendMessageTimeout, std::move(sharing_message),
       base::BindOnce(
           [](base::OnceCallback<void(int)> callback,
-             SharingSendMessageResult result) {
+             SharingSendMessageResult result,
+             std::unique_ptr<chrome_browser_sharing::ResponseMessage>
+                 response) {
             std::move(callback).Run(static_cast<int>(result));
           },
           std::move(callback)));

chrome/browser/sharing/sharing_service_unittest.cc

@@ -42,7 +42,7 @@ const char kFcmToken[] = "fcm_token";
 const char kDeviceName[] = "other_name";
 const char kMessageId[] = "message_id";
 const char kAuthorizedEntity[] = "authorized_entity";
-constexpr base::TimeDelta kTtl = base::TimeDelta::FromSeconds(10);
+constexpr base::TimeDelta kTimeout = base::TimeDelta::FromSeconds(15);
 const char kSenderFcmToken[] = "sender_fcm_token";
 const char kSenderP256dh[] = "sender_p256dh";
 const char kSenderAuthSecret[] = "sender_auth_secret";
@@ -194,14 +194,21 @@ class SharingServiceTest : public testing::Test {
     SharingSyncPreference::RegisterProfilePrefs(prefs_.registry());
   }
 
-  void OnMessageSent(SharingSendMessageResult result) {
+  void OnMessageSent(
+      SharingSendMessageResult result,
+      std::unique_ptr<chrome_browser_sharing::ResponseMessage> response) {
     send_message_result_ = base::make_optional(result);
+    send_message_response_ = std::move(response);
   }
 
-  base::Optional<SharingSendMessageResult> send_message_result() {
+  const base::Optional<SharingSendMessageResult>& send_message_result() {
     return send_message_result_;
   }
 
+  const chrome_browser_sharing::ResponseMessage* send_message_response() {
+    return send_message_response_.get();
+  }
+
   void OnDeviceCandidatesInitialized() {
     device_candidates_initialized_ = true;
   }
@@ -268,8 +275,17 @@ class SharingServiceTest : public testing::Test {
  private:
   std::unique_ptr<SharingService> sharing_service_ = nullptr;
   base::Optional<SharingSendMessageResult> send_message_result_;
+  std::unique_ptr<chrome_browser_sharing::ResponseMessage>
+      send_message_response_;
 };
 
+bool ProtoEquals(const google::protobuf::MessageLite& expected,
+                 const google::protobuf::MessageLite& actual) {
+  std::string expected_serialized, actual_serialized;
+  expected.SerializeToString(&expected_serialized);
+  actual.SerializeToString(&actual_serialized);
+  return expected_serialized == actual_serialized;
+}
 }  // namespace
 
 TEST_F(SharingServiceTest, SharedClipboard_IsAdded) {
@@ -400,7 +416,7 @@ TEST_F(SharingServiceTest, SendMessageToDeviceSuccess) {
       kAuthorizedEntity, base::Time::Now()));
 
   GetSharingService()->SendMessageToDevice(
-      id, kTtl, chrome_browser_sharing::SharingMessage(),
+      id, kTimeout, chrome_browser_sharing::SharingMessage(),
       base::BindOnce(&SharingServiceTest::OnMessageSent,
                      base::Unretained(this)));
 
@@ -422,9 +438,13 @@ TEST_F(SharingServiceTest, SendMessageToDeviceSuccess) {
 
   chrome_browser_sharing::SharingMessage ack_message;
   ack_message.mutable_ack_message()->set_original_message_id(kMessageId);
-  ack_message_handler->OnMessage(ack_message);
+  ack_message.mutable_ack_message()->mutable_response_message();
+  ack_message_handler->OnMessage(ack_message, base::DoNothing());
 
   EXPECT_EQ(SharingSendMessageResult::kSuccessful, send_message_result());
+  ASSERT_TRUE(send_message_response());
+  EXPECT_TRUE(ProtoEquals(ack_message.ack_message().response_message(),
+                          *send_message_response()));
 }
 
 TEST_F(SharingServiceTest, SendMessageToDeviceFCMNotResponding) {
@@ -442,7 +462,7 @@ TEST_F(SharingServiceTest, SendMessageToDeviceFCMNotResponding) {
   fake_gcm_driver_.set_should_respond(false);
 
   GetSharingService()->SendMessageToDevice(
-      id, kTtl, chrome_browser_sharing::SharingMessage(),
+      id, kTimeout, chrome_browser_sharing::SharingMessage(),
       base::BindOnce(&SharingServiceTest::OnMessageSent,
                      base::Unretained(this)));
 
@@ -451,7 +471,7 @@ TEST_F(SharingServiceTest, SendMessageToDeviceFCMNotResponding) {
   EXPECT_EQ(kFcmToken, fake_gcm_driver_.fcm_token());
 
   // Advance time so send message will expire.
-  task_environment_.FastForwardBy(kSendMessageTimeout);
+  task_environment_.FastForwardBy(kTimeout);
   EXPECT_EQ(SharingSendMessageResult::kAckTimeout, send_message_result());
 
   // Simulate ack message received by AckMessageHandler, which will be
@@ -462,7 +482,7 @@ TEST_F(SharingServiceTest, SendMessageToDeviceFCMNotResponding) {
 
   chrome_browser_sharing::SharingMessage ack_message;
   ack_message.mutable_ack_message()->set_original_message_id(kMessageId);
-  ack_message_handler->OnMessage(ack_message);
+  ack_message_handler->OnMessage(ack_message, base::DoNothing());
 
   EXPECT_EQ(SharingSendMessageResult::kAckTimeout, send_message_result());
 }
@@ -479,7 +499,7 @@ TEST_F(SharingServiceTest, SendMessageToDeviceExpired) {
       kAuthorizedEntity, base::Time::Now()));
 
   GetSharingService()->SendMessageToDevice(
-      id, kTtl, chrome_browser_sharing::SharingMessage(),
+      id, kTimeout, chrome_browser_sharing::SharingMessage(),
       base::BindOnce(&SharingServiceTest::OnMessageSent,
                      base::Unretained(this)));
 
@@ -499,7 +519,7 @@ TEST_F(SharingServiceTest, SendMessageToDeviceExpired) {
 
   chrome_browser_sharing::SharingMessage ack_message;
   ack_message.mutable_ack_message()->set_original_message_id(kMessageId);
-  ack_message_handler->OnMessage(ack_message);
+  ack_message_handler->OnMessage(ack_message, base::DoNothing());
 
   EXPECT_EQ(SharingSendMessageResult::kAckTimeout, send_message_result());
 }

chrome/browser/sharing/sharing_ui_controller.cc

@@ -177,7 +177,7 @@ void SharingUiController::SendMessageToDevice(
   UpdateIcon();
 
   sharing_service_->SendMessageToDevice(
-      device.guid(), kSharingMessageTTL, std::move(sharing_message),
+      device.guid(), kSendMessageTimeout, std::move(sharing_message),
       base::Bind(&SharingUiController::OnMessageSentToDevice,
                  weak_ptr_factory_.GetWeakPtr(), last_dialog_id_));
 }
@@ -223,7 +223,8 @@ base::string16 SharingUiController::GetTargetDeviceName() const {
 
 void SharingUiController::OnMessageSentToDevice(
     int dialog_id,
-    SharingSendMessageResult result) {
+    SharingSendMessageResult result,
+    std::unique_ptr<chrome_browser_sharing::ResponseMessage> response) {
   if (dialog_id != last_dialog_id_)
     return;
 

chrome/browser/sharing/sharing_ui_controller.h

@@ -121,7 +121,10 @@ class SharingUiController {
 
   // Called after a message got sent to a device. Shows a new error dialog if
   // |success| is false and updates the omnibox icon.
-  void OnMessageSentToDevice(int dialog_id, SharingSendMessageResult result);
+  void OnMessageSentToDevice(
+      int dialog_id,
+      SharingSendMessageResult result,
+      std::unique_ptr<chrome_browser_sharing::ResponseMessage> response);
 
   void OnAppsReceived(int dialog_id,
                       const base::Optional<url::Origin>& initiating_origin,

chrome/browser/sharing/sms/sms_fetch_request_handler.cc

@@ -5,14 +5,15 @@
 #include "chrome/browser/sharing/sms/sms_fetch_request_handler.h"
 
 #include "base/logging.h"
-#include "chrome/browser/sharing/proto/sms_fetch_request.pb.h"
+#include "chrome/browser/sharing/proto/sms_fetch_message.pb.h"
 
 SmsFetchRequestHandler::SmsFetchRequestHandler() = default;
 
 SmsFetchRequestHandler::~SmsFetchRequestHandler() = default;
 
 void SmsFetchRequestHandler::OnMessage(
-    const chrome_browser_sharing::SharingMessage& message) {
+    chrome_browser_sharing::SharingMessage message,
+    SharingMessageHandler::DoneCallback done_callback) {
   DCHECK(message.has_sms_fetch_request());
   // TODO(crbug.com/1015645): implementation left pending deliberately.
   NOTIMPLEMENTED();

chrome/browser/sharing/sms/sms_fetch_request_handler.h

@@ -16,8 +16,8 @@ class SmsFetchRequestHandler : public SharingMessageHandler {
   ~SmsFetchRequestHandler() override;
 
   // SharingMessageHandler
-  void OnMessage(
-      const chrome_browser_sharing::SharingMessage& message) override;
+  void OnMessage(chrome_browser_sharing::SharingMessage message,
+                 SharingMessageHandler::DoneCallback done_callback) override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SmsFetchRequestHandler);