Split FCM sender from SharingMessageSender
To reuse all logic inside SharingMessageSender (metrics, timeouts and
acks) this introduces a SendMessageDelegate which is implemented once
for FCM and will be implemented for WebRTC in a future CL. This allows
us to dynamically choose the send implementation in the SharingService.
Bug: 1041032
Change-Id: Ib8abc2cf0c963a511ced12b71e09acde6fc0fa75
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1995332
Commit-Queue: Richard Knoll <[email protected]>
Reviewed-by: Himanshu Jaju <[email protected]>
Cr-Commit-Position: refs/heads/master@{#731652}
diff --git a/chrome/browser/sharing/ack_message_handler_unittest.cc b/chrome/browser/sharing/ack_message_handler_unittest.cc
index eb955b41..9d937da 100644
--- a/chrome/browser/sharing/ack_message_handler_unittest.cc
+++ b/chrome/browser/sharing/ack_message_handler_unittest.cc
@@ -19,7 +19,9 @@
class MockSharingMessageSender : public SharingMessageSender {
public:
MockSharingMessageSender()
- : SharingMessageSender(nullptr, nullptr, nullptr) {}
+ : SharingMessageSender(
+ /*sync_prefs=*/nullptr,
+ /*local_device_info_provider=*/nullptr) {}
~MockSharingMessageSender() override = default;
MOCK_METHOD3(
diff --git a/chrome/browser/sharing/sharing_fcm_handler.cc b/chrome/browser/sharing/sharing_fcm_handler.cc
index 9425d4f..8ed590ed 100644
--- a/chrome/browser/sharing/sharing_fcm_handler.cc
+++ b/chrome/browser/sharing/sharing_fcm_handler.cc
@@ -174,7 +174,7 @@
if (response)
ack_message->set_allocated_response_message(response.release());
- sharing_fcm_sender_->SendMessageToDevice(
+ sharing_fcm_sender_->SendMessageToTargetInfo(
std::move(*target_info), kAckTimeToLive, std::move(sharing_message),
base::BindOnce(&SharingFCMHandler::OnAckMessageSent,
weak_ptr_factory_.GetWeakPtr(),
diff --git a/chrome/browser/sharing/sharing_fcm_handler_unittest.cc b/chrome/browser/sharing/sharing_fcm_handler_unittest.cc
index f73e3597..3f945d7 100644
--- a/chrome/browser/sharing/sharing_fcm_handler_unittest.cc
+++ b/chrome/browser/sharing/sharing_fcm_handler_unittest.cc
@@ -70,10 +70,11 @@
MockSharingFCMSender()
: SharingFCMSender(/*gcm_driver=*/nullptr,
/*sync_preference=*/nullptr,
- /*vapid_key_manager=*/nullptr) {}
+ /*vapid_key_manager=*/nullptr,
+ /*local_device_info_provider=*/nullptr) {}
~MockSharingFCMSender() override {}
- MOCK_METHOD4(SendMessageToDevice,
+ MOCK_METHOD4(SendMessageToTargetInfo,
void(syncer::DeviceInfo::SharingTargetInfo target,
base::TimeDelta time_to_live,
SharingMessage message,
@@ -155,7 +156,7 @@
EXPECT_CALL(mock_sharing_message_handler_,
OnMessage(ProtoEquals(sharing_message), _));
- EXPECT_CALL(mock_sharing_fcm_sender_, SendMessageToDevice(_, _, _, _))
+ EXPECT_CALL(mock_sharing_fcm_sender_, SendMessageToTargetInfo(_, _, _, _))
.Times(0);
handler_registry_->SetSharingHandler(SharingMessage::kAckMessage,
&mock_sharing_message_handler_);
@@ -181,7 +182,7 @@
// 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_, SendMessageToTargetInfo(_, _, _, _))
.Times(0);
sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
@@ -194,9 +195,10 @@
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), _));
+ EXPECT_CALL(
+ mock_sharing_fcm_sender_,
+ SendMessageToTargetInfo(DeviceMatcher(), testing::Eq(kAckTimeToLive),
+ ProtoEquals(sharing_ack_message), _));
handler_registry_->SetSharingHandler(SharingMessage::kPingMessage,
&mock_sharing_message_handler_);
sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
@@ -204,7 +206,7 @@
// 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_, SendMessageToTargetInfo(_, _, _, _))
.Times(0);
handler_registry_->SetSharingHandler(SharingMessage::kPingMessage, nullptr);
sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
@@ -237,9 +239,10 @@
.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), _));
+ EXPECT_CALL(
+ mock_sharing_fcm_sender_,
+ SendMessageToTargetInfo(DeviceMatcher(), testing::Eq(kAckTimeToLive),
+ ProtoEquals(sharing_ack_message), _));
handler_registry_->SetSharingHandler(SharingMessage::kPingMessage,
&mock_sharing_message_handler_);
sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
@@ -271,9 +274,10 @@
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), _));
+ EXPECT_CALL(
+ mock_sharing_fcm_sender_,
+ SendMessageToTargetInfo(DeviceMatcher(), testing::Eq(kAckTimeToLive),
+ ProtoEquals(sharing_ack_message), _));
handler_registry_->SetSharingHandler(SharingMessage::kPingMessage,
&mock_sharing_message_handler_);
sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
@@ -306,9 +310,10 @@
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), _));
+ EXPECT_CALL(
+ mock_sharing_fcm_sender_,
+ SendMessageToTargetInfo(DeviceMatcher(), testing::Eq(kAckTimeToLive),
+ ProtoEquals(sharing_ack_message), _));
handler_registry_->SetSharingHandler(SharingMessage::kPingMessage,
&mock_sharing_message_handler_);
sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
diff --git a/chrome/browser/sharing/sharing_fcm_sender.cc b/chrome/browser/sharing/sharing_fcm_sender.cc
index cbf1369b..90a33de 100644
--- a/chrome/browser/sharing/sharing_fcm_sender.cc
+++ b/chrome/browser/sharing/sharing_fcm_sender.cc
@@ -11,17 +11,21 @@
#include "chrome/browser/sharing/vapid_key_manager.h"
#include "components/gcm_driver/common/gcm_message.h"
#include "components/gcm_driver/gcm_driver.h"
+#include "components/sync_device_info/local_device_info_provider.h"
-SharingFCMSender::SharingFCMSender(gcm::GCMDriver* gcm_driver,
- SharingSyncPreference* sync_preference,
- VapidKeyManager* vapid_key_manager)
+SharingFCMSender::SharingFCMSender(
+ gcm::GCMDriver* gcm_driver,
+ SharingSyncPreference* sync_preference,
+ VapidKeyManager* vapid_key_manager,
+ syncer::LocalDeviceInfoProvider* local_device_info_provider)
: gcm_driver_(gcm_driver),
sync_preference_(sync_preference),
- vapid_key_manager_(vapid_key_manager) {}
+ vapid_key_manager_(vapid_key_manager),
+ local_device_info_provider_(local_device_info_provider) {}
SharingFCMSender::~SharingFCMSender() = default;
-void SharingFCMSender::SendMessageToDevice(
+void SharingFCMSender::SendMessageToTargetInfo(
syncer::DeviceInfo::SharingTargetInfo target,
base::TimeDelta time_to_live,
SharingMessage message,
@@ -56,6 +60,53 @@
weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
}
+void SharingFCMSender::DoSendMessageToDevice(const syncer::DeviceInfo& device,
+ base::TimeDelta time_to_live,
+ SharingMessage message,
+ SendMessageCallback callback) {
+ base::Optional<syncer::DeviceInfo::SharingTargetInfo> target_info =
+ GetTargetInfo(device);
+ if (!target_info) {
+ std::move(callback).Run(SharingSendMessageResult::kDeviceNotFound,
+ /*response=*/nullptr);
+ return;
+ }
+
+ if (!SetMessageSenderInfo(&message)) {
+ std::move(callback).Run(SharingSendMessageResult::kInternalError,
+ /*response=*/nullptr);
+ return;
+ }
+
+ SendMessageToTargetInfo(std::move(*target_info), time_to_live,
+ std::move(message), std::move(callback));
+}
+
+base::Optional<syncer::DeviceInfo::SharingTargetInfo>
+SharingFCMSender::GetTargetInfo(const syncer::DeviceInfo& device) {
+ if (device.sharing_info())
+ return device.sharing_info()->vapid_target_info;
+
+ // TODO(crbug/1015411): Here we assume caller gets the |device| from
+ // GetDeviceCandidates, so DeviceInfoTracker is ready. It's better to queue up
+ // the message and wait until DeviceInfoTracker is ready.
+ return sync_preference_->GetTargetInfo(device.guid());
+}
+
+bool SharingFCMSender::SetMessageSenderInfo(SharingMessage* message) {
+ base::Optional<syncer::DeviceInfo::SharingInfo> sharing_info =
+ sync_preference_->GetLocalSharingInfo(
+ local_device_info_provider_->GetLocalDeviceInfo());
+ if (!sharing_info)
+ return false;
+
+ auto* sender_info = message->mutable_sender_info();
+ sender_info->set_fcm_token(sharing_info->vapid_target_info.fcm_token);
+ sender_info->set_p256dh(sharing_info->vapid_target_info.p256dh);
+ sender_info->set_auth_secret(sharing_info->vapid_target_info.auth_secret);
+ return true;
+}
+
void SharingFCMSender::OnMessageSent(SendMessageCallback callback,
gcm::SendWebPushMessageResult result,
base::Optional<std::string> message_id) {
diff --git a/chrome/browser/sharing/sharing_fcm_sender.h b/chrome/browser/sharing/sharing_fcm_sender.h
index b9729d0b..ba617a0a 100644
--- a/chrome/browser/sharing/sharing_fcm_sender.h
+++ b/chrome/browser/sharing/sharing_fcm_sender.h
@@ -10,11 +10,11 @@
#include <vector>
#include "base/callback_forward.h"
-#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "base/time/time.h"
#include "chrome/browser/sharing/proto/sharing_message.pb.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/sync_device_info/device_info.h"
@@ -24,11 +24,15 @@
enum class SendWebPushMessageResult;
} // namespace gcm
+namespace syncer {
+class LocalDeviceInfoProvider;
+} // namespace syncer
+
class SharingSyncPreference;
class VapidKeyManager;
// Responsible for sending FCM messages within Sharing infrastructure.
-class SharingFCMSender {
+class SharingFCMSender : public SharingMessageSender::SendMessageDelegate {
public:
using SharingMessage = chrome_browser_sharing::SharingMessage;
using SendMessageCallback =
@@ -37,18 +41,34 @@
SharingFCMSender(gcm::GCMDriver* gcm_driver,
SharingSyncPreference* sync_preference,
- VapidKeyManager* vapid_key_manager);
- virtual ~SharingFCMSender();
+ VapidKeyManager* vapid_key_manager,
+ syncer::LocalDeviceInfoProvider* local_device_info_provider);
+ SharingFCMSender(const SharingFCMSender&) = delete;
+ SharingFCMSender& operator=(const SharingFCMSender&) = delete;
+ ~SharingFCMSender() override;
// 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.
- virtual void SendMessageToDevice(syncer::DeviceInfo::SharingTargetInfo target,
- base::TimeDelta time_to_live,
- SharingMessage message,
- SendMessageCallback callback);
+ virtual void SendMessageToTargetInfo(
+ syncer::DeviceInfo::SharingTargetInfo target,
+ base::TimeDelta time_to_live,
+ SharingMessage message,
+ SendMessageCallback callback);
+
+ protected:
+ // SharingMessageSender::SendMessageDelegate:
+ void DoSendMessageToDevice(const syncer::DeviceInfo& device,
+ base::TimeDelta time_to_live,
+ SharingMessage message,
+ SendMessageCallback callback) override;
private:
+ base::Optional<syncer::DeviceInfo::SharingTargetInfo> GetTargetInfo(
+ const syncer::DeviceInfo& device);
+
+ bool SetMessageSenderInfo(SharingMessage* message);
+
void OnMessageSent(SendMessageCallback callback,
gcm::SendWebPushMessageResult result,
base::Optional<std::string> message_id);
@@ -56,10 +76,9 @@
gcm::GCMDriver* gcm_driver_;
SharingSyncPreference* sync_preference_;
VapidKeyManager* vapid_key_manager_;
+ syncer::LocalDeviceInfoProvider* local_device_info_provider_;
base::WeakPtrFactory<SharingFCMSender> weak_ptr_factory_{this};
-
- DISALLOW_COPY_AND_ASSIGN(SharingFCMSender);
};
#endif // CHROME_BROWSER_SHARING_SHARING_FCM_SENDER_H_
diff --git a/chrome/browser/sharing/sharing_fcm_sender_unittest.cc b/chrome/browser/sharing/sharing_fcm_sender_unittest.cc
index 5853ac5..fb58931c 100644
--- a/chrome/browser/sharing/sharing_fcm_sender_unittest.cc
+++ b/chrome/browser/sharing/sharing_fcm_sender_unittest.cc
@@ -15,6 +15,7 @@
#include "components/gcm_driver/fake_gcm_driver.h"
#include "components/sync_device_info/device_info.h"
#include "components/sync_device_info/fake_device_info_sync_service.h"
+#include "components/sync_device_info/fake_local_device_info_provider.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
#include "crypto/ec_private_key.h"
#include "testing/gmock/include/gmock/gmock.h"
@@ -94,14 +95,16 @@
protected:
SharingFCMSenderTest()
- : sync_prefs_(&prefs_, &fake_device_info_sync_service),
+ : sync_prefs_(&prefs_, &fake_device_info_sync_service_),
sharing_fcm_sender_(&fake_gcm_driver_,
&sync_prefs_,
- &vapid_key_manager_) {
+ &vapid_key_manager_,
+ &fake_local_device_info_provider_) {
SharingSyncPreference::RegisterProfilePrefs(prefs_.registry());
}
- syncer::FakeDeviceInfoSyncService fake_device_info_sync_service;
+ syncer::FakeDeviceInfoSyncService fake_device_info_sync_service_;
+ syncer::FakeLocalDeviceInfoProvider fake_local_device_info_provider_;
FakeGCMDriver fake_gcm_driver_;
SharingSyncPreference sync_prefs_;
@@ -128,7 +131,7 @@
base::Optional<std::string> message_id;
chrome_browser_sharing::SharingMessage sharing_message;
sharing_message.mutable_ack_message();
- sharing_fcm_sender_.SendMessageToDevice(
+ sharing_fcm_sender_.SendMessageToTargetInfo(
std::move(target), base::TimeDelta::FromSeconds(kTtlSeconds),
std::move(sharing_message),
base::BindOnce(&SharingFCMSenderTest::OnMessageSent,
@@ -150,7 +153,7 @@
base::Optional<std::string> message_id;
chrome_browser_sharing::SharingMessage sharing_message;
sharing_message.mutable_ack_message();
- sharing_fcm_sender_.SendMessageToDevice(
+ sharing_fcm_sender_.SendMessageToTargetInfo(
std::move(target), base::TimeDelta::FromSeconds(kTtlSeconds),
std::move(sharing_message),
base::BindOnce(&SharingFCMSenderTest::OnMessageSent,
@@ -204,7 +207,7 @@
base::Optional<std::string> message_id;
chrome_browser_sharing::SharingMessage sharing_message;
sharing_message.mutable_ping_message();
- sharing_fcm_sender_.SendMessageToDevice(
+ sharing_fcm_sender_.SendMessageToTargetInfo(
std::move(target), base::TimeDelta::FromSeconds(kTtlSeconds),
std::move(sharing_message),
base::BindOnce(&SharingFCMSenderTest::OnMessageSent,
diff --git a/chrome/browser/sharing/sharing_message_sender.cc b/chrome/browser/sharing/sharing_message_sender.cc
index cd7441b..6d4d27b 100644
--- a/chrome/browser/sharing/sharing_message_sender.cc
+++ b/chrome/browser/sharing/sharing_message_sender.cc
@@ -3,10 +3,10 @@
// 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_constants.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"
@@ -15,11 +15,9 @@
#include "content/public/browser/browser_task_traits.h"
SharingMessageSender::SharingMessageSender(
- std::unique_ptr<SharingFCMSender> sharing_fcm_sender,
SharingSyncPreference* sync_prefs,
syncer::LocalDeviceInfoProvider* local_device_info_provider)
- : fcm_sender_(std::move(sharing_fcm_sender)),
- sync_prefs_(sync_prefs),
+ : sync_prefs_(sync_prefs),
local_device_info_provider_(local_device_info_provider) {}
SharingMessageSender::~SharingMessageSender() = default;
@@ -28,7 +26,9 @@
const syncer::DeviceInfo& device,
base::TimeDelta response_timeout,
chrome_browser_sharing::SharingMessage message,
+ DelegateType delegate_type,
ResponseCallback callback) {
+ DCHECK_GE(response_timeout, kAckTimeToLive);
DCHECK(message.payload_case() !=
chrome_browser_sharing::SharingMessage::kAckMessage);
@@ -39,28 +39,20 @@
SharingDevicePlatform receiver_device_platform =
sync_prefs_->GetDevicePlatform(device.guid());
- base::PostDelayedTask(
- FROM_HERE, {base::TaskPriority::USER_VISIBLE, content::BrowserThread::UI},
- base::BindOnce(&SharingMessageSender::InvokeSendMessageCallback,
- weak_ptr_factory_.GetWeakPtr(), message_guid, message_type,
- receiver_device_platform,
- SharingSendMessageResult::kAckTimeout,
- /*response=*/nullptr),
- response_timeout);
-
- // TODO(crbug/1015411): Here we assume the caller gets the 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.
- auto target_info = sync_prefs_->GetTargetInfo(device.guid());
- if (!target_info) {
+ auto delegate_iter = send_delegates_.find(delegate_type);
+ if (delegate_iter == send_delegates_.end()) {
InvokeSendMessageCallback(message_guid, message_type,
receiver_device_platform,
- SharingSendMessageResult::kDeviceNotFound,
+ SharingSendMessageResult::kInternalError,
/*response=*/nullptr);
return;
}
+ SendMessageDelegate* delegate = delegate_iter->second.get();
+ DCHECK(delegate);
+ // TODO(crbug/1015411): Here we assume the caller gets the |device| from
+ // GetDeviceCandidates, so LocalDeviceInfoProvider is ready. It's better to
+ // queue up the message and wait until LocalDeviceInfoProvider is ready.
const syncer::DeviceInfo* local_device_info =
local_device_info_provider_->GetLocalDeviceInfo();
if (!local_device_info) {
@@ -71,15 +63,14 @@
return;
}
- base::Optional<syncer::DeviceInfo::SharingInfo> sharing_info =
- sync_prefs_->GetLocalSharingInfo(local_device_info);
- if (!sharing_info) {
- InvokeSendMessageCallback(message_guid, message_type,
- receiver_device_platform,
- SharingSendMessageResult::kInternalError,
- /*response=*/nullptr);
- return;
- }
+ base::PostDelayedTask(
+ FROM_HERE, {base::TaskPriority::USER_VISIBLE, content::BrowserThread::UI},
+ base::BindOnce(&SharingMessageSender::InvokeSendMessageCallback,
+ weak_ptr_factory_.GetWeakPtr(), message_guid, message_type,
+ receiver_device_platform,
+ SharingSendMessageResult::kAckTimeout,
+ /*response=*/nullptr),
+ response_timeout);
LogSharingDeviceLastUpdatedAge(
message_type, base::Time::Now() - device.last_updated_timestamp());
@@ -89,15 +80,8 @@
message.set_sender_device_name(
send_tab_to_self::GetSharingDeviceNames(local_device_info).full_name);
- auto* sender_info = message.mutable_sender_info();
- sender_info->set_fcm_token(sharing_info->vapid_target_info.fcm_token);
- sender_info->set_p256dh(sharing_info->vapid_target_info.p256dh);
- sender_info->set_auth_secret(sharing_info->vapid_target_info.auth_secret);
-
- DCHECK_GE(response_timeout, kAckTimeToLive);
- fcm_sender_->SendMessageToDevice(
- std::move(*target_info), response_timeout - kAckTimeToLive,
- std::move(message),
+ delegate->DoSendMessageToDevice(
+ device, response_timeout - kAckTimeToLive, std::move(message),
base::BindOnce(&SharingMessageSender::OnMessageSent,
weak_ptr_factory_.GetWeakPtr(), base::TimeTicks::Now(),
message_guid, message_type, receiver_device_platform));
@@ -151,6 +135,13 @@
SharingSendMessageResult::kSuccessful, std::move(response));
}
+void SharingMessageSender::RegisterSendDelegate(
+ DelegateType type,
+ std::unique_ptr<SendMessageDelegate> delegate) {
+ auto result = send_delegates_.emplace(type, std::move(delegate));
+ DCHECK(result.second) << "Delegate type already registered";
+}
+
void SharingMessageSender::InvokeSendMessageCallback(
const std::string& message_guid,
chrome_browser_sharing::MessageType message_type,
diff --git a/chrome/browser/sharing/sharing_message_sender.h b/chrome/browser/sharing/sharing_message_sender.h
index 2886a4d..cd835b3 100644
--- a/chrome/browser/sharing/sharing_message_sender.h
+++ b/chrome/browser/sharing/sharing_message_sender.h
@@ -10,7 +10,6 @@
#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"
@@ -26,7 +25,6 @@
class LocalDeviceInfoProvider;
} // namespace syncer
-class SharingFCMSender;
class SharingSyncPreference;
enum class SharingDevicePlatform;
enum class SharingSendMessageResult;
@@ -37,16 +35,38 @@
SharingSendMessageResult,
std::unique_ptr<chrome_browser_sharing::ResponseMessage>)>;
+ // Delegate class used to swap the actual message sending implementation.
+ class SendMessageDelegate {
+ public:
+ using SendMessageCallback =
+ base::OnceCallback<void(SharingSendMessageResult,
+ base::Optional<std::string>)>;
+ virtual ~SendMessageDelegate() = default;
+
+ virtual void DoSendMessageToDevice(
+ const syncer::DeviceInfo& device,
+ base::TimeDelta time_to_live,
+ chrome_browser_sharing::SharingMessage message,
+ SendMessageCallback callback) = 0;
+ };
+
+ // Delegate type used to send a message.
+ enum class DelegateType {
+ kFCM,
+ };
+
SharingMessageSender(
- std::unique_ptr<SharingFCMSender> sharing_fcm_sender,
SharingSyncPreference* sync_prefs,
syncer::LocalDeviceInfoProvider* local_device_info_provider);
+ SharingMessageSender(const SharingMessageSender&) = delete;
+ SharingMessageSender& operator=(const SharingMessageSender&) = delete;
virtual ~SharingMessageSender();
virtual void SendMessageToDevice(
const syncer::DeviceInfo& device,
base::TimeDelta response_timeout,
chrome_browser_sharing::SharingMessage message,
+ DelegateType delegate_type,
ResponseCallback callback);
virtual void OnAckReceived(
@@ -54,6 +74,11 @@
const std::string& message_id,
std::unique_ptr<chrome_browser_sharing::ResponseMessage> response);
+ // Registers the given |delegate| to send messages when SendMessageToDevice is
+ // called with |type|.
+ void RegisterSendDelegate(DelegateType type,
+ std::unique_ptr<SendMessageDelegate> delegate);
+
private:
void OnMessageSent(base::TimeTicks start_time,
const std::string& message_guid,
@@ -69,7 +94,6 @@
SharingSendMessageResult result,
std::unique_ptr<chrome_browser_sharing::ResponseMessage> response);
- std::unique_ptr<SharingFCMSender> fcm_sender_;
SharingSyncPreference* sync_prefs_;
syncer::LocalDeviceInfoProvider* local_device_info_provider_;
@@ -82,9 +106,10 @@
// Map of FCM message_id to platform of receiver device for metrics.
std::map<std::string, SharingDevicePlatform> receiver_device_platform_;
- base::WeakPtrFactory<SharingMessageSender> weak_ptr_factory_{this};
+ // Registered delegates to send messages.
+ std::map<DelegateType, std::unique_ptr<SendMessageDelegate>> send_delegates_;
- DISALLOW_COPY_AND_ASSIGN(SharingMessageSender);
+ base::WeakPtrFactory<SharingMessageSender> weak_ptr_factory_{this};
};
#endif // CHROME_BROWSER_SHARING_SHARING_MESSAGE_SENDER_H_
diff --git a/chrome/browser/sharing/sharing_message_sender_unittest.cc b/chrome/browser/sharing/sharing_message_sender_unittest.cc
index 57d894d..6114b9d 100644
--- a/chrome/browser/sharing/sharing_message_sender_unittest.cc
+++ b/chrome/browser/sharing/sharing_message_sender_unittest.cc
@@ -34,25 +34,57 @@
const char kSenderMessageID[] = "sender_message_id";
constexpr base::TimeDelta kTimeToLive = base::TimeDelta::FromSeconds(10);
+namespace {
+
class MockSharingFCMSender : public SharingFCMSender {
public:
- MockSharingFCMSender() : SharingFCMSender(nullptr, nullptr, nullptr) {}
+ MockSharingFCMSender(
+ SharingSyncPreference* sync_preference,
+ syncer::LocalDeviceInfoProvider* local_device_info_provider)
+ : SharingFCMSender(
+ /*gcm_driver=*/nullptr,
+ sync_preference,
+ /*vapid_key_manager=*/nullptr,
+ local_device_info_provider) {}
+ MockSharingFCMSender(const MockSharingFCMSender&) = delete;
+ MockSharingFCMSender& operator=(const MockSharingFCMSender&) = delete;
~MockSharingFCMSender() override = default;
- MOCK_METHOD4(SendMessageToDevice,
+ MOCK_METHOD4(SendMessageToTargetInfo,
void(syncer::DeviceInfo::SharingTargetInfo target,
base::TimeDelta time_to_live,
SharingMessage message,
SendMessageCallback callback));
-
- private:
- DISALLOW_COPY_AND_ASSIGN(MockSharingFCMSender);
};
+class MockSendMessageDelegate
+ : public SharingMessageSender::SendMessageDelegate {
+ public:
+ MockSendMessageDelegate() = default;
+ MockSendMessageDelegate(const MockSendMessageDelegate&) = delete;
+ MockSendMessageDelegate& operator=(const MockSendMessageDelegate&) = delete;
+ ~MockSendMessageDelegate() override = default;
+
+ MOCK_METHOD4(DoSendMessageToDevice,
+ void(const syncer::DeviceInfo& device,
+ base::TimeDelta time_to_live,
+ chrome_browser_sharing::SharingMessage message,
+ SendMessageCallback callback));
+};
+
+} // namespace
+
class SharingMessageSenderTest : public testing::Test {
public:
SharingMessageSenderTest() {
SharingSyncPreference::RegisterProfilePrefs(prefs_.registry());
+ auto mock_sharing_fcm_sender = std::make_unique<MockSharingFCMSender>(
+ &sharing_sync_preference_,
+ fake_device_info_sync_service_.GetLocalDeviceInfoProvider());
+ mock_sharing_fcm_sender_ = mock_sharing_fcm_sender.get();
+ sharing_message_sender_.RegisterSendDelegate(
+ SharingMessageSender::DelegateType::kFCM,
+ std::move(mock_sharing_fcm_sender));
}
~SharingMessageSenderTest() override = default;
@@ -61,15 +93,13 @@
base::test::TaskEnvironment::TimeSource::MOCK_TIME};
sync_preferences::TestingPrefServiceSyncable prefs_;
syncer::FakeDeviceInfoSyncService fake_device_info_sync_service_;
-
- testing::NiceMock<MockSharingFCMSender>* mock_sharing_fcm_sender_ =
- new testing::NiceMock<MockSharingFCMSender>();
SharingSyncPreference sharing_sync_preference_{
&prefs_, &fake_device_info_sync_service_};
SharingMessageSender sharing_message_sender_{
- base::WrapUnique(mock_sharing_fcm_sender_), &sharing_sync_preference_,
+ &sharing_sync_preference_,
fake_device_info_sync_service_.GetLocalDeviceInfoProvider()};
+ MockSharingFCMSender* mock_sharing_fcm_sender_;
DISALLOW_COPY_AND_ASSIGN(SharingMessageSenderTest);
};
@@ -133,13 +163,14 @@
kSenderMessageID, /*response=*/nullptr);
};
- ON_CALL(*mock_sharing_fcm_sender_,
- SendMessageToDevice(testing::_, testing::_, testing::_, testing::_))
- .WillByDefault(testing::Invoke(simulate_timeout));
+ EXPECT_CALL(
+ *mock_sharing_fcm_sender_,
+ SendMessageToTargetInfo(testing::_, testing::_, testing::_, testing::_))
+ .WillOnce(testing::Invoke(simulate_timeout));
sharing_message_sender_.SendMessageToDevice(
*device_info.get(), kTimeToLive, chrome_browser_sharing::SharingMessage(),
- mock_callback.Get());
+ SharingMessageSender::DelegateType::kFCM, mock_callback.Get());
}
TEST_F(SharingMessageSenderTest, SendMessageToDevice_InternalError) {
@@ -174,13 +205,14 @@
kSenderMessageID, /*response=*/nullptr);
};
- ON_CALL(*mock_sharing_fcm_sender_,
- SendMessageToDevice(testing::_, testing::_, testing::_, testing::_))
- .WillByDefault(testing::Invoke(simulate_internal_error));
+ EXPECT_CALL(
+ *mock_sharing_fcm_sender_,
+ SendMessageToTargetInfo(testing::_, testing::_, testing::_, testing::_))
+ .WillOnce(testing::Invoke(simulate_internal_error));
sharing_message_sender_.SendMessageToDevice(
*device_info.get(), kTimeToLive, chrome_browser_sharing::SharingMessage(),
- mock_callback.Get());
+ SharingMessageSender::DelegateType::kFCM, mock_callback.Get());
}
TEST_F(SharingMessageSenderTest, MessageSent_AckReceived) {
@@ -236,11 +268,30 @@
kSenderMessageID, std::move(response_message));
};
- ON_CALL(*mock_sharing_fcm_sender_,
- SendMessageToDevice(testing::_, testing::_, testing::_, testing::_))
- .WillByDefault(testing::Invoke(simulate_expected_ack_message_received));
+ EXPECT_CALL(
+ *mock_sharing_fcm_sender_,
+ SendMessageToTargetInfo(testing::_, testing::_, testing::_, testing::_))
+ .WillOnce(testing::Invoke(simulate_expected_ack_message_received));
- sharing_message_sender_.SendMessageToDevice(*device_info.get(), kTimeToLive,
- std::move(sent_message),
- mock_callback.Get());
+ sharing_message_sender_.SendMessageToDevice(
+ *device_info.get(), kTimeToLive, std::move(sent_message),
+ SharingMessageSender::DelegateType::kFCM, mock_callback.Get());
+}
+
+TEST_F(SharingMessageSenderTest, NonExistingDelegate) {
+ SharingMessageSender sharing_message_sender{
+ &sharing_sync_preference_,
+ fake_device_info_sync_service_.GetLocalDeviceInfoProvider()};
+
+ std::unique_ptr<syncer::DeviceInfo> device_info = CreateFakeDeviceInfo(
+ kReceiverGUID, kReceiverDeviceName, CreateSharingInfo());
+
+ base::MockCallback<SharingMessageSender::ResponseCallback> mock_callback;
+ EXPECT_CALL(mock_callback,
+ Run(testing::Eq(SharingSendMessageResult::kInternalError),
+ testing::Eq(nullptr)));
+
+ sharing_message_sender.SendMessageToDevice(
+ *device_info.get(), kTimeToLive, chrome_browser_sharing::SharingMessage(),
+ SharingMessageSender::DelegateType::kFCM, mock_callback.Get());
}
diff --git a/chrome/browser/sharing/sharing_service.cc b/chrome/browser/sharing/sharing_service.cc
index e63067a..2091a6d 100644
--- a/chrome/browser/sharing/sharing_service.cc
+++ b/chrome/browser/sharing/sharing_service.cc
@@ -76,8 +76,9 @@
base::TimeDelta response_timeout,
chrome_browser_sharing::SharingMessage message,
SharingMessageSender::ResponseCallback callback) {
- message_sender_->SendMessageToDevice(device, response_timeout,
- std::move(message), std::move(callback));
+ message_sender_->SendMessageToDevice(
+ device, response_timeout, std::move(message),
+ SharingMessageSender::DelegateType::kFCM, std::move(callback));
}
SharingDeviceSource* SharingService::GetDeviceSource() const {
diff --git a/chrome/browser/sharing/sharing_service.h b/chrome/browser/sharing/sharing_service.h
index 7c5faf5..12215bed 100644
--- a/chrome/browser/sharing/sharing_service.h
+++ b/chrome/browser/sharing/sharing_service.h
@@ -40,7 +40,7 @@
// Class to manage lifecycle of sharing feature, and provide APIs to send
// sharing messages to other devices.
-class SharingService : public KeyedService, syncer::SyncServiceObserver {
+class SharingService : public KeyedService, public syncer::SyncServiceObserver {
public:
using SharingDeviceList = std::vector<std::unique_ptr<syncer::DeviceInfo>>;
@@ -63,6 +63,8 @@
std::unique_ptr<SharingDeviceSource> device_source,
std::unique_ptr<SharingFCMHandler> fcm_handler,
syncer::SyncService* sync_service);
+ SharingService(const SharingService&) = delete;
+ SharingService& operator=(const SharingService&) = delete;
~SharingService() override;
// Returns the device matching |guid|, or nullptr if no match was found.
@@ -139,8 +141,6 @@
#endif // defined(OS_ANDROID)
base::WeakPtrFactory<SharingService> weak_ptr_factory_{this};
-
- DISALLOW_COPY_AND_ASSIGN(SharingService);
};
#endif // CHROME_BROWSER_SHARING_SHARING_SERVICE_H_
diff --git a/chrome/browser/sharing/sharing_service_factory.cc b/chrome/browser/sharing/sharing_service_factory.cc
index 4015e470..5453780 100644
--- a/chrome/browser/sharing/sharing_service_factory.cc
+++ b/chrome/browser/sharing/sharing_service_factory.cc
@@ -117,10 +117,14 @@
vapid_key_manager.get());
auto fcm_sender = std::make_unique<SharingFCMSender>(
- gcm_driver, sync_prefs.get(), vapid_key_manager.get());
+ gcm_driver, sync_prefs.get(), vapid_key_manager.get(),
+ local_device_info_provider);
SharingFCMSender* fcm_sender_ptr = fcm_sender.get();
auto sharing_message_sender = std::make_unique<SharingMessageSender>(
- std::move(fcm_sender), sync_prefs.get(), local_device_info_provider);
+ sync_prefs.get(), local_device_info_provider);
+
+ sharing_message_sender->RegisterSendDelegate(
+ SharingMessageSender::DelegateType::kFCM, std::move(fcm_sender));
auto device_source = std::make_unique<SharingDeviceSourceSync>(
sync_service, local_device_info_provider, device_info_tracker);
diff --git a/chrome/browser/sharing/sharing_service_unittest.cc b/chrome/browser/sharing/sharing_service_unittest.cc
index 8895750..63bb92b 100644
--- a/chrome/browser/sharing/sharing_service_unittest.cc
+++ b/chrome/browser/sharing/sharing_service_unittest.cc
@@ -61,7 +61,10 @@
public:
MockSharingFCMHandler()
- : SharingFCMHandler(nullptr, nullptr, nullptr, nullptr) {}
+ : SharingFCMHandler(/*gcm_driver=*/nullptr,
+ /*sharing_fcm_sender=*/nullptr,
+ /*sync_preference=*/nullptr,
+ /*handler_registry=*/nullptr) {}
~MockSharingFCMHandler() = default;
MOCK_METHOD0(StartListening, void());
@@ -71,13 +74,16 @@
class MockSharingMessageSender : public SharingMessageSender {
public:
MockSharingMessageSender()
- : SharingMessageSender(nullptr, nullptr, nullptr) {}
+ : SharingMessageSender(
+ /*sync_prefs=*/nullptr,
+ /*local_device_info_provider=*/nullptr) {}
~MockSharingMessageSender() override = default;
- MOCK_METHOD4(SendMessageToDevice,
+ MOCK_METHOD5(SendMessageToDevice,
void(const syncer::DeviceInfo&,
base::TimeDelta,
chrome_browser_sharing::SharingMessage,
+ DelegateType,
ResponseCallback));
private:
@@ -314,6 +320,7 @@
auto run_callback = [&](const syncer::DeviceInfo& device_info,
base::TimeDelta response_timeout,
chrome_browser_sharing::SharingMessage message,
+ SharingMessageSender::DelegateType delegate_type,
SharingMessageSender::ResponseCallback callback) {
std::unique_ptr<chrome_browser_sharing::ResponseMessage> response_message =
std::make_unique<chrome_browser_sharing::ResponseMessage>();
@@ -323,7 +330,8 @@
};
ON_CALL(*sharing_message_sender_,
- SendMessageToDevice(testing::_, testing::_, testing::_, testing::_))
+ SendMessageToDevice(testing::_, testing::_, testing::_, testing::_,
+ testing::_))
.WillByDefault(testing::Invoke(run_callback));
GetSharingService()->SendMessageToDevice(
