Send device name as part of sharing message.

Also, always show a notification, without the device name if not available.

Bug: 1005289
Change-Id: I78e20c174661e2e56f0a3f5152de478572243c3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1829380Reviewed-by: Alex Chau <alexchau@chromium.org>
Commit-Queue: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701527}

chrome/app/generated_resources.grd

@@ -6586,6 +6586,9 @@ the Bookmarks menu.">
         <message name="IDS_CONTENT_CONTEXT_SHARING_SHARED_CLIPBOARD_SINGLE_DEVICE" desc="The label of item for shared clipboard in context menu for showing device name when one device is available.">
           Copy to <ph name="DEVICE_NAME">$1<ex>Jimmy's Pixel</ex></ph>
         </message>
+        <message name="IDS_CONTENT_CONTEXT_SHARING_SHARED_CLIPBOARD_NOTIFICATION_TITLE_UNKNOWN_DEVICE" desc="Title text displayed in a shared clipboard notification when the source device name is unknown.">
+          Text shared from other device
+        </message>
         <message name="IDS_CONTENT_CONTEXT_SHARING_SHARED_CLIPBOARD_NOTIFICATION_TITLE" desc="The title of the notification shown when a clipboard was shared.">
           Text shared from <ph name="DEVICE_NAME">$1<ex>Jimmy's Pixel</ex></ph>
         </message>
@@ -6607,6 +6610,9 @@ the Bookmarks menu.">
         <message name="IDS_CONTENT_CONTEXT_SHARING_SHARED_CLIPBOARD_SINGLE_DEVICE" desc="In Title Case: The label of item for shared clipboard in context menu when single device is available.">
           Copy to <ph name="DEVICE_NAME">$1<ex>Jimmy's Pixel</ex></ph>
         </message>
+        <message name="IDS_CONTENT_CONTEXT_SHARING_SHARED_CLIPBOARD_NOTIFICATION_TITLE_UNKNOWN_DEVICE" desc="Title text displayed in a shared clipboard notification when the source device name is unknown.">
+          Text shared from Other Device
+        </message>
         <message name="IDS_CONTENT_CONTEXT_SHARING_SHARED_CLIPBOARD_NOTIFICATION_TITLE" desc="In Title Case: The title of the notification shown when a clipboard was shared.">
           Text shared from <ph name="DEVICE_NAME">$1<ex>Jimmy's Pixel</ex></ph>
         </message>

chrome/browser/sharing/proto/sharing_message.proto

@@ -27,6 +27,9 @@ message SharingMessage {
 
   // RecipientInfo for responding an AckMessage to the sender. optional.
   RecipientInfo sender_info = 6;
+
+  // The name of the device sending this message. optional.
+  string sender_device_name = 7;
 }
 
 // Message for pinging the receiver expecting an acknowledgement.

chrome/browser/sharing/shared_clipboard/shared_clipboard_message_handler.cc

@@ -30,6 +30,7 @@ void SharedClipboardMessageHandler::OnMessage(
 
   std::unique_ptr<syncer::DeviceInfo> device =
       sharing_service_->GetDeviceByGuid(message.sender_guid());
-  if (device)
-    ShowNotification(std::move(device));
+  const std::string& device_name =
+      device ? device->client_name() : message.sender_device_name();
+  ShowNotification(device_name);
 }

chrome/browser/sharing/shared_clipboard/shared_clipboard_message_handler.h

@@ -5,15 +5,11 @@
 #ifndef CHROME_BROWSER_SHARING_SHARED_CLIPBOARD_SHARED_CLIPBOARD_MESSAGE_HANDLER_H_
 #define CHROME_BROWSER_SHARING_SHARED_CLIPBOARD_SHARED_CLIPBOARD_MESSAGE_HANDLER_H_
 
-#include <memory>
+#include <string>
 
 #include "base/macros.h"
 #include "chrome/browser/sharing/sharing_message_handler.h"
 
-namespace syncer {
-class DeviceInfo;
-}  // namespace syncer
-
 class SharingService;
 
 // Handles incoming messages for the shared clipboard feature.
@@ -28,9 +24,8 @@ class SharedClipboardMessageHandler : public SharingMessageHandler {
 
  protected:
   // Called after the message has been copied to the clipboard. Implementers
-  // should display a notification using information from |device_info|.
-  virtual void ShowNotification(
-      std::unique_ptr<syncer::DeviceInfo> device_info) = 0;
+  // should display a notification using |device_name|.
+  virtual void ShowNotification(const std::string& device_name) = 0;
 
  private:
   SharingService* sharing_service_ = nullptr;

chrome/browser/sharing/shared_clipboard/shared_clipboard_message_handler_android.cc

@@ -6,7 +6,6 @@
 
 #include "base/android/jni_string.h"
 #include "chrome/android/chrome_jni_headers/SharedClipboardMessageHandler_jni.h"
-#include "components/sync_device_info/device_info.h"
 #include "ui/base/clipboard/clipboard_buffer.h"
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
 
@@ -18,9 +17,8 @@ SharedClipboardMessageHandlerAndroid::~SharedClipboardMessageHandlerAndroid() =
     default;
 
 void SharedClipboardMessageHandlerAndroid::ShowNotification(
-    std::unique_ptr<syncer::DeviceInfo> device_info) {
+    const std::string& device_name) {
   JNIEnv* env = base::android::AttachCurrentThread();
   Java_SharedClipboardMessageHandler_showNotification(
-      env,
-      base::android::ConvertUTF8ToJavaString(env, device_info->client_name()));
+      env, base::android::ConvertUTF8ToJavaString(env, device_name));
 }

chrome/browser/sharing/shared_clipboard/shared_clipboard_message_handler_android.h

@@ -19,8 +19,7 @@ class SharedClipboardMessageHandlerAndroid
 
  private:
   // SharedClipboardMessageHandler implementation.
-  void ShowNotification(
-      std::unique_ptr<syncer::DeviceInfo> device_info) override;
+  void ShowNotification(const std::string& device_name) override;
 
   DISALLOW_COPY_AND_ASSIGN(SharedClipboardMessageHandlerAndroid);
 };

chrome/browser/sharing/shared_clipboard/shared_clipboard_message_handler_desktop.cc

@@ -5,10 +5,10 @@
 #include "chrome/browser/sharing/shared_clipboard/shared_clipboard_message_handler_desktop.h"
 
 #include "base/guid.h"
+#include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/notifications/notification_display_service.h"
 #include "chrome/grit/generated_resources.h"
-#include "components/sync_device_info/device_info.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/image/image.h"
 #include "ui/message_center/public/cpp/notification.h"
@@ -25,18 +25,22 @@ SharedClipboardMessageHandlerDesktop::~SharedClipboardMessageHandlerDesktop() =
     default;
 
 void SharedClipboardMessageHandlerDesktop::ShowNotification(
-    std::unique_ptr<syncer::DeviceInfo> device_info) {
+    const std::string& device_name) {
   DCHECK(notification_display_service_);
-  DCHECK(device_info);
 
   std::string notification_id = base::GenerateGUID();
-  std::string device_name = device_info->client_name();
+
+  base::string16 notification_title =
+      device_name.empty()
+          ? l10n_util::GetStringUTF16(
+                IDS_CONTENT_CONTEXT_SHARING_SHARED_CLIPBOARD_NOTIFICATION_TITLE_UNKNOWN_DEVICE)
+          : l10n_util::GetStringFUTF16(
+                IDS_CONTENT_CONTEXT_SHARING_SHARED_CLIPBOARD_NOTIFICATION_TITLE,
+                base::UTF8ToUTF16(device_name));
 
   message_center::Notification notification(
       message_center::NOTIFICATION_TYPE_SIMPLE, notification_id,
-      l10n_util::GetStringFUTF16(
-          IDS_CONTENT_CONTEXT_SHARING_SHARED_CLIPBOARD_NOTIFICATION_TITLE,
-          base::UTF8ToUTF16(device_name)),
+      notification_title,
       l10n_util::GetStringUTF16(
           IDS_CONTENT_CONTEXT_SHARING_SHARED_CLIPBOARD_NOTIFICATION_DESCRIPTION),
       /* icon= */ gfx::Image(),
@@ -47,4 +51,4 @@ void SharedClipboardMessageHandlerDesktop::ShowNotification(
 
   notification_display_service_->Display(NotificationHandler::Type::SHARING,
                                          notification, /* metadata= */ nullptr);
-}
\ No newline at end of file
+}

chrome/browser/sharing/shared_clipboard/shared_clipboard_message_handler_desktop.h

@@ -22,8 +22,7 @@ class SharedClipboardMessageHandlerDesktop
 
  private:
   // SharedClipboardMessageHandler implementation.
-  void ShowNotification(
-      std::unique_ptr<syncer::DeviceInfo> device_info) override;
+  void ShowNotification(const std::string& device_name) override;
 
   NotificationDisplayService* notification_display_service_ = nullptr;
 

chrome/browser/sharing/shared_clipboard/shared_clipboard_message_handler_desktop_unittest.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/sharing/sharing_fcm_handler.h"
 #include "chrome/browser/sharing/sharing_service.h"
 #include "chrome/browser/sharing/vapid_key_manager.h"
+#include "chrome/grit/generated_resources.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/sync/protocol/sync_enums.pb.h"
 #include "components/sync_device_info/device_info.h"
@@ -20,11 +21,14 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/clipboard/clipboard.h"
+#include "ui/base/l10n/l10n_util.h"
 
 namespace {
 
-const char kText[] = "clipboard";
-const char kClientName[] = "Foo's Pixel";
+const char kText[] = "clipboard text";
+const char kEmptyDeviceName[] = "";
+const char kDeviceNameInDeviceInfo[] = "DeviceNameInDeviceInfo";
+const char kDeviceNameInMessage[] = "DeviceNameInMessage";
 
 class MockSharingService : public SharingService {
  public:
@@ -67,13 +71,35 @@ class SharedClipboardMessageHandlerTest : public testing::Test {
     ui::Clipboard::DestroyClipboardForCurrentThread();
   }
 
-  chrome_browser_sharing::SharingMessage CreateMessage(const std::string guid) {
+  chrome_browser_sharing::SharingMessage CreateMessage(
+      std::string guid,
+      std::string device_name) {
     chrome_browser_sharing::SharingMessage message;
     message.set_sender_guid(guid);
     message.mutable_shared_clipboard_message()->set_text(kText);
+    message.set_sender_device_name(device_name);
     return message;
   }
 
+  std::string GetClipboardText() {
+    base::string16 text;
+    ui::Clipboard::GetForCurrentThread()->ReadText(
+        ui::ClipboardBuffer::kCopyPaste, &text);
+    return base::UTF16ToUTF8(text);
+  }
+
+  message_center::Notification GetNotification() {
+    auto notifications =
+        notification_display_service_->GetDisplayedNotificationsForType(
+            NotificationHandler::Type::SHARING);
+    EXPECT_EQ(notifications.size(), 1u);
+
+    const message_center::Notification& notification = notifications[0];
+    EXPECT_EQ(message_center::NOTIFICATION_TYPE_SIMPLE, notification.type());
+
+    return notification;
+  }
+
  protected:
   content::BrowserTaskEnvironment task_environment_;
   std::unique_ptr<StubNotificationDisplayService> notification_display_service_;
@@ -86,7 +112,7 @@ class SharedClipboardMessageHandlerTest : public testing::Test {
 
 }  // namespace
 
-TEST_F(SharedClipboardMessageHandlerTest, MessageCopiedToClipboard) {
+TEST_F(SharedClipboardMessageHandlerTest, NotificationWithoutDeviceName) {
   std::string guid = base::GenerateGUID();
   {
     EXPECT_CALL(*sharing_service_, GetDeviceByGuid(guid))
@@ -94,27 +120,26 @@ TEST_F(SharedClipboardMessageHandlerTest, MessageCopiedToClipboard) {
             [](const std::string& guid) -> std::unique_ptr<syncer::DeviceInfo> {
               return nullptr;
             });
-    message_handler_->OnMessage(CreateMessage(guid));
+    message_handler_->OnMessage(CreateMessage(guid, kEmptyDeviceName));
   }
-  EXPECT_TRUE(notification_display_service_
-                  ->GetDisplayedNotificationsForType(
-                      NotificationHandler::Type::TRANSIENT)
-                  .empty());
-
-  base::string16 text;
-  ui::Clipboard::GetForCurrentThread()->ReadText(
-      ui::ClipboardBuffer::kCopyPaste, &text);
-  EXPECT_EQ(base::UTF16ToUTF8(text), kText);
+  EXPECT_EQ(GetClipboardText(), kText);
+
+  message_center::Notification notification = GetNotification();
+  EXPECT_EQ(
+      l10n_util::GetStringUTF16(
+          IDS_CONTENT_CONTEXT_SHARING_SHARED_CLIPBOARD_NOTIFICATION_TITLE_UNKNOWN_DEVICE),
+      notification.title());
 }
 
-TEST_F(SharedClipboardMessageHandlerTest, NotificationDisplayed) {
+TEST_F(SharedClipboardMessageHandlerTest,
+       NotificationWithDeviceNameFromDeviceInfo) {
   std::string guid = base::GenerateGUID();
   {
     EXPECT_CALL(*sharing_service_, GetDeviceByGuid(guid))
         .WillOnce(
             [](const std::string& guid) -> std::unique_ptr<syncer::DeviceInfo> {
               return std::make_unique<syncer::DeviceInfo>(
-                  base::GenerateGUID(), kClientName,
+                  base::GenerateGUID(), kDeviceNameInDeviceInfo,
                   /*chrome_version=*/"78.0.0.0",
                   /*sync_user_agent=*/"Chrome", sync_pb::SyncEnums::TYPE_LINUX,
                   /*signin_scoped_device_id=*/base::GenerateGUID(),
@@ -122,15 +147,31 @@ TEST_F(SharedClipboardMessageHandlerTest, NotificationDisplayed) {
                   /*send_tab_to_self_receiving_enabled=*/false,
                   /*sharing_info=*/base::nullopt);
             });
-    message_handler_->OnMessage(CreateMessage(guid));
+    message_handler_->OnMessage(CreateMessage(guid, kEmptyDeviceName));
+  }
+  message_center::Notification notification = GetNotification();
+  EXPECT_EQ(l10n_util::GetStringFUTF16(
+                IDS_CONTENT_CONTEXT_SHARING_SHARED_CLIPBOARD_NOTIFICATION_TITLE,
+                base::ASCIIToUTF16(kDeviceNameInDeviceInfo)),
+            notification.title());
+}
+
+TEST_F(SharedClipboardMessageHandlerTest,
+       NotificationWithDeviceNameFromMessage) {
+  std::string guid = base::GenerateGUID();
+  {
+    EXPECT_CALL(*sharing_service_, GetDeviceByGuid(guid))
+        .WillOnce(
+            [](const std::string& guid) -> std::unique_ptr<syncer::DeviceInfo> {
+              return nullptr;
+            });
+    message_handler_->OnMessage(CreateMessage(guid, kDeviceNameInMessage));
   }
-  auto notifications =
-      notification_display_service_->GetDisplayedNotificationsForType(
-          NotificationHandler::Type::SHARING);
-  ASSERT_EQ(notifications.size(), 1u);
-
-  const message_center::Notification& notification = notifications[0];
-  EXPECT_EQ(message_center::NOTIFICATION_TYPE_SIMPLE, notification.type());
-  EXPECT_EQ("Text shared from " + std::string(kClientName),
-            base::UTF16ToUTF8(notification.title()));
+  EXPECT_EQ(GetClipboardText(), kText);
+
+  message_center::Notification notification = GetNotification();
+  EXPECT_EQ(l10n_util::GetStringFUTF16(
+                IDS_CONTENT_CONTEXT_SHARING_SHARED_CLIPBOARD_NOTIFICATION_TITLE,
+                base::ASCIIToUTF16(kDeviceNameInMessage)),
+            notification.title());
 }

chrome/browser/sharing/sharing_fcm_sender.cc

@@ -59,8 +59,6 @@ void SharingFCMSender::DoSendMessageToDevice(Device target,
                                              base::TimeDelta time_to_live,
                                              SharingMessage message,
                                              SendMessageCallback callback) {
-  message.set_sender_guid(device_info_provider_->GetLocalDeviceInfo()->guid());
-
   auto fcm_registration = sync_preference_->GetFCMRegistration();
   if (!fcm_registration) {
     LOG(ERROR) << "Unable to retrieve FCM registration";
@@ -69,7 +67,12 @@ void SharingFCMSender::DoSendMessageToDevice(Device target,
     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) {
+    message.set_sender_device_name(local_device_info->client_name());
     auto* sender_info = message.mutable_sender_info();
     sender_info->set_fcm_token(fcm_registration->fcm_token);
     sender_info->set_p256dh(fcm_registration->p256dh);

chrome/browser/sharing/sharing_fcm_sender_unittest.cc

@@ -33,6 +33,7 @@ const char kAuthSecret[] = "auth_secret";
 const std::set<sync_pb::SharingSpecificFields::EnabledFeatures>
     kNoFeaturesEnabled;
 const char kSenderGuid[] = "test_sender_guid";
+const char kSenderDeviceName[] = "sender_device_name";
 const char kSenderFcmToken[] = "sender_fcm_token";
 const char kSenderP256dh[] = "sender_p256dh";
 const char kSenderAuthSecret[] = "sender_auth_secret";
@@ -86,7 +87,7 @@ class FakeLocalDeviceInfoProvider : public syncer::LocalDeviceInfoProvider {
  public:
   FakeLocalDeviceInfoProvider()
       : local_device_info_(kSenderGuid,
-                           "name",
+                           kSenderDeviceName,
                            "chrome_version",
                            "user_agent",
                            sync_pb::SyncEnums_DeviceType_TYPE_LINUX,
@@ -250,6 +251,7 @@ TEST_P(SharingFCMSenderResultTest, ResultTest) {
   message_sent.ParseFromString(fake_gcm_driver_.message().payload);
   EXPECT_EQ(kSenderGuid, message_sent.sender_guid());
   EXPECT_TRUE(message_sent.has_ping_message());
+  EXPECT_EQ(kSenderDeviceName, 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());