Extract SharingMessageSender

SharingMessageSender replaces ResponseCallbackHelper. This ensures
SharingFCMSender does not need to surface any other API.

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

chrome/browser/BUILD.gn

@@ -1553,8 +1553,6 @@ jumbo_static_library("browser") {
     "sharing/features.h",
     "sharing/ping_message_handler.cc",
     "sharing/ping_message_handler.h",
-    "sharing/response_callback_helper.cc",
-    "sharing/response_callback_helper.h",
     "sharing/shared_clipboard/feature_flags.cc",
     "sharing/shared_clipboard/feature_flags.h",
     "sharing/shared_clipboard/shared_clipboard_message_handler.cc",
@@ -1569,6 +1567,8 @@ jumbo_static_library("browser") {
     "sharing/sharing_fcm_sender.cc",
     "sharing/sharing_fcm_sender.h",
     "sharing/sharing_message_handler.h",
+    "sharing/sharing_message_sender.cc",
+    "sharing/sharing_message_sender.h",
     "sharing/sharing_metrics.cc",
     "sharing/sharing_metrics.h",
     "sharing/sharing_send_message_result.h",

chrome/browser/sharing/ack_message_handler.cc

@@ -5,11 +5,12 @@
 #include "chrome/browser/sharing/ack_message_handler.h"
 
 #include "base/memory/ptr_util.h"
+#include "chrome/browser/sharing/sharing_message_sender.h"
 #include "components/sync/protocol/sharing_message.pb.h"
 
 AckMessageHandler::AckMessageHandler(
-    ResponseCallbackHelper* response_callback_helper)
-    : response_callback_helper_(response_callback_helper) {}
+    SharingMessageSender* sharing_message_sender)
+    : sharing_message_sender_(sharing_message_sender) {}
 
 AckMessageHandler::~AckMessageHandler() = default;
 
@@ -23,9 +24,9 @@ void AckMessageHandler::OnMessage(
   if (ack_message->has_response_message())
     response = base::WrapUnique(ack_message->release_response_message());
 
-  response_callback_helper_->OnFCMAckReceived(
-      ack_message->original_message_type(), ack_message->original_message_id(),
-      std::move(response));
+  sharing_message_sender_->OnAckReceived(ack_message->original_message_type(),
+                                         ack_message->original_message_id(),
+                                         std::move(response));
 
   std::move(done_callback).Run(/*response=*/nullptr);
 }

chrome/browser/sharing/ack_message_handler.h

@@ -10,13 +10,14 @@
 
 #include "base/callback_forward.h"
 #include "base/macros.h"
-#include "chrome/browser/sharing/response_callback_helper.h"
 #include "chrome/browser/sharing/sharing_message_handler.h"
 
+class SharingMessageSender;
+
 // Class to managae ack message and notify observers.
 class AckMessageHandler : public SharingMessageHandler {
  public:
-  explicit AckMessageHandler(ResponseCallbackHelper* response_callback_helper);
+  explicit AckMessageHandler(SharingMessageSender* sharing_message_sender);
   ~AckMessageHandler() override;
 
   // SharingMessageHandler implementation:
@@ -24,7 +25,7 @@ class AckMessageHandler : public SharingMessageHandler {
                  SharingMessageHandler::DoneCallback done_callback) override;
 
  private:
-  ResponseCallbackHelper* response_callback_helper_;
+  SharingMessageSender* sharing_message_sender_;
 
   DISALLOW_COPY_AND_ASSIGN(AckMessageHandler);
 };

chrome/browser/sharing/ack_message_handler_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/test/mock_callback.h"
+#include "chrome/browser/sharing/sharing_message_sender.h"
 #include "components/sync/protocol/sharing_message.pb.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -14,20 +15,20 @@
 namespace {
 constexpr char kTestMessageId[] = "test_message_id";
 
-class MockResponseCallbackHelper : public ResponseCallbackHelper {
+class MockSharingMessageSender : public SharingMessageSender {
  public:
-  MockResponseCallbackHelper() = default;
-  ~MockResponseCallbackHelper() override = default;
+  MockSharingMessageSender()
+      : SharingMessageSender(nullptr, nullptr, nullptr) {}
+  ~MockSharingMessageSender() override = default;
 
   MOCK_METHOD3(
-      OnFCMAckReceived,
+      OnAckReceived,
       void(chrome_browser_sharing::MessageType message_type,
            const std::string& fcm_message_id,
-           std::unique_ptr<ResponseCallbackHelper::SharingResponseMessage>
-               response));
+           std::unique_ptr<chrome_browser_sharing::ResponseMessage> response));
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(MockResponseCallbackHelper);
+  DISALLOW_COPY_AND_ASSIGN(MockSharingMessageSender);
 };
 
 class AckMessageHandlerTest : public testing::Test {
@@ -35,7 +36,7 @@ class AckMessageHandlerTest : public testing::Test {
   AckMessageHandlerTest()
       : ack_message_handler_(&mock_response_callback_helper_) {}
 
-  testing::NiceMock<MockResponseCallbackHelper> mock_response_callback_helper_;
+  testing::NiceMock<MockSharingMessageSender> mock_response_callback_helper_;
   AckMessageHandler ack_message_handler_;
 };
 
@@ -61,10 +62,10 @@ TEST_F(AckMessageHandlerTest, OnMessageNoResponse) {
   base::MockCallback<SharingMessageHandler::DoneCallback> done_callback;
   EXPECT_CALL(done_callback, Run(testing::Eq(nullptr)));
 
-  EXPECT_CALL(mock_response_callback_helper_,
-              OnFCMAckReceived(
-                  testing::Eq(chrome_browser_sharing::CLICK_TO_CALL_MESSAGE),
-                  testing::Eq(kTestMessageId), testing::Eq(nullptr)));
+  EXPECT_CALL(
+      mock_response_callback_helper_,
+      OnAckReceived(testing::Eq(chrome_browser_sharing::CLICK_TO_CALL_MESSAGE),
+                    testing::Eq(kTestMessageId), testing::Eq(nullptr)));
 
   ack_message_handler_.OnMessage(std::move(sharing_message),
                                  done_callback.Get());
@@ -86,9 +87,9 @@ TEST_F(AckMessageHandlerTest, OnMessageWithResponse) {
 
   EXPECT_CALL(
       mock_response_callback_helper_,
-      OnFCMAckReceived(
-          testing::Eq(chrome_browser_sharing::CLICK_TO_CALL_MESSAGE),
-          testing::Eq(kTestMessageId), ProtoEquals(response_message_copy)));
+      OnAckReceived(testing::Eq(chrome_browser_sharing::CLICK_TO_CALL_MESSAGE),
+                    testing::Eq(kTestMessageId),
+                    ProtoEquals(response_message_copy)));
 
   ack_message_handler_.OnMessage(std::move(sharing_message),
                                  done_callback.Get());

chrome/browser/sharing/click_to_call/click_to_call_utils_unittest.cc

@@ -89,7 +89,10 @@ class ClickToCallUtilsTest : public testing::Test {
         /* vapid_key_manager= */ nullptr,
         std::make_unique<MockSharingDeviceRegistration>(),
         /* fcm_sender= */ nullptr,
-        std::make_unique<SharingFCMHandler>(nullptr, nullptr, nullptr),
+        std::make_unique<SharingFCMHandler>(
+            /*gcm_driver=*/nullptr, /*sharing_fcm_sender=*/nullptr,
+            /*sync_preference=nullptr*/ nullptr),
+        /*message_sender_=*/nullptr,
         /* gcm_driver= */ nullptr,
         /* device_info_tracker= */ nullptr,
         /* local_device_info_provider= */ nullptr,

chrome/browser/sharing/mock_sharing_service.cc

@@ -22,6 +22,7 @@ MockSharingService::MockSharingService()
           std::make_unique<SharingFCMHandler>(/*gcm_driver=*/nullptr,
                                               /*sharing_fcm_sender=*/nullptr,
                                               /*sync_preference=*/nullptr),
+          /*message_sender=*/nullptr,
           /*gcm_driver=*/nullptr,
           /*device_info_tracker=*/nullptr,
           /*local_device_info_provider=*/nullptr,

chrome/browser/sharing/mock_sharing_service.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_SHARING_MOCK_SHARING_SERVICE_H_
 #define CHROME_BROWSER_SHARING_MOCK_SHARING_SERVICE_H_
 
+#include "chrome/browser/sharing/sharing_message_sender.h"
 #include "chrome/browser/sharing/sharing_service.h"
 
 #include "testing/gmock/include/gmock/gmock.h"
@@ -23,7 +24,7 @@ class MockSharingService : public SharingService {
                void(const std::string& device_guid,
                     base::TimeDelta response_timeout,
                     chrome_browser_sharing::SharingMessage message,
-                    ResponseCallbackHelper::ResponseCallback callback));
+                    SharingMessageSender::ResponseCallback callback));
 
   MOCK_CONST_METHOD1(
       GetDeviceByGuid,

chrome/browser/sharing/response_callback_helper.cc

-// Copyright 2019 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.
-
-#include "chrome/browser/sharing/response_callback_helper.h"
-
-#include "base/callback.h"
-#include "chrome/browser/sharing/sharing_metrics.h"
-#include "chrome/browser/sharing/sharing_send_message_result.h"
-#include "components/sync/protocol/sharing_message.pb.h"
-
-ResponseCallbackHelper::ResponseCallbackHelper() = default;
-ResponseCallbackHelper::~ResponseCallbackHelper() = default;
-
-void ResponseCallbackHelper::RegisterCallback(const std::string& message_guid,
-                                              ResponseCallback callback) {
-  DCHECK(!callbacks_.count(message_guid));
-  callbacks_.emplace(message_guid, std::move(callback));
-}
-
-void ResponseCallbackHelper::RunCallback(
-    const std::string& message_guid,
-    chrome_browser_sharing::MessageType message_type,
-    SharingSendMessageResult result,
-    std::unique_ptr<SharingResponseMessage> response) {
-  auto iter = callbacks_.find(message_guid);
-  if (iter == callbacks_.end())
-    return;
-
-  LogSendSharingMessageResult(message_type, result);
-  ResponseCallback callback = std::move(iter->second);
-  callbacks_.erase(iter);
-  std::move(callback).Run(result, std::move(response));
-}
-
-void ResponseCallbackHelper::OnFCMMessageSent(
-    base::TimeTicks start_time,
-    const std::string& message_guid,
-    chrome_browser_sharing::MessageType message_type,
-    SharingSendMessageResult result,
-    base::Optional<std::string> fcm_message_id) {
-  if (result != SharingSendMessageResult::kSuccessful) {
-    RunCallback(message_guid, message_type, result, /*response=*/nullptr);
-    return;
-  }
-
-  message_sent_time_.emplace(message_guid, start_time);
-  message_guids_.emplace(*fcm_message_id, message_guid);
-}
-
-void ResponseCallbackHelper::OnFCMAckReceived(
-    chrome_browser_sharing::MessageType message_type,
-    const std::string& fcm_message_id,
-    std::unique_ptr<SharingResponseMessage> response) {
-  auto iter = message_guids_.find(fcm_message_id);
-  if (iter == message_guids_.end())
-    return;
-
-  std::string message_guid = std::move(iter->second);
-  message_guids_.erase(iter);
-
-  auto times_iter = message_sent_time_.find(message_guid);
-  if (times_iter != message_sent_time_.end()) {
-    LogSharingMessageAckTime(message_type,
-                             base::TimeTicks::Now() - times_iter->second);
-    message_sent_time_.erase(times_iter);
-  }
-
-  RunCallback(message_guid, message_type, SharingSendMessageResult::kSuccessful,
-              std::move(response));
-}

chrome/browser/sharing/response_callback_helper.h

-// Copyright 2019 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_SHARING_RESPONSE_CALLBACK_HELPER_H_
-#define CHROME_BROWSER_SHARING_RESPONSE_CALLBACK_HELPER_H_
-
-#include <map>
-#include <memory>
-#include <string>
-
-#include "base/callback_forward.h"
-#include "base/macros.h"
-#include "base/optional.h"
-#include "base/time/time.h"
-
-namespace chrome_browser_sharing {
-enum MessageType : int;
-class ResponseMessage;
-}  // namespace chrome_browser_sharing
-
-enum class SharingSendMessageResult;
-
-// ResponseCallbackHelper stores callbacks for every message sent. The
-// registered callback is run when an ack is received for the same message_guid.
-class ResponseCallbackHelper {
- public:
-  using SharingResponseMessage = chrome_browser_sharing::ResponseMessage;
-  using ResponseCallback =
-      base::OnceCallback<void(SharingSendMessageResult,
-                              std::unique_ptr<SharingResponseMessage>)>;
-
-  ResponseCallbackHelper();
-  virtual ~ResponseCallbackHelper();
-
-  // Registers a |callback| to be run when a response is received for
-  // |message_guid|.
-  void RegisterCallback(const std::string& message_guid,
-                        ResponseCallback callback);
-
-  // Runs the registered |callback| for |message_guid|. Multiple calls for the
-  // same guid or calling RunCallback without registering a callback results in
-  // noops.
-  void RunCallback(const std::string& message_guid,
-                   chrome_browser_sharing::MessageType message_type,
-                   SharingSendMessageResult result,
-                   std::unique_ptr<SharingResponseMessage> response);
-
-  // Called after FCM has sent the message successfully or if an error was
-  // encountered while sending the message.
-  void OnFCMMessageSent(base::TimeTicks start_time,
-                        const std::string& message_guid,
-                        chrome_browser_sharing::MessageType message_type,
-                        SharingSendMessageResult result,
-                        base::Optional<std::string> fcm_message_id);
-
-  // Called when an AckMessage is received from FCM.
-  virtual void OnFCMAckReceived(
-      chrome_browser_sharing::MessageType message_type,
-      const std::string& fcm_message_id,
-      std::unique_ptr<SharingResponseMessage> response);
-
- private:
-  // Maps FCM id to random message GUID.
-  std::map<std::string, std::string> message_guids_;
-  // Maps GUID to the message sending time.
-  std::map<std::string, base::TimeTicks> message_sent_time_;
-  // Maps GUID to a callback that is run when a response is received.
-  std::map<std::string, ResponseCallback> callbacks_;
-
-  DISALLOW_COPY_AND_ASSIGN(ResponseCallbackHelper);
-};
-
-#endif  // CHROME_BROWSER_SHARING_RESPONSE_CALLBACK_HELPER_H_

chrome/browser/sharing/response_callback_helper_unittest.cc

-// Copyright 2019 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.
-
-#include "chrome/browser/sharing/response_callback_helper.h"
-
-#include <memory>
-
-#include "base/bind_helpers.h"
-#include "base/guid.h"
-#include "base/strings/strcat.h"
-#include "base/test/bind_test_util.h"
-#include "base/test/metrics/histogram_tester.h"
-#include "base/test/mock_callback.h"
-#include "chrome/browser/sharing/sharing_send_message_result.h"
-#include "components/sync/protocol/sharing_message.pb.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-TEST(ResponseCallbackHelperTest, RunCallbackForUnregisteredGUID) {
-  ResponseCallbackHelper callback_helper;
-  base::HistogramTester histogram_tester;
-  const std::string message_guid1 = base::GenerateGUID();
-  const std::string message_guid2 = base::GenerateGUID();
-
-  int times_called = 0;
-  auto callback = base::BindLambdaForTesting(
-      [&](SharingSendMessageResult result,
-          std::unique_ptr<chrome_browser_sharing::ResponseMessage> response) {
-        times_called++;
-      });
-
-  callback_helper.RegisterCallback(message_guid1, std::move(callback));
-
-  callback_helper.RunCallback(message_guid2,
-                              chrome_browser_sharing::CLICK_TO_CALL_MESSAGE,
-                              SharingSendMessageResult::kSuccessful,
-                              /*response=*/nullptr);
-  ASSERT_EQ(0, times_called);
-  histogram_tester.ExpectTotalCount("Sharing.SendMessageResult", 0);
-
-  callback_helper.RunCallback(message_guid1,
-                              chrome_browser_sharing::CLICK_TO_CALL_MESSAGE,
-                              SharingSendMessageResult::kSuccessful,
-                              /*response=*/nullptr);
-  EXPECT_EQ(1, times_called);
-  histogram_tester.ExpectTotalCount("Sharing.SendMessageResult", 1);
-}
-
-TEST(ResponseCallbackHelperTest, RegisterAndRunMultipleCallbacks) {
-  ResponseCallbackHelper callback_helper;
-  base::HistogramTester histogram_tester;
-  const std::string message_guid1 = base::GenerateGUID();
-  const std::string message_guid2 = base::GenerateGUID();
-
-  int times_called = 0;
-  auto callback = base::BindLambdaForTesting(
-      [&](SharingSendMessageResult result,
-          std::unique_ptr<chrome_browser_sharing::ResponseMessage> response) {
-        times_called++;
-      });
-
-  callback_helper.RegisterCallback(message_guid1, std::move(callback));
-  callback_helper.RegisterCallback(message_guid2, base::DoNothing());
-
-  callback_helper.RunCallback(message_guid2,
-                              chrome_browser_sharing::CLICK_TO_CALL_MESSAGE,
-                              SharingSendMessageResult::kSuccessful,
-                              /*response=*/nullptr);
-  ASSERT_EQ(0, times_called);
-  histogram_tester.ExpectTotalCount("Sharing.SendMessageResult", 1);
-
-  callback_helper.RunCallback(message_guid1,
-                              chrome_browser_sharing::CLICK_TO_CALL_MESSAGE,
-                              SharingSendMessageResult::kSuccessful,
-                              /*response=*/nullptr);
-  EXPECT_EQ(1, times_called);
-  histogram_tester.ExpectTotalCount("Sharing.SendMessageResult", 2);
-}
-
-TEST(ResponseCallbackHelperTest, SimulateSuccessfulMessageSent) {
-  ResponseCallbackHelper callback_helper;
-  base::HistogramTester histogram_tester;
-  const std::string message_guid = "message_guid";
-  const std::string fcm_message_id = "fcm_message_id";
-  const chrome_browser_sharing::MessageType message_type =
-      chrome_browser_sharing::CLICK_TO_CALL_MESSAGE;
-
-  base::MockCallback<ResponseCallbackHelper::ResponseCallback> mock_callback;
-  EXPECT_CALL(mock_callback,
-              Run(testing::Eq(SharingSendMessageResult::kSuccessful),
-                  testing::Eq(nullptr)));
-
-  callback_helper.RegisterCallback(message_guid, mock_callback.Get());
-  callback_helper.OnFCMMessageSent(
-      base::TimeTicks::Now(), message_guid, message_type,
-      SharingSendMessageResult::kSuccessful, fcm_message_id);
-
-  callback_helper.OnFCMAckReceived(message_type, fcm_message_id, nullptr);
-
-  const std::string ack_time_histogram_name =
-      base::StrCat({"Sharing.MessageAckTime.",
-                    chrome_browser_sharing::MessageType_Name(message_type)});
-
-  histogram_tester.ExpectTotalCount("Sharing.SendMessageResult", 1);
-  histogram_tester.ExpectTotalCount(ack_time_histogram_name, 1);
-}

chrome/browser/sharing/shared_clipboard/shared_clipboard_utils_unittest.cc

@@ -73,7 +73,10 @@ class SharedClipboardUtilsTest : public testing::Test {
         /* vapid_key_manager= */ nullptr,
         std::make_unique<MockSharingDeviceRegistration>(),
         /* fcm_sender= */ nullptr,
-        std::make_unique<SharingFCMHandler>(nullptr, nullptr, nullptr),
+        std::make_unique<SharingFCMHandler>(
+            /*gcm_driver=*/nullptr, /*sharing_fcm_sender=*/nullptr,
+            /*sync_preference=nullptr*/ nullptr),
+        /*message_sender_=*/nullptr,
         /* gcm_driver= */ nullptr,
         /* device_info_tracker= */ nullptr,
         /* local_device_info_provider= */ nullptr,

chrome/browser/sharing/sharing_fcm_handler.cc

@@ -183,7 +183,6 @@ void SharingFCMHandler::SendAckMessage(
 
   sharing_fcm_sender_->SendMessageToDevice(
       std::move(*sharing_info), kAckTimeToLive, std::move(sharing_message),
-      /*sender_device_info=*/nullptr,
       base::BindOnce(&SharingFCMHandler::OnAckMessageSent,
                      weak_ptr_factory_.GetWeakPtr(),
                      std::move(original_message_id), original_message_type));

chrome/browser/sharing/sharing_fcm_handler_unittest.cc

@@ -51,11 +51,10 @@ class MockSharingFCMSender : public SharingFCMSender {
                          /*vapid_key_manager=*/nullptr) {}
   ~MockSharingFCMSender() override {}
 
-  MOCK_METHOD5(SendMessageToDevice,
+  MOCK_METHOD4(SendMessageToDevice,
                void(syncer::DeviceInfo::SharingInfo target,
                     base::TimeDelta time_to_live,
                     SharingMessage message,
-                    std::unique_ptr<syncer::DeviceInfo> sender_device_info,
                     SendMessageCallback callback));
 };
 
@@ -126,7 +125,7 @@ TEST_F(SharingFCMHandlerTest, AckMessageHandler) {
 
   EXPECT_CALL(mock_sharing_message_handler_,
               OnMessage(ProtoEquals(sharing_message), _));
-  EXPECT_CALL(mock_sharing_fcm_sender_, SendMessageToDevice(_, _, _, _, _))
+  EXPECT_CALL(mock_sharing_fcm_sender_, SendMessageToDevice(_, _, _, _))
       .Times(0);
   sharing_fcm_handler_->AddSharingHandler(SharingMessage::kAckMessage,
                                           &mock_sharing_message_handler_);
@@ -152,7 +151,7 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandler) {
 
   // Tests OnMessage flow in SharingFCMHandler when no handler is registered.
   EXPECT_CALL(mock_sharing_message_handler_, OnMessage(_, _)).Times(0);
-  EXPECT_CALL(mock_sharing_fcm_sender_, SendMessageToDevice(_, _, _, _, _))
+  EXPECT_CALL(mock_sharing_fcm_sender_, SendMessageToDevice(_, _, _, _))
       .Times(0);
   sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
 
@@ -167,8 +166,7 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandler) {
   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), _));
+                                  ProtoEquals(sharing_ack_message), _));
   sharing_fcm_handler_->AddSharingHandler(SharingMessage::kPingMessage,
                                           &mock_sharing_message_handler_);
   sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
@@ -176,7 +174,7 @@ 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_fcm_sender_, SendMessageToDevice(_, _, _, _, _))
+  EXPECT_CALL(mock_sharing_fcm_sender_, SendMessageToDevice(_, _, _, _))
       .Times(0);
   sharing_fcm_handler_->RemoveSharingHandler(SharingMessage::kPingMessage);
   sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
@@ -211,8 +209,7 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandlerWithResponse) {
   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), _));
+                                  ProtoEquals(sharing_ack_message), _));
   sharing_fcm_handler_->AddSharingHandler(SharingMessage::kPingMessage,
                                           &mock_sharing_message_handler_);
   sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
@@ -246,8 +243,7 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandlerSecondaryUser) {
           }));
   EXPECT_CALL(mock_sharing_fcm_sender_,
               SendMessageToDevice(DeviceMatcher(), testing::Eq(kAckTimeToLive),
-                                  ProtoEquals(sharing_ack_message),
-                                  testing::Eq(nullptr), _));
+                                  ProtoEquals(sharing_ack_message), _));
   sharing_fcm_handler_->AddSharingHandler(SharingMessage::kPingMessage,
                                           &mock_sharing_message_handler_);
   sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
@@ -282,8 +278,7 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandlerWithRecipientInfo) {
           }));
   EXPECT_CALL(mock_sharing_fcm_sender_,
               SendMessageToDevice(DeviceMatcher(), testing::Eq(kAckTimeToLive),
-                                  ProtoEquals(sharing_ack_message),
-                                  testing::Eq(nullptr), _));
+                                  ProtoEquals(sharing_ack_message), _));
   sharing_fcm_handler_->AddSharingHandler(SharingMessage::kPingMessage,
                                           &mock_sharing_message_handler_);
   sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);

chrome/browser/sharing/sharing_fcm_sender.cc

@@ -26,7 +26,6 @@ void SharingFCMSender::SendMessageToDevice(
     syncer::DeviceInfo::SharingInfo target,
     base::TimeDelta time_to_live,
     SharingMessage message,
-    std::unique_ptr<syncer::DeviceInfo> sender_device_info,
     SendMessageCallback callback) {
   base::Optional<SharingSyncPreference::FCMRegistration> fcm_registration =
       sync_preference_->GetFCMRegistration();
@@ -45,22 +44,6 @@ void SharingFCMSender::SendMessageToDevice(
     return;
   }
 
-  if (message.payload_case() != SharingMessage::kAckMessage) {
-    DCHECK(sender_device_info);
-
-    message.set_sender_guid(sender_device_info->guid());
-    message.set_sender_device_name(sender_device_info->client_name());
-
-    base::Optional<syncer::DeviceInfo::SharingInfo> sharing_info =
-        sender_device_info->sharing_info();
-    DCHECK(sharing_info);
-
-    auto* sender_info = message.mutable_sender_info();
-    sender_info->set_fcm_token(sharing_info->fcm_token);
-    sender_info->set_p256dh(sharing_info->p256dh);
-    sender_info->set_auth_secret(sharing_info->auth_secret);
-  }
-
   gcm::WebPushMessage web_push_message;
   web_push_message.time_to_live = time_to_live.InSeconds();
   web_push_message.urgency = gcm::WebPushMessage::Urgency::kHigh;

chrome/browser/sharing/sharing_fcm_sender.h

@@ -47,12 +47,10 @@ class SharingFCMSender {
   // Sends a |message| to device identified by |target|, which expires
   // after |time_to_live| seconds. |callback| will be invoked with message_id if
   // asynchronous operation succeeded, or base::nullopt if operation failed.
-  // |sender_device_info| must be provided except for sending ack messages.
   virtual void SendMessageToDevice(
       syncer::DeviceInfo::SharingInfo target,
       base::TimeDelta time_to_live,
       SharingMessage message,
-      std::unique_ptr<syncer::DeviceInfo> sender_device_info,
       SendMessageCallback callback);
 
  private:

chrome/browser/sharing/sharing_fcm_sender_unittest.cc

@@ -25,12 +25,7 @@ const char kMessageId[] = "message_id";
 const char kFcmToken[] = "fcm_token";
 const char kP256dh[] = "p256dh";
 const char kAuthSecret[] = "auth_secret";
-const char kSenderFcmToken[] = "sender_fcm_token";
-const char kSenderP256dh[] = "sender_p256dh";
-const char kSenderAuthSecret[] = "sender_auth_secret";
 const char kAuthorizedEntity[] = "authorized_entity";
-const char kLocalGuid[] = "id";
-const char kLocalClientName[] = "name";
 const int kTtlSeconds = 10;
 
 class FakeGCMDriver : public gcm::FakeGCMDriver {
@@ -136,7 +131,7 @@ TEST_F(SharingFCMSenderTest, NoFcmRegistration) {
   sharing_message.mutable_ack_message();
   sharing_fcm_sender_.SendMessageToDevice(
       std::move(target), base::TimeDelta::FromSeconds(kTtlSeconds),
-      std::move(sharing_message), /*sender_device_info=*/nullptr,
+      std::move(sharing_message),
       base::BindOnce(&SharingFCMSenderTest::OnMessageSent,
                      base::Unretained(this), &result, &message_id));
 
@@ -160,7 +155,7 @@ TEST_F(SharingFCMSenderTest, NoVapidKey) {
   sharing_message.mutable_ack_message();
   sharing_fcm_sender_.SendMessageToDevice(
       std::move(target), base::TimeDelta::FromSeconds(kTtlSeconds),
-      std::move(sharing_message), /*sender_device_info=*/nullptr,
+      std::move(sharing_message),
       base::BindOnce(&SharingFCMSenderTest::OnMessageSent,
                      base::Unretained(this), &result, &message_id));
 
@@ -199,16 +194,6 @@ class SharingFCMSenderResultTest
 TEST_P(SharingFCMSenderResultTest, ResultTest) {
   sync_prefs_.SetFCMRegistration(SharingSyncPreference::FCMRegistration(
       kAuthorizedEntity, base::Time::Now()));
-  std::unique_ptr<syncer::DeviceInfo> sender_device_info =
-      std::make_unique<syncer::DeviceInfo>(
-          "id", "name", "chrome_version", "user_agent",
-          sync_pb::SyncEnums_DeviceType_TYPE_LINUX, "device_id",
-          base::SysInfo::HardwareInfo{"model", "manufacturer", "serial"},
-          /*last_updated_timestamp=*/base::Time::Now(),
-          /*send_tab_to_self_receiving_enabled=*/false,
-          syncer::DeviceInfo::SharingInfo(
-              kSenderFcmToken, kSenderP256dh, kSenderAuthSecret,
-              std::set<sync_pb::SharingSpecificFields::EnabledFeatures>()));
   fake_gcm_driver_.set_result(GetParam().web_push_result);
 
   std::unique_ptr<crypto::ECPrivateKey> vapid_key =
@@ -226,7 +211,7 @@ TEST_P(SharingFCMSenderResultTest, ResultTest) {
   sharing_message.mutable_ping_message();
   sharing_fcm_sender_.SendMessageToDevice(
       std::move(target), base::TimeDelta::FromSeconds(kTtlSeconds),
-      std::move(sharing_message), std::move(sender_device_info),
+      std::move(sharing_message),
       base::BindOnce(&SharingFCMSenderTest::OnMessageSent,
                      base::Unretained(this), &result, &message_id));
 
@@ -245,12 +230,6 @@ TEST_P(SharingFCMSenderResultTest, ResultTest) {
   chrome_browser_sharing::SharingMessage message_sent;
   message_sent.ParseFromString(fake_gcm_driver_.message().payload);
   EXPECT_TRUE(message_sent.has_ping_message());
-  EXPECT_EQ(kLocalGuid, message_sent.sender_guid());
-  EXPECT_EQ(kLocalClientName, message_sent.sender_device_name());
-  EXPECT_TRUE(message_sent.has_sender_info());
-  EXPECT_EQ(kSenderFcmToken, message_sent.sender_info().fcm_token());
-  EXPECT_EQ(kSenderP256dh, message_sent.sender_info().p256dh());
-  EXPECT_EQ(kSenderAuthSecret, message_sent.sender_info().auth_secret());
 
   EXPECT_EQ(GetParam().expected_result, result);
   EXPECT_EQ(kMessageId, message_id);

chrome/browser/sharing/sharing_message_sender.cc

+// Copyright 2019 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.
+
+#include "chrome/browser/sharing/sharing_message_sender.h"
+#include "base/guid.h"
+#include "base/task/post_task.h"
+#include "chrome/browser/sharing/sharing_fcm_sender.h"
+#include "chrome/browser/sharing/sharing_metrics.h"
+#include "chrome/browser/sharing/sharing_sync_preference.h"
+#include "chrome/browser/sharing/sharing_utils.h"
+#include "components/sync_device_info/local_device_info_provider.h"
+#include "content/public/browser/browser_task_traits.h"
+
+SharingMessageSender::SharingMessageSender(
+    SharingFCMSender* sharing_fcm_sender,
+    SharingSyncPreference* sync_prefs,
+    syncer::LocalDeviceInfoProvider* local_device_info_provider)
+    : fcm_sender_(sharing_fcm_sender),
+      sync_prefs_(sync_prefs),
+      local_device_info_provider_(local_device_info_provider) {}
+
+SharingMessageSender::~SharingMessageSender() = default;
+
+void SharingMessageSender::SendMessageToDevice(
+    const std::string& device_guid,
+    base::TimeDelta response_timeout,
+    chrome_browser_sharing::SharingMessage message,
+    ResponseCallback callback) {
+  DCHECK(message.payload_case() !=
+         chrome_browser_sharing::SharingMessage::kAckMessage);
+
+  std::string message_guid = base::GenerateGUID();
+  send_message_callbacks_.emplace(message_guid, std::move(callback));
+  chrome_browser_sharing::MessageType message_type =
+      SharingPayloadCaseToMessageType(message.payload_case());
+
+  base::PostDelayedTask(
+      FROM_HERE, {base::TaskPriority::USER_VISIBLE, content::BrowserThread::UI},
+      base::BindOnce(&SharingMessageSender::InvokeSendMessageCallback,
+                     weak_ptr_factory_.GetWeakPtr(), message_guid, message_type,
+                     SharingSendMessageResult::kAckTimeout,
+                     /*response=*/nullptr),
+      response_timeout);
+
+  // TODO(crbug/1015411): Here we assume caller gets |device_guid| from
+  // GetDeviceCandidates, so both DeviceInfoTracker and LocalDeviceInfoProvider
+  // are already ready. It's better to queue up the message and wait until
+  // DeviceInfoTracker and LocalDeviceInfoProvider are ready.
+  base::Optional<syncer::DeviceInfo::SharingInfo> target_sharing_info =
+      sync_prefs_->GetSharingInfo(device_guid);
+  if (!target_sharing_info) {
+    InvokeSendMessageCallback(message_guid, message_type,
+                              SharingSendMessageResult::kDeviceNotFound,
+                              /*response=*/nullptr);
+    return;
+  }
+
+  const syncer::DeviceInfo* local_device_info =
+      local_device_info_provider_->GetLocalDeviceInfo();
+  if (!local_device_info) {
+    InvokeSendMessageCallback(message_guid, message_type,
+                              SharingSendMessageResult::kInternalError,
+                              /*response=*/nullptr);
+    return;
+  }
+
+  base::Optional<syncer::DeviceInfo::SharingInfo> sharing_info =
+      sync_prefs_->GetLocalSharingInfo(local_device_info);
+  if (!sharing_info) {
+    InvokeSendMessageCallback(message_guid, message_type,
+                              SharingSendMessageResult::kInternalError,
+                              /*response=*/nullptr);
+    return;
+  }
+
+  message.set_sender_guid(local_device_info->guid());
+  message.set_sender_device_name(
+      GetSharingDeviceNames(local_device_info).full_name);
+
+  auto* sender_info = message.mutable_sender_info();
+  sender_info->set_fcm_token(sharing_info->fcm_token);
+  sender_info->set_p256dh(sharing_info->p256dh);
+  sender_info->set_auth_secret(sharing_info->auth_secret);
+
+  DCHECK_GE(response_timeout, kAckTimeToLive);
+  fcm_sender_->SendMessageToDevice(
+      std::move(*target_sharing_info), response_timeout - kAckTimeToLive,
+      std::move(message),
+      base::BindOnce(&SharingMessageSender::OnMessageSent,
+                     weak_ptr_factory_.GetWeakPtr(), base::TimeTicks::Now(),
+                     message_guid, message_type));
+}
+
+void SharingMessageSender::OnMessageSent(
+    base::TimeTicks start_time,
+    const std::string& message_guid,
+    chrome_browser_sharing::MessageType message_type,
+    SharingSendMessageResult result,
+    base::Optional<std::string> message_id) {
+  if (result != SharingSendMessageResult::kSuccessful) {
+    InvokeSendMessageCallback(message_guid, message_type, result,
+                              /*response=*/nullptr);
+    return;
+  }
+
+  send_message_times_.emplace(*message_id, start_time);
+  message_guids_.emplace(*message_id, message_guid);
+}
+
+void SharingMessageSender::OnAckReceived(
+    chrome_browser_sharing::MessageType message_type,
+    const 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,
+                             base::TimeTicks::Now() - times_iter->second);
+    send_message_times_.erase(times_iter);
+  }
+
+  auto iter = message_guids_.find(message_id);
+  if (iter == message_guids_.end())
+    return;
+
+  std::string message_guid = std::move(iter->second);
+  message_guids_.erase(iter);
+  InvokeSendMessageCallback(message_guid, message_type,
+                            SharingSendMessageResult::kSuccessful,
+                            std::move(response));
+}
+
+void SharingMessageSender::InvokeSendMessageCallback(
+    const std::string& message_guid,
+    chrome_browser_sharing::MessageType message_type,
+    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;
+
+  ResponseCallback callback = std::move(iter->second);
+  send_message_callbacks_.erase(iter);
+  std::move(callback).Run(result, std::move(response));
+  LogSendSharingMessageResult(message_type, result);
+}

chrome/browser/sharing/sharing_message_sender.h

+// Copyright 2019 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_SHARING_SHARING_MESSAGE_SENDER_H_
+#define CHROME_BROWSER_SHARING_SHARING_MESSAGE_SENDER_H_
+
+#include <map>
+#include <string>
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/optional.h"
+#include "base/time/time.h"
+
+namespace chrome_browser_sharing {
+enum MessageType : int;
+class ResponseMessage;
+class SharingMessage;
+}  // namespace chrome_browser_sharing
+
+namespace syncer {
+class LocalDeviceInfoProvider;
+}  // namespace syncer
+
+class SharingFCMSender;
+class SharingSyncPreference;
+enum class SharingSendMessageResult;
+
+class SharingMessageSender {
+ public:
+  using ResponseCallback = base::OnceCallback<void(
+      SharingSendMessageResult,
+      std::unique_ptr<chrome_browser_sharing::ResponseMessage>)>;
+
+  SharingMessageSender(
+      SharingFCMSender* sharing_fcm_sender,
+      SharingSyncPreference* sync_prefs,
+      syncer::LocalDeviceInfoProvider* local_device_info_provider);
+  virtual ~SharingMessageSender();
+
+  virtual void SendMessageToDevice(
+      const std::string& device_guid,
+      base::TimeDelta response_timeout,
+      chrome_browser_sharing::SharingMessage message,
+      ResponseCallback callback);
+
+  virtual void OnAckReceived(
+      chrome_browser_sharing::MessageType message_type,
+      const std::string& message_id,
+      std::unique_ptr<chrome_browser_sharing::ResponseMessage> response);
+
+ private:
+  void OnMessageSent(base::TimeTicks start_time,
+                     const std::string& message_guid,
+                     chrome_browser_sharing::MessageType message_type,
+                     SharingSendMessageResult result,
+                     base::Optional<std::string> message_id);
+
+  void InvokeSendMessageCallback(
+      const std::string& message_guid,
+      chrome_browser_sharing::MessageType message_type,
+      SharingSendMessageResult result,
+      std::unique_ptr<chrome_browser_sharing::ResponseMessage> response);
+
+  SharingFCMSender* fcm_sender_;
+  SharingSyncPreference* sync_prefs_;
+  syncer::LocalDeviceInfoProvider* local_device_info_provider_;
+
+  // Map of random GUID to SendMessageCallback.
+  std::map<std::string, ResponseCallback> send_message_callbacks_;
+  // Map of FCM message_id to time at start of send message request to FCM.
+  std::map<std::string, base::TimeTicks> send_message_times_;
+  // Map of FCM message_id to random GUID.
+  std::map<std::string, std::string> message_guids_;
+
+  base::WeakPtrFactory<SharingMessageSender> weak_ptr_factory_{this};
+
+  DISALLOW_COPY_AND_ASSIGN(SharingMessageSender);
+};
+
+#endif  // CHROME_BROWSER_SHARING_SHARING_MESSAGE_SENDER_H_

chrome/browser/sharing/sharing_service.cc

@@ -49,27 +49,13 @@
 #include "chrome/browser/sharing/shared_clipboard/remote_copy_message_handler.h"
 #endif
 
-namespace {
-// Clones device with new device name.
-std::unique_ptr<syncer::DeviceInfo> CloneDevice(
-    const syncer::DeviceInfo* device,
-    const std::string& device_name) {
-  return std::make_unique<syncer::DeviceInfo>(
-      device->guid(), device_name, device->chrome_version(),
-      device->sync_user_agent(), device->device_type(),
-      device->signin_scoped_device_id(), device->hardware_info(),
-      device->last_updated_timestamp(),
-      device->send_tab_to_self_receiving_enabled(), device->sharing_info());
-}
-
-}  // namespace
-
 SharingService::SharingService(
     std::unique_ptr<SharingSyncPreference> sync_prefs,
     std::unique_ptr<VapidKeyManager> vapid_key_manager,
     std::unique_ptr<SharingDeviceRegistration> sharing_device_registration,
     std::unique_ptr<SharingFCMSender> fcm_sender,
     std::unique_ptr<SharingFCMHandler> fcm_handler,
+    std::unique_ptr<SharingMessageSender> message_sender,
     gcm::GCMDriver* gcm_driver,
     syncer::DeviceInfoTracker* device_info_tracker,
     syncer::LocalDeviceInfoProvider* local_device_info_provider,
@@ -80,6 +66,7 @@ SharingService::SharingService(
       sharing_device_registration_(std::move(sharing_device_registration)),
       fcm_sender_(std::move(fcm_sender)),
       fcm_handler_(std::move(fcm_handler)),
+      message_sender_(std::move(message_sender)),
       device_info_tracker_(device_info_tracker),
       local_device_info_provider_(local_device_info_provider),
       sync_service_(sync_service),
@@ -104,7 +91,7 @@ SharingService::SharingService(
       &ping_message_handler_);
 
   ack_message_handler_ =
-      std::make_unique<AckMessageHandler>(&response_callback_helper_);
+      std::make_unique<AckMessageHandler>(message_sender_.get());
   fcm_handler_->AddSharingHandler(
       chrome_browser_sharing::SharingMessage::kAckMessage,
       ack_message_handler_.get());
@@ -239,61 +226,9 @@ void SharingService::SendMessageToDevice(
     const std::string& device_guid,
     base::TimeDelta response_timeout,
     chrome_browser_sharing::SharingMessage message,
-    ResponseCallbackHelper::ResponseCallback callback) {
-  std::string message_guid = base::GenerateGUID();
-  response_callback_helper_.RegisterCallback(message_guid, std::move(callback));
-  chrome_browser_sharing::MessageType message_type =
-      SharingPayloadCaseToMessageType(message.payload_case());
-
-  base::PostDelayedTask(
-      FROM_HERE, {base::TaskPriority::USER_VISIBLE, content::BrowserThread::UI},
-      base::BindOnce(&SharingService::InvokeSendMessageCallback,
-                     weak_ptr_factory_.GetWeakPtr(), message_guid, message_type,
-                     SharingSendMessageResult::kAckTimeout,
-                     /*response=*/nullptr),
-      response_timeout);
-
-  // TODO(crbug/1015411): Here we assume caller gets |device_guid| from
-  // GetDeviceCandidates, so both DeviceInfoTracker and LocalDeviceInfoProvider
-  // are already ready. It's better to queue up the message and wait until
-  // DeviceInfoTracker and LocalDeviceInfoProvider are ready.
-  base::Optional<syncer::DeviceInfo::SharingInfo> target_sharing_info =
-      sync_prefs_->GetSharingInfo(device_guid);
-  if (!target_sharing_info) {
-    InvokeSendMessageCallback(message_guid, message_type,
-                              SharingSendMessageResult::kDeviceNotFound,
-                              /*response=*/nullptr);
-    return;
-  }
-
-  const syncer::DeviceInfo* local_device_info =
-      local_device_info_provider_->GetLocalDeviceInfo();
-  if (!local_device_info) {
-    InvokeSendMessageCallback(message_guid, message_type,
-                              SharingSendMessageResult::kInternalError,
-                              /*response=*/nullptr);
-    return;
-  }
-
-  std::unique_ptr<syncer::DeviceInfo> sender_device_info = CloneDevice(
-      local_device_info, GetSharingDeviceNames(local_device_info).full_name);
-  sender_device_info->set_sharing_info(
-      sync_prefs_->GetLocalSharingInfo(local_device_info));
-
-  if (!sender_device_info->sharing_info()) {
-    InvokeSendMessageCallback(message_guid, message_type,
-                              SharingSendMessageResult::kInternalError,
-                              /*response=*/nullptr);
-    return;
-  }
-
-  DCHECK_GE(response_timeout, kAckTimeToLive);
-  fcm_sender_->SendMessageToDevice(
-      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));
+    SharingMessageSender::ResponseCallback callback) {
+  message_sender_->SendMessageToDevice(device_guid, response_timeout,
+                                       std::move(message), std::move(callback));
 }
 
 void SharingService::SetDeviceInfoTrackerForTesting(
@@ -309,25 +244,6 @@ SharingSyncPreference* SharingService::GetSyncPreferencesForTesting() const {
   return sync_prefs_.get();
 }
 
-void SharingService::OnMessageSent(
-    base::TimeTicks start_time,
-    const std::string& message_guid,
-    chrome_browser_sharing::MessageType message_type,
-    SharingSendMessageResult result,
-    base::Optional<std::string> message_id) {
-  response_callback_helper_.OnFCMMessageSent(
-      start_time, message_guid, message_type, result, std::move(message_id));
-}
-
-void SharingService::InvokeSendMessageCallback(
-    const std::string& message_guid,
-    chrome_browser_sharing::MessageType message_type,
-    SharingSendMessageResult result,
-    std::unique_ptr<chrome_browser_sharing::ResponseMessage> response) {
-  response_callback_helper_.RunCallback(message_guid, message_type, result,
-                                        std::move(response));
-}
-
 void SharingService::OnDeviceInfoChange() {
   if (!device_info_tracker_->IsSyncing() ||
       !local_device_info_provider_->GetLocalDeviceInfo()) {

chrome/browser/sharing/sharing_service.h

@@ -16,8 +16,8 @@
 #include "build/build_config.h"
 #include "chrome/browser/sharing/ack_message_handler.h"
 #include "chrome/browser/sharing/ping_message_handler.h"
-#include "chrome/browser/sharing/response_callback_helper.h"
 #include "chrome/browser/sharing/sharing_device_registration.h"
+#include "chrome/browser/sharing/sharing_message_sender.h"
 #include "chrome/browser/sharing/sharing_send_message_result.h"
 #include "components/gcm_driver/web_push_common.h"
 #include "components/keyed_service/core/keyed_service.h"
@@ -78,6 +78,7 @@ class SharingService : public KeyedService,
       std::unique_ptr<SharingDeviceRegistration> sharing_device_registration,
       std::unique_ptr<SharingFCMSender> fcm_sender,
       std::unique_ptr<SharingFCMHandler> fcm_handler,
+      std::unique_ptr<SharingMessageSender> message_sender_,
       gcm::GCMDriver* gcm_driver,
       syncer::DeviceInfoTracker* device_info_tracker,
       syncer::LocalDeviceInfoProvider* local_device_info_provider,
@@ -109,7 +110,7 @@ class SharingService : public KeyedService,
       const std::string& device_guid,
       base::TimeDelta response_timeout,
       chrome_browser_sharing::SharingMessage message,
-      ResponseCallbackHelper::ResponseCallback callback);
+      SharingMessageSender::ResponseCallback callback);
 
   // Used to register devices with required capabilities in tests.
   void RegisterDeviceInTesting(
@@ -141,17 +142,6 @@ class SharingService : public KeyedService,
   void OnDeviceRegistered(SharingDeviceRegistrationResult result);
   void OnDeviceUnregistered(SharingDeviceRegistrationResult result);
 
-  void OnMessageSent(base::TimeTicks start_time,
-                     const std::string& message_guid,
-                     chrome_browser_sharing::MessageType message_type,
-                     SharingSendMessageResult result,
-                     base::Optional<std::string> message_id);
-  void InvokeSendMessageCallback(
-      const std::string& message_guid,
-      chrome_browser_sharing::MessageType message_type,
-      SharingSendMessageResult result,
-      std::unique_ptr<chrome_browser_sharing::ResponseMessage> response);
-
   // Returns true if required sync feature is enabled.
   bool IsSyncEnabled() const;
 
@@ -180,13 +170,12 @@ class SharingService : public KeyedService,
   void InitPersonalizableLocalDeviceName(
       std::string personalizable_local_device_name);
 
-  ResponseCallbackHelper response_callback_helper_;
-
   std::unique_ptr<SharingSyncPreference> sync_prefs_;
   std::unique_ptr<VapidKeyManager> vapid_key_manager_;
   std::unique_ptr<SharingDeviceRegistration> sharing_device_registration_;
   std::unique_ptr<SharingFCMSender> fcm_sender_;
   std::unique_ptr<SharingFCMHandler> fcm_handler_;
+  std::unique_ptr<SharingMessageSender> message_sender_;
 
   syncer::DeviceInfoTracker* device_info_tracker_;
   syncer::LocalDeviceInfoProvider* local_device_info_provider_;

chrome/browser/sharing/sharing_service_factory.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/sharing/sharing_device_registration.h"
 #include "chrome/browser/sharing/sharing_fcm_handler.h"
 #include "chrome/browser/sharing/sharing_fcm_sender.h"
+#include "chrome/browser/sharing/sharing_message_sender.h"
 #include "chrome/browser/sharing/sharing_service.h"
 #include "chrome/browser/sharing/sharing_sync_preference.h"
 #include "chrome/browser/sharing/vapid_key_manager.h"
@@ -98,12 +99,16 @@ KeyedService* SharingServiceFactory::BuildServiceInstanceFor(
   std::unique_ptr<SharingFCMHandler> fcm_handler =
       std::make_unique<SharingFCMHandler>(gcm_driver, fcm_sender.get(),
                                           sync_prefs.get());
-
-  return new SharingService(
-      std::move(sync_prefs), std::move(vapid_key_manager),
-      std::move(sharing_device_registration), std::move(fcm_sender),
-      std::move(fcm_handler), gcm_driver, device_info_tracker,
-      local_device_info_provider, sync_service, notification_display_service);
+  std::unique_ptr<SharingMessageSender> sharing_message_sender =
+      std::make_unique<SharingMessageSender>(fcm_sender.get(), sync_prefs.get(),
+                                             local_device_info_provider);
+
+  return new SharingService(std::move(sync_prefs), std::move(vapid_key_manager),
+                            std::move(sharing_device_registration),
+                            std::move(fcm_sender), std::move(fcm_handler),
+                            std::move(sharing_message_sender), gcm_driver,
+                            device_info_tracker, local_device_info_provider,
+                            sync_service, notification_display_service);
 }
 
 content::BrowserContext* SharingServiceFactory::GetBrowserContextToUse(

chrome/test/BUILD.gn

@@ -3222,10 +3222,10 @@ test("unit_tests") {
     "../browser/sharing/ack_message_handler_unittest.cc",
     "../browser/sharing/mock_sharing_service.cc",
     "../browser/sharing/mock_sharing_service.h",
-    "../browser/sharing/response_callback_helper_unittest.cc",
     "../browser/sharing/sharing_device_registration_unittest.cc",
     "../browser/sharing/sharing_fcm_handler_unittest.cc",
     "../browser/sharing/sharing_fcm_sender_unittest.cc",
+    "../browser/sharing/sharing_message_sender_unittest.cc",
     "../browser/sharing/sharing_service_unittest.cc",
     "../browser/sharing/sharing_sync_preference_unittest.cc",
     "../browser/sharing/sharing_utils_unittest.cc",