Commit 49e25a72 authored by Alex Chau's avatar Alex Chau Committed by Commit Bot

Fix sender names in SharedClipboard

- SharedClipboard sender names are embedded in SharingMessage or
  determined in SharedClipboardMessageHandler
- SharingService::GetDeviceByGuid now changes client_name returned
- sender_guid is no longer embeded in AckMessage as it's only used for
  sending acks
- Removed dependency of LocalDeviceInfoProvider in SharingFCMSender,
  instead sender_device_info is passed in and constructed by
  SharingService with updated client_name

Bug: 1013565
Change-Id: If901fe96ba140b804a87fcd8d2565512d76fa167
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1865318Reviewed-by: default avatarRichard Knoll <knollr@chromium.org>
Reviewed-by: default avatarMichael van Ouwerkerk <mvanouwerkerk@chromium.org>
Commit-Queue: Alex Chau <alexchau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706941}
parent 20dd9b71
...@@ -26,7 +26,7 @@ enum MessageType { ...@@ -26,7 +26,7 @@ enum MessageType {
// Message for sending between devices in Sharing. // Message for sending between devices in Sharing.
message SharingMessage { message SharingMessage {
// Identifier of sender. required. // Identifier of sender. required except for AckMessage.
string sender_guid = 1; string sender_guid = 1;
// Payload of the message, contains one of the messages below. required. // Payload of the message, contains one of the messages below. required.
......
...@@ -178,6 +178,7 @@ void SharingFCMHandler::SendAckMessage(const SharingMessage& original_message, ...@@ -178,6 +178,7 @@ void SharingFCMHandler::SendAckMessage(const SharingMessage& original_message,
sharing_fcm_sender_->SendMessageToDevice( sharing_fcm_sender_->SendMessageToDevice(
std::move(*sharing_info), kAckTimeToLive, std::move(ack_message), std::move(*sharing_info), kAckTimeToLive, std::move(ack_message),
/*sender_device_info=*/nullptr,
base::BindOnce(&SharingFCMHandler::OnAckMessageSent, base::BindOnce(&SharingFCMHandler::OnAckMessageSent,
weak_ptr_factory_.GetWeakPtr(), original_message_id, weak_ptr_factory_.GetWeakPtr(), original_message_id,
original_message_type)); original_message_type));
......
...@@ -44,13 +44,16 @@ class MockSharingMessageHandler : public SharingMessageHandler { ...@@ -44,13 +44,16 @@ class MockSharingMessageHandler : public SharingMessageHandler {
class MockSharingFCMSender : public SharingFCMSender { class MockSharingFCMSender : public SharingFCMSender {
public: public:
MockSharingFCMSender() MockSharingFCMSender()
: SharingFCMSender(nullptr, nullptr, nullptr, nullptr) {} : SharingFCMSender(/*gcm_driver=*/nullptr,
/*sync_preference=*/nullptr,
/*vapid_key_manager=*/nullptr) {}
~MockSharingFCMSender() override {} ~MockSharingFCMSender() override {}
MOCK_METHOD4(SendMessageToDevice, MOCK_METHOD5(SendMessageToDevice,
void(syncer::DeviceInfo::SharingInfo target, void(syncer::DeviceInfo::SharingInfo target,
base::TimeDelta time_to_live, base::TimeDelta time_to_live,
SharingMessage message, SharingMessage message,
std::unique_ptr<syncer::DeviceInfo> sender_device_info,
SendMessageCallback callback)); SendMessageCallback callback));
}; };
...@@ -121,7 +124,7 @@ TEST_F(SharingFCMHandlerTest, AckMessageHandler) { ...@@ -121,7 +124,7 @@ TEST_F(SharingFCMHandlerTest, AckMessageHandler) {
EXPECT_CALL(mock_sharing_message_handler_, EXPECT_CALL(mock_sharing_message_handler_,
OnMessage(ProtoEquals(sharing_message))); OnMessage(ProtoEquals(sharing_message)));
EXPECT_CALL(mock_sharing_fcm_sender_, SendMessageToDevice(_, _, _, _)) EXPECT_CALL(mock_sharing_fcm_sender_, SendMessageToDevice(_, _, _, _, _))
.Times(0); .Times(0);
sharing_fcm_handler_->AddSharingHandler(SharingMessage::kAckMessage, sharing_fcm_handler_->AddSharingHandler(SharingMessage::kAckMessage,
&mock_sharing_message_handler_); &mock_sharing_message_handler_);
...@@ -147,7 +150,7 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandler) { ...@@ -147,7 +150,7 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandler) {
// Tests OnMessage flow in SharingFCMHandler when no handler is registered. // 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(_, _, _, _)) EXPECT_CALL(mock_sharing_fcm_sender_, SendMessageToDevice(_, _, _, _, _))
.Times(0); .Times(0);
sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message); sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
...@@ -156,7 +159,8 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandler) { ...@@ -156,7 +159,8 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandler) {
OnMessage(ProtoEquals(sharing_message))); OnMessage(ProtoEquals(sharing_message)));
EXPECT_CALL(mock_sharing_fcm_sender_, EXPECT_CALL(mock_sharing_fcm_sender_,
SendMessageToDevice(DeviceMatcher(), testing::Eq(kAckTimeToLive), SendMessageToDevice(DeviceMatcher(), testing::Eq(kAckTimeToLive),
ProtoEquals(sharing_ack_message), _)); ProtoEquals(sharing_ack_message),
testing::Eq(nullptr), _));
sharing_fcm_handler_->AddSharingHandler(SharingMessage::kPingMessage, sharing_fcm_handler_->AddSharingHandler(SharingMessage::kPingMessage,
&mock_sharing_message_handler_); &mock_sharing_message_handler_);
sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message); sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
...@@ -164,7 +168,7 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandler) { ...@@ -164,7 +168,7 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandler) {
// Tests OnMessage flow in SharingFCMHandler after registered handler is // Tests OnMessage flow in SharingFCMHandler after registered handler is
// removed. // 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(_, _, _, _)) EXPECT_CALL(mock_sharing_fcm_sender_, SendMessageToDevice(_, _, _, _, _))
.Times(0); .Times(0);
sharing_fcm_handler_->RemoveSharingHandler(SharingMessage::kPingMessage); sharing_fcm_handler_->RemoveSharingHandler(SharingMessage::kPingMessage);
sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message); sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
...@@ -193,7 +197,8 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandlerSecondaryUser) { ...@@ -193,7 +197,8 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandlerSecondaryUser) {
OnMessage(ProtoEquals(sharing_message))); OnMessage(ProtoEquals(sharing_message)));
EXPECT_CALL(mock_sharing_fcm_sender_, EXPECT_CALL(mock_sharing_fcm_sender_,
SendMessageToDevice(DeviceMatcher(), testing::Eq(kAckTimeToLive), SendMessageToDevice(DeviceMatcher(), testing::Eq(kAckTimeToLive),
ProtoEquals(sharing_ack_message), _)); ProtoEquals(sharing_ack_message),
testing::Eq(nullptr), _));
sharing_fcm_handler_->AddSharingHandler(SharingMessage::kPingMessage, sharing_fcm_handler_->AddSharingHandler(SharingMessage::kPingMessage,
&mock_sharing_message_handler_); &mock_sharing_message_handler_);
sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message); sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
...@@ -223,7 +228,8 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandlerWithRecipientInfo) { ...@@ -223,7 +228,8 @@ TEST_F(SharingFCMHandlerTest, PingMessageHandlerWithRecipientInfo) {
OnMessage(ProtoEquals(sharing_message))); OnMessage(ProtoEquals(sharing_message)));
EXPECT_CALL(mock_sharing_fcm_sender_, EXPECT_CALL(mock_sharing_fcm_sender_,
SendMessageToDevice(DeviceMatcher(), testing::Eq(kAckTimeToLive), SendMessageToDevice(DeviceMatcher(), testing::Eq(kAckTimeToLive),
ProtoEquals(sharing_ack_message), _)); ProtoEquals(sharing_ack_message),
testing::Eq(nullptr), _));
sharing_fcm_handler_->AddSharingHandler(SharingMessage::kPingMessage, sharing_fcm_handler_->AddSharingHandler(SharingMessage::kPingMessage,
&mock_sharing_message_handler_); &mock_sharing_message_handler_);
sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message); sharing_fcm_handler_->OnMessage(kTestAppId, incoming_message);
......
...@@ -13,13 +13,10 @@ ...@@ -13,13 +13,10 @@
#include "components/gcm_driver/gcm_driver.h" #include "components/gcm_driver/gcm_driver.h"
#include "components/sync_device_info/device_info.h" #include "components/sync_device_info/device_info.h"
SharingFCMSender::SharingFCMSender( SharingFCMSender::SharingFCMSender(gcm::GCMDriver* gcm_driver,
gcm::GCMDriver* gcm_driver,
syncer::LocalDeviceInfoProvider* device_info_provider,
SharingSyncPreference* sync_preference, SharingSyncPreference* sync_preference,
VapidKeyManager* vapid_key_manager) VapidKeyManager* vapid_key_manager)
: gcm_driver_(gcm_driver), : gcm_driver_(gcm_driver),
device_info_provider_(device_info_provider),
sync_preference_(sync_preference), sync_preference_(sync_preference),
vapid_key_manager_(vapid_key_manager) {} vapid_key_manager_(vapid_key_manager) {}
...@@ -29,45 +26,12 @@ void SharingFCMSender::SendMessageToDevice( ...@@ -29,45 +26,12 @@ void SharingFCMSender::SendMessageToDevice(
syncer::DeviceInfo::SharingInfo target, syncer::DeviceInfo::SharingInfo target,
base::TimeDelta time_to_live, base::TimeDelta time_to_live,
SharingMessage message, SharingMessage message,
SendMessageCallback callback) { std::unique_ptr<syncer::DeviceInfo> sender_device_info,
auto send_message_closure = base::BindOnce(
&SharingFCMSender::DoSendMessageToDevice, weak_ptr_factory_.GetWeakPtr(),
std::move(target), time_to_live, std::move(message), std::move(callback));
if (device_info_provider_->GetLocalDeviceInfo()) {
std::move(send_message_closure).Run();
return;
}
if (!local_device_info_ready_subscription_) {
local_device_info_ready_subscription_ =
device_info_provider_->RegisterOnInitializedCallback(
base::AdaptCallbackForRepeating(
base::BindOnce(&SharingFCMSender::OnLocalDeviceInfoInitialized,
weak_ptr_factory_.GetWeakPtr())));
}
send_message_queue_.emplace_back(std::move(send_message_closure));
}
void SharingFCMSender::OnLocalDeviceInfoInitialized() {
for (auto& closure : send_message_queue_)
std::move(closure).Run();
send_message_queue_.clear();
local_device_info_ready_subscription_.reset();
}
void SharingFCMSender::DoSendMessageToDevice(
syncer::DeviceInfo::SharingInfo target,
base::TimeDelta time_to_live,
SharingMessage message,
SendMessageCallback callback) { SendMessageCallback callback) {
base::Optional<SharingSyncPreference::FCMRegistration> fcm_registration = base::Optional<SharingSyncPreference::FCMRegistration> fcm_registration =
sync_preference_->GetFCMRegistration(); sync_preference_->GetFCMRegistration();
base::Optional<syncer::DeviceInfo::SharingInfo> sharing_info = if (!fcm_registration) {
sync_preference_->GetLocalSharingInfo(); LOG(ERROR) << "Unable to retrieve FCM registration";
if (!fcm_registration || !sharing_info) {
LOG(ERROR) << "Unable to retrieve FCM registration or sharing info";
std::move(callback).Run(SharingSendMessageResult::kInternalError, std::move(callback).Run(SharingSendMessageResult::kInternalError,
base::nullopt); base::nullopt);
return; return;
...@@ -81,12 +45,16 @@ void SharingFCMSender::DoSendMessageToDevice( ...@@ -81,12 +45,16 @@ void SharingFCMSender::DoSendMessageToDevice(
return; return;
} }
const syncer::DeviceInfo* local_device_info =
device_info_provider_->GetLocalDeviceInfo();
message.set_sender_guid(local_device_info->guid());
if (message.payload_case() != SharingMessage::kAckMessage) { if (message.payload_case() != SharingMessage::kAckMessage) {
message.set_sender_device_name(local_device_info->client_name()); 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(); auto* sender_info = message.mutable_sender_info();
sender_info->set_fcm_token(sharing_info->fcm_token); sender_info->set_fcm_token(sharing_info->fcm_token);
sender_info->set_p256dh(sharing_info->p256dh); sender_info->set_p256dh(sharing_info->p256dh);
......
...@@ -18,13 +18,16 @@ ...@@ -18,13 +18,16 @@
#include "chrome/browser/sharing/sharing_send_message_result.h" #include "chrome/browser/sharing/sharing_send_message_result.h"
#include "components/gcm_driver/web_push_common.h" #include "components/gcm_driver/web_push_common.h"
#include "components/sync_device_info/device_info.h" #include "components/sync_device_info/device_info.h"
#include "components/sync_device_info/local_device_info_provider.h"
namespace gcm { namespace gcm {
class GCMDriver; class GCMDriver;
enum class SendWebPushMessageResult; enum class SendWebPushMessageResult;
} // namespace gcm } // namespace gcm
namespace syncer {
class DeviceInfo;
} // namespace syncer
class SharingSyncPreference; class SharingSyncPreference;
class VapidKeyManager; class VapidKeyManager;
...@@ -37,7 +40,6 @@ class SharingFCMSender { ...@@ -37,7 +40,6 @@ class SharingFCMSender {
base::Optional<std::string>)>; base::Optional<std::string>)>;
SharingFCMSender(gcm::GCMDriver* gcm_driver, SharingFCMSender(gcm::GCMDriver* gcm_driver,
syncer::LocalDeviceInfoProvider* device_info_provider,
SharingSyncPreference* sync_preference, SharingSyncPreference* sync_preference,
VapidKeyManager* vapid_key_manager); VapidKeyManager* vapid_key_manager);
virtual ~SharingFCMSender(); virtual ~SharingFCMSender();
...@@ -45,40 +47,23 @@ class SharingFCMSender { ...@@ -45,40 +47,23 @@ class SharingFCMSender {
// Sends a |message| to device identified by |target|, which expires // Sends a |message| to device identified by |target|, which expires
// after |time_to_live| seconds. |callback| will be invoked with message_id if // after |time_to_live| seconds. |callback| will be invoked with message_id if
// asynchronous operation succeeded, or base::nullopt if operation failed. // asynchronous operation succeeded, or base::nullopt if operation failed.
// If |recipient_info| is provided, it will be used for encryption and message // |sender_device_info| must be provided except for sending ack messages.
// delivery instead of looking up sync preference. virtual void SendMessageToDevice(
virtual void SendMessageToDevice(syncer::DeviceInfo::SharingInfo target, syncer::DeviceInfo::SharingInfo target,
base::TimeDelta time_to_live, base::TimeDelta time_to_live,
SharingMessage message, SharingMessage message,
std::unique_ptr<syncer::DeviceInfo> sender_device_info,
SendMessageCallback callback); SendMessageCallback callback);
private: private:
// Called once after the local device info is ready. This will only be used if
// we tried to send a message before the local device info was ready.
void OnLocalDeviceInfoInitialized();
// Actually sends the |message| to |target|. This will be called when our
// local device info is ready.
void DoSendMessageToDevice(syncer::DeviceInfo::SharingInfo target,
base::TimeDelta time_to_live,
SharingMessage message,
SendMessageCallback callback);
void OnMessageSent(SendMessageCallback callback, void OnMessageSent(SendMessageCallback callback,
gcm::SendWebPushMessageResult result, gcm::SendWebPushMessageResult result,
base::Optional<std::string> message_id); base::Optional<std::string> message_id);
gcm::GCMDriver* gcm_driver_; gcm::GCMDriver* gcm_driver_;
syncer::LocalDeviceInfoProvider* device_info_provider_;
SharingSyncPreference* sync_preference_; SharingSyncPreference* sync_preference_;
VapidKeyManager* vapid_key_manager_; VapidKeyManager* vapid_key_manager_;
// Subscription for the case when the local device info is not ready yet.
std::unique_ptr<syncer::LocalDeviceInfoProvider::Subscription>
local_device_info_ready_subscription_;
// Queued messages to be sent in OnLocalDeviceInfoInitialized.
std::vector<base::OnceClosure> send_message_queue_;
base::WeakPtrFactory<SharingFCMSender> weak_ptr_factory_{this}; base::WeakPtrFactory<SharingFCMSender> weak_ptr_factory_{this};
DISALLOW_COPY_AND_ASSIGN(SharingFCMSender); DISALLOW_COPY_AND_ASSIGN(SharingFCMSender);
......
...@@ -29,6 +29,8 @@ const char kSenderFcmToken[] = "sender_fcm_token"; ...@@ -29,6 +29,8 @@ const char kSenderFcmToken[] = "sender_fcm_token";
const char kSenderP256dh[] = "sender_p256dh"; const char kSenderP256dh[] = "sender_p256dh";
const char kSenderAuthSecret[] = "sender_auth_secret"; const char kSenderAuthSecret[] = "sender_auth_secret";
const char kAuthorizedEntity[] = "authorized_entity"; const char kAuthorizedEntity[] = "authorized_entity";
const char kLocalGuid[] = "id";
const char kLocalClientName[] = "name";
const int kTtlSeconds = 10; const int kTtlSeconds = 10;
class FakeGCMDriver : public gcm::FakeGCMDriver { class FakeGCMDriver : public gcm::FakeGCMDriver {
...@@ -97,9 +99,7 @@ class SharingFCMSenderTest : public testing::Test { ...@@ -97,9 +99,7 @@ class SharingFCMSenderTest : public testing::Test {
protected: protected:
SharingFCMSenderTest() SharingFCMSenderTest()
: sync_prefs_(&prefs_, &fake_device_info_sync_service), : sync_prefs_(&prefs_, &fake_device_info_sync_service),
sharing_fcm_sender_( sharing_fcm_sender_(&fake_gcm_driver_,
&fake_gcm_driver_,
fake_device_info_sync_service.GetLocalDeviceInfoProvider(),
&sync_prefs_, &sync_prefs_,
&vapid_key_manager_) { &vapid_key_manager_) {
SharingSyncPreference::RegisterProfilePrefs(prefs_.registry()); SharingSyncPreference::RegisterProfilePrefs(prefs_.registry());
...@@ -120,13 +120,6 @@ class SharingFCMSenderTest : public testing::Test { ...@@ -120,13 +120,6 @@ class SharingFCMSenderTest : public testing::Test {
TEST_F(SharingFCMSenderTest, NoFcmRegistration) { TEST_F(SharingFCMSenderTest, NoFcmRegistration) {
sync_prefs_.ClearFCMRegistration(); sync_prefs_.ClearFCMRegistration();
syncer::DeviceInfo* local_device_info =
fake_device_info_sync_service.GetLocalDeviceInfoProvider()
->GetMutableDeviceInfo();
local_device_info->set_sharing_info(syncer::DeviceInfo::SharingInfo(
kSenderFcmToken, kSenderP256dh, kSenderAuthSecret,
std::set<sync_pb::SharingSpecificFields::EnabledFeatures>()));
fake_device_info_sync_service.GetLocalDeviceInfoProvider()->SetReady(true);
std::unique_ptr<crypto::ECPrivateKey> vapid_key = std::unique_ptr<crypto::ECPrivateKey> vapid_key =
crypto::ECPrivateKey::Create(); crypto::ECPrivateKey::Create();
...@@ -140,10 +133,10 @@ TEST_F(SharingFCMSenderTest, NoFcmRegistration) { ...@@ -140,10 +133,10 @@ TEST_F(SharingFCMSenderTest, NoFcmRegistration) {
SharingSendMessageResult result; SharingSendMessageResult result;
base::Optional<std::string> message_id; base::Optional<std::string> message_id;
chrome_browser_sharing::SharingMessage sharing_message; chrome_browser_sharing::SharingMessage sharing_message;
sharing_message.mutable_ping_message(); sharing_message.mutable_ack_message();
sharing_fcm_sender_.SendMessageToDevice( sharing_fcm_sender_.SendMessageToDevice(
std::move(target), base::TimeDelta::FromSeconds(kTtlSeconds), std::move(target), base::TimeDelta::FromSeconds(kTtlSeconds),
std::move(sharing_message), std::move(sharing_message), /*sender_device_info=*/nullptr,
base::BindOnce(&SharingFCMSenderTest::OnMessageSent, base::BindOnce(&SharingFCMSenderTest::OnMessageSent,
base::Unretained(this), &result, &message_id)); base::Unretained(this), &result, &message_id));
...@@ -153,13 +146,6 @@ TEST_F(SharingFCMSenderTest, NoFcmRegistration) { ...@@ -153,13 +146,6 @@ TEST_F(SharingFCMSenderTest, NoFcmRegistration) {
TEST_F(SharingFCMSenderTest, NoVapidKey) { TEST_F(SharingFCMSenderTest, NoVapidKey) {
sync_prefs_.SetFCMRegistration(SharingSyncPreference::FCMRegistration( sync_prefs_.SetFCMRegistration(SharingSyncPreference::FCMRegistration(
kAuthorizedEntity, base::Time::Now())); kAuthorizedEntity, base::Time::Now()));
syncer::DeviceInfo* local_device_info =
fake_device_info_sync_service.GetLocalDeviceInfoProvider()
->GetMutableDeviceInfo();
local_device_info->set_sharing_info(syncer::DeviceInfo::SharingInfo(
kSenderFcmToken, kSenderP256dh, kSenderAuthSecret,
std::set<sync_pb::SharingSpecificFields::EnabledFeatures>()));
fake_device_info_sync_service.GetLocalDeviceInfoProvider()->SetReady(true);
ON_CALL(vapid_key_manager_, GetOrCreateKey()) ON_CALL(vapid_key_manager_, GetOrCreateKey())
.WillByDefault(testing::Return(nullptr)); .WillByDefault(testing::Return(nullptr));
...@@ -171,10 +157,10 @@ TEST_F(SharingFCMSenderTest, NoVapidKey) { ...@@ -171,10 +157,10 @@ TEST_F(SharingFCMSenderTest, NoVapidKey) {
SharingSendMessageResult result; SharingSendMessageResult result;
base::Optional<std::string> message_id; base::Optional<std::string> message_id;
chrome_browser_sharing::SharingMessage sharing_message; chrome_browser_sharing::SharingMessage sharing_message;
sharing_message.mutable_ping_message(); sharing_message.mutable_ack_message();
sharing_fcm_sender_.SendMessageToDevice( sharing_fcm_sender_.SendMessageToDevice(
std::move(target), base::TimeDelta::FromSeconds(kTtlSeconds), std::move(target), base::TimeDelta::FromSeconds(kTtlSeconds),
std::move(sharing_message), std::move(sharing_message), /*sender_device_info=*/nullptr,
base::BindOnce(&SharingFCMSenderTest::OnMessageSent, base::BindOnce(&SharingFCMSenderTest::OnMessageSent,
base::Unretained(this), &result, &message_id)); base::Unretained(this), &result, &message_id));
...@@ -184,38 +170,27 @@ TEST_F(SharingFCMSenderTest, NoVapidKey) { ...@@ -184,38 +170,27 @@ TEST_F(SharingFCMSenderTest, NoVapidKey) {
struct SharingFCMSenderResultTestData { struct SharingFCMSenderResultTestData {
const gcm::SendWebPushMessageResult web_push_result; const gcm::SendWebPushMessageResult web_push_result;
const SharingSendMessageResult expected_result; const SharingSendMessageResult expected_result;
const bool ready_before_send_message;
} kSharingFCMSenderResultTestData[] = { } kSharingFCMSenderResultTestData[] = {
{gcm::SendWebPushMessageResult::kSuccessful, {gcm::SendWebPushMessageResult::kSuccessful,
SharingSendMessageResult::kSuccessful, SharingSendMessageResult::kSuccessful},
/*ready_before_send_message=*/false},
{gcm::SendWebPushMessageResult::kSuccessful, {gcm::SendWebPushMessageResult::kSuccessful,
SharingSendMessageResult::kSuccessful, SharingSendMessageResult::kSuccessful},
/*ready_before_send_message=*/true},
{gcm::SendWebPushMessageResult::kDeviceGone, {gcm::SendWebPushMessageResult::kDeviceGone,
SharingSendMessageResult::kDeviceNotFound, SharingSendMessageResult::kDeviceNotFound},
/*ready_before_send_message=*/true},
{gcm::SendWebPushMessageResult::kNetworkError, {gcm::SendWebPushMessageResult::kNetworkError,
SharingSendMessageResult::kNetworkError, SharingSendMessageResult::kNetworkError},
/*ready_before_send_message=*/true},
{gcm::SendWebPushMessageResult::kPayloadTooLarge, {gcm::SendWebPushMessageResult::kPayloadTooLarge,
SharingSendMessageResult::kPayloadTooLarge, SharingSendMessageResult::kPayloadTooLarge},
/*ready_before_send_message=*/true},
{gcm::SendWebPushMessageResult::kEncryptionFailed, {gcm::SendWebPushMessageResult::kEncryptionFailed,
SharingSendMessageResult::kInternalError, SharingSendMessageResult::kInternalError},
/*ready_before_send_message=*/true},
{gcm::SendWebPushMessageResult::kCreateJWTFailed, {gcm::SendWebPushMessageResult::kCreateJWTFailed,
SharingSendMessageResult::kInternalError, SharingSendMessageResult::kInternalError},
/*ready_before_send_message=*/true},
{gcm::SendWebPushMessageResult::kServerError, {gcm::SendWebPushMessageResult::kServerError,
SharingSendMessageResult::kInternalError, SharingSendMessageResult::kInternalError},
/*ready_before_send_message=*/true},
{gcm::SendWebPushMessageResult::kParseResponseFailed, {gcm::SendWebPushMessageResult::kParseResponseFailed,
SharingSendMessageResult::kInternalError, SharingSendMessageResult::kInternalError},
/*ready_before_send_message=*/true},
{gcm::SendWebPushMessageResult::kVapidKeyInvalid, {gcm::SendWebPushMessageResult::kVapidKeyInvalid,
SharingSendMessageResult::kInternalError, SharingSendMessageResult::kInternalError}};
/*ready_before_send_message=*/true}};
class SharingFCMSenderResultTest class SharingFCMSenderResultTest
: public SharingFCMSenderTest, : public SharingFCMSenderTest,
...@@ -224,14 +199,16 @@ class SharingFCMSenderResultTest ...@@ -224,14 +199,16 @@ class SharingFCMSenderResultTest
TEST_P(SharingFCMSenderResultTest, ResultTest) { TEST_P(SharingFCMSenderResultTest, ResultTest) {
sync_prefs_.SetFCMRegistration(SharingSyncPreference::FCMRegistration( sync_prefs_.SetFCMRegistration(SharingSyncPreference::FCMRegistration(
kAuthorizedEntity, base::Time::Now())); kAuthorizedEntity, base::Time::Now()));
syncer::DeviceInfo* local_device_info = std::unique_ptr<syncer::DeviceInfo> sender_device_info =
fake_device_info_sync_service.GetLocalDeviceInfoProvider() std::make_unique<syncer::DeviceInfo>(
->GetMutableDeviceInfo(); "id", "name", "chrome_version", "user_agent",
local_device_info->set_sharing_info(syncer::DeviceInfo::SharingInfo( 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, kSenderFcmToken, kSenderP256dh, kSenderAuthSecret,
std::set<sync_pb::SharingSpecificFields::EnabledFeatures>())); std::set<sync_pb::SharingSpecificFields::EnabledFeatures>()));
fake_device_info_sync_service.GetLocalDeviceInfoProvider()->SetReady(
GetParam().ready_before_send_message);
fake_gcm_driver_.set_result(GetParam().web_push_result); fake_gcm_driver_.set_result(GetParam().web_push_result);
std::unique_ptr<crypto::ECPrivateKey> vapid_key = std::unique_ptr<crypto::ECPrivateKey> vapid_key =
...@@ -249,7 +226,7 @@ TEST_P(SharingFCMSenderResultTest, ResultTest) { ...@@ -249,7 +226,7 @@ TEST_P(SharingFCMSenderResultTest, ResultTest) {
sharing_message.mutable_ping_message(); sharing_message.mutable_ping_message();
sharing_fcm_sender_.SendMessageToDevice( sharing_fcm_sender_.SendMessageToDevice(
std::move(target), base::TimeDelta::FromSeconds(kTtlSeconds), std::move(target), base::TimeDelta::FromSeconds(kTtlSeconds),
std::move(sharing_message), std::move(sharing_message), std::move(sender_device_info),
base::BindOnce(&SharingFCMSenderTest::OnMessageSent, base::BindOnce(&SharingFCMSenderTest::OnMessageSent,
base::Unretained(this), &result, &message_id)); base::Unretained(this), &result, &message_id));
...@@ -267,10 +244,9 @@ TEST_P(SharingFCMSenderResultTest, ResultTest) { ...@@ -267,10 +244,9 @@ TEST_P(SharingFCMSenderResultTest, ResultTest) {
fake_gcm_driver_.message().urgency); fake_gcm_driver_.message().urgency);
chrome_browser_sharing::SharingMessage message_sent; chrome_browser_sharing::SharingMessage message_sent;
message_sent.ParseFromString(fake_gcm_driver_.message().payload); message_sent.ParseFromString(fake_gcm_driver_.message().payload);
EXPECT_EQ(local_device_info->guid(), message_sent.sender_guid());
EXPECT_TRUE(message_sent.has_ping_message()); EXPECT_TRUE(message_sent.has_ping_message());
EXPECT_EQ(local_device_info->client_name(), EXPECT_EQ(kLocalGuid, message_sent.sender_guid());
message_sent.sender_device_name()); EXPECT_EQ(kLocalClientName, message_sent.sender_device_name());
EXPECT_TRUE(message_sent.has_sender_info()); EXPECT_TRUE(message_sent.has_sender_info());
EXPECT_EQ(kSenderFcmToken, message_sent.sender_info().fcm_token()); EXPECT_EQ(kSenderFcmToken, message_sent.sender_info().fcm_token());
EXPECT_EQ(kSenderP256dh, message_sent.sender_info().p256dh()); EXPECT_EQ(kSenderP256dh, message_sent.sender_info().p256dh());
......
...@@ -216,7 +216,10 @@ std::unique_ptr<syncer::DeviceInfo> SharingService::GetDeviceByGuid( ...@@ -216,7 +216,10 @@ std::unique_ptr<syncer::DeviceInfo> SharingService::GetDeviceByGuid(
if (!IsSyncEnabled()) if (!IsSyncEnabled())
return nullptr; return nullptr;
return device_info_tracker_->GetDeviceInfo(guid); std::unique_ptr<syncer::DeviceInfo> device_info =
device_info_tracker_->GetDeviceInfo(guid);
return CloneDevice(device_info.get(),
GetDeviceNames(device_info.get()).full_name);
} }
SharingService::SharingDeviceList SharingService::GetDeviceCandidates( SharingService::SharingDeviceList SharingService::GetDeviceCandidates(
...@@ -278,16 +281,40 @@ void SharingService::SendMessageToDevice( ...@@ -278,16 +281,40 @@ void SharingService::SendMessageToDevice(
SharingSendMessageResult::kAckTimeout), SharingSendMessageResult::kAckTimeout),
kSendMessageTimeout); kSendMessageTimeout);
base::Optional<syncer::DeviceInfo::SharingInfo> sharing_info = // 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); sync_prefs_->GetSharingInfo(device_guid);
if (!sharing_info) { if (!target_sharing_info) {
InvokeSendMessageCallback(message_guid, message_type, InvokeSendMessageCallback(message_guid, message_type,
SharingSendMessageResult::kDeviceNotFound); SharingSendMessageResult::kDeviceNotFound);
return; return;
} }
const syncer::DeviceInfo* local_device_info =
local_device_info_provider_->GetLocalDeviceInfo();
if (!local_device_info) {
InvokeSendMessageCallback(message_guid, message_type,
SharingSendMessageResult::kInternalError);
return;
}
std::unique_ptr<syncer::DeviceInfo> sender_device_info = CloneDevice(
local_device_info, GetDeviceNames(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);
return;
}
fcm_sender_->SendMessageToDevice( fcm_sender_->SendMessageToDevice(
std::move(*sharing_info), time_to_live, std::move(message), std::move(*target_sharing_info), time_to_live, std::move(message),
std::move(sender_device_info),
base::BindOnce(&SharingService::OnMessageSent, base::BindOnce(&SharingService::OnMessageSent,
weak_ptr_factory_.GetWeakPtr(), base::TimeTicks::Now(), weak_ptr_factory_.GetWeakPtr(), base::TimeTicks::Now(),
message_guid, message_type)); message_guid, message_type));
......
...@@ -93,8 +93,7 @@ KeyedService* SharingServiceFactory::BuildServiceInstanceFor( ...@@ -93,8 +93,7 @@ KeyedService* SharingServiceFactory::BuildServiceInstanceFor(
profile->GetPrefs(), sync_prefs.get(), instance_id_service->driver(), profile->GetPrefs(), sync_prefs.get(), instance_id_service->driver(),
vapid_key_manager.get()); vapid_key_manager.get());
std::unique_ptr<SharingFCMSender> fcm_sender = std::unique_ptr<SharingFCMSender> fcm_sender =
std::make_unique<SharingFCMSender>(gcm_driver, local_device_info_provider, std::make_unique<SharingFCMSender>(gcm_driver, sync_prefs.get(),
sync_prefs.get(),
vapid_key_manager.get()); vapid_key_manager.get());
std::unique_ptr<SharingFCMHandler> fcm_handler = std::unique_ptr<SharingFCMHandler> fcm_handler =
std::make_unique<SharingFCMHandler>(gcm_driver, fcm_sender.get(), std::make_unique<SharingFCMHandler>(gcm_driver, fcm_sender.get(),
......
...@@ -63,6 +63,7 @@ class FakeGCMDriver : public gcm::FakeGCMDriver { ...@@ -63,6 +63,7 @@ class FakeGCMDriver : public gcm::FakeGCMDriver {
p256dh_ = p256dh; p256dh_ = p256dh;
auth_secret_ = auth_secret; auth_secret_ = auth_secret;
fcm_token_ = fcm_token; fcm_token_ = fcm_token;
message_ = std::move(message);
if (should_respond_) if (should_respond_)
std::move(callback).Run(gcm::SendWebPushMessageResult::kSuccessful, std::move(callback).Run(gcm::SendWebPushMessageResult::kSuccessful,
base::make_optional(kMessageId)); base::make_optional(kMessageId));
...@@ -79,9 +80,11 @@ class FakeGCMDriver : public gcm::FakeGCMDriver { ...@@ -79,9 +80,11 @@ class FakeGCMDriver : public gcm::FakeGCMDriver {
const std::string& p256dh() { return p256dh_; } const std::string& p256dh() { return p256dh_; }
const std::string& auth_secret() { return auth_secret_; } const std::string& auth_secret() { return auth_secret_; }
const std::string& fcm_token() { return fcm_token_; } const std::string& fcm_token() { return fcm_token_; }
const gcm::WebPushMessage& message() { return message_; }
private: private:
std::string p256dh_, auth_secret_, fcm_token_; std::string p256dh_, auth_secret_, fcm_token_;
gcm::WebPushMessage message_;
bool should_respond_ = true; bool should_respond_ = true;
}; };
...@@ -185,9 +188,7 @@ class SharingServiceTest : public testing::Test { ...@@ -185,9 +188,7 @@ class SharingServiceTest : public testing::Test {
/* pref_service= */ nullptr, sync_prefs_, &mock_instance_id_driver_, /* pref_service= */ nullptr, sync_prefs_, &mock_instance_id_driver_,
vapid_key_manager_, vapid_key_manager_,
fake_device_info_sync_service.GetLocalDeviceInfoProvider()); fake_device_info_sync_service.GetLocalDeviceInfoProvider());
fcm_sender_ = new SharingFCMSender( fcm_sender_ = new SharingFCMSender(&fake_gcm_driver_, sync_prefs_,
&fake_gcm_driver_,
fake_device_info_sync_service.GetLocalDeviceInfoProvider(), sync_prefs_,
vapid_key_manager_); vapid_key_manager_);
fcm_handler_ = new testing::NiceMock<MockSharingFCMHandler>(); fcm_handler_ = new testing::NiceMock<MockSharingFCMHandler>();
SharingSyncPreference::RegisterProfilePrefs(prefs_.registry()); SharingSyncPreference::RegisterProfilePrefs(prefs_.registry());
...@@ -407,6 +408,13 @@ TEST_F(SharingServiceTest, SendMessageToDeviceSuccess) { ...@@ -407,6 +408,13 @@ TEST_F(SharingServiceTest, SendMessageToDeviceSuccess) {
EXPECT_EQ(kAuthSecret, fake_gcm_driver_.auth_secret()); EXPECT_EQ(kAuthSecret, fake_gcm_driver_.auth_secret());
EXPECT_EQ(kFcmToken, fake_gcm_driver_.fcm_token()); EXPECT_EQ(kFcmToken, fake_gcm_driver_.fcm_token());
chrome_browser_sharing::SharingMessage sharing_message;
ASSERT_TRUE(
sharing_message.ParseFromString(fake_gcm_driver_.message().payload));
EXPECT_EQ("id", sharing_message.sender_guid());
EXPECT_EQ("model Computer manufacturer",
sharing_message.sender_device_name());
// Simulate ack message received by AckMessageHandler. // Simulate ack message received by AckMessageHandler.
SharingMessageHandler* ack_message_handler = fcm_handler_->GetSharingHandler( SharingMessageHandler* ack_message_handler = fcm_handler_->GetSharingHandler(
chrome_browser_sharing::SharingMessage::kAckMessage); chrome_browser_sharing::SharingMessage::kAckMessage);
...@@ -901,3 +909,15 @@ TEST_F(SharingServiceTest, DeviceCandidatesNames_Chromebooks) { ...@@ -901,3 +909,15 @@ TEST_F(SharingServiceTest, DeviceCandidatesNames_Chromebooks) {
EXPECT_EQ("HP Chromebook", candidates[0]->client_name()); EXPECT_EQ("HP Chromebook", candidates[0]->client_name());
EXPECT_EQ("Dell Chromebook", candidates[1]->client_name()); EXPECT_EQ("Dell Chromebook", candidates[1]->client_name());
} }
TEST_F(SharingServiceTest, GetDeviceByGuid) {
std::string guid = base::GenerateGUID();
std::unique_ptr<syncer::DeviceInfo> computer1 = CreateFakeDeviceInfo(
guid, "Fake device 1", sync_pb::SyncEnums_DeviceType_TYPE_LINUX,
{"Dell", "sno one", "serial no"});
fake_device_info_sync_service.GetDeviceInfoTracker()->Add(computer1.get());
std::unique_ptr<syncer::DeviceInfo> device_info =
GetSharingService()->GetDeviceByGuid(guid);
EXPECT_EQ("Dell Computer sno one", device_info->client_name());
}
...@@ -193,7 +193,12 @@ SharingSyncPreference::GetSharingInfo( ...@@ -193,7 +193,12 @@ SharingSyncPreference::GetSharingInfo(
base::Optional<syncer::DeviceInfo::SharingInfo> base::Optional<syncer::DeviceInfo::SharingInfo>
SharingSyncPreference::GetLocalSharingInfo() const { SharingSyncPreference::GetLocalSharingInfo() const {
auto* device_info = local_device_info_provider_->GetLocalDeviceInfo(); return GetLocalSharingInfo(local_device_info_provider_->GetLocalDeviceInfo());
}
base::Optional<syncer::DeviceInfo::SharingInfo>
SharingSyncPreference::GetLocalSharingInfo(
const syncer::DeviceInfo* device_info) const {
if (!device_info) if (!device_info)
return base::nullopt; return base::nullopt;
......
...@@ -92,6 +92,9 @@ class SharingSyncPreference { ...@@ -92,6 +92,9 @@ class SharingSyncPreference {
base::Optional<syncer::DeviceInfo::SharingInfo> GetLocalSharingInfo() const; base::Optional<syncer::DeviceInfo::SharingInfo> GetLocalSharingInfo() const;
base::Optional<syncer::DeviceInfo::SharingInfo> GetLocalSharingInfo(
const syncer::DeviceInfo* device_info) const;
void SetLocalSharingInfo(syncer::DeviceInfo::SharingInfo sharing_info); void SetLocalSharingInfo(syncer::DeviceInfo::SharingInfo sharing_info);
void ClearLocalSharingInfo(); void ClearLocalSharingInfo();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment