Revert "Add image support to Remote Copy."

This reverts commit 031922a6.

Reason for revert: RemoteCopyBrowserTest.ImageUrl is failing crbug.com/1023709

Original change's description:
> Add image support to Remote Copy.
> 
> Bug: 1018138, 1018132, 1020582
> Change-Id: I50d131ec34ccee3bf5b76167178eac4728b51bea
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1903203
> Reviewed-by: Richard Knoll <knollr@chromium.org>
> Reviewed-by: Darwin Huang <huangdarwin@chromium.org>
> Reviewed-by: Mike West <mkwst@chromium.org>
> Reviewed-by: Mikel Astiz <mastiz@chromium.org>
> Commit-Queue: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#714193}

TBR=mvanouwerkerk@chromium.org,mastiz@chromium.org,mkwst@chromium.org,huangdarwin@chromium.org,knollr@chromium.org

Change-Id: Ia5f3ffae72f8812b7caf0c678bc17a65e92c89a9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1018138, 1018132, 1020582
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1911063Reviewed-by: Keishi Hattori <keishi@chromium.org>
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714387}

chrome/browser/sharing/shared_clipboard/remote_copy_browsertest.cc

@@ -4,11 +4,9 @@
 
 #include <memory>
 #include <string>
-#include <vector>
 
 #include "base/guid.h"
 #include "base/macros.h"
-#include "base/optional.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_feature_list.h"
 #include "chrome/browser/notifications/notification_display_service_tester.h"
@@ -22,29 +20,13 @@
 #include "chrome/grit/generated_resources.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "ui/base/clipboard/clipboard.h"
-#include "ui/base/clipboard/clipboard_constants.h"
-#include "ui/base/clipboard/clipboard_monitor.h"
-#include "ui/base/clipboard/clipboard_observer.h"
 #include "ui/base/clipboard/test/test_clipboard.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/message_center/public/cpp/notification.h"
 
 namespace {
-const char kDeviceName[] = "test device name";
-const char kText[] = "test text";
-
-class ClipboardObserver : public ui::ClipboardObserver {
- public:
-  explicit ClipboardObserver(base::RepeatingClosure callback)
-      : callback_(callback) {}
-
-  void OnClipboardDataChanged() override { callback_.Run(); }
-
- private:
-  base::RepeatingClosure callback_;
-
-  DISALLOW_COPY_AND_ASSIGN(ClipboardObserver);
-};
+const char kText[] = "clipboard text";
+const char kDeviceName[] = "DeviceName";
 }  // namespace
 
 // Browser tests for the Remote Copy feature.
@@ -66,26 +48,13 @@ class RemoteCopyBrowserTestBase : public InProcessBrowserTest {
     ui::Clipboard::DestroyClipboardForCurrentThread();
   }
 
-  GURL StartServerAndGetURL(const std::string& relative_url) {
-    server_ = std::make_unique<net::EmbeddedTestServer>(
-        net::EmbeddedTestServer::TYPE_HTTPS);
-    server_->ServeFilesFromSourceDirectory(GetChromeTestDataDir());
-    EXPECT_TRUE(server_->Start());
-    return server_->GetURL(relative_url);
-  }
-
-  gcm::IncomingMessage CreateMessage(const std::string& device_name,
-                                     base::Optional<std::string> text,
-                                     base::Optional<GURL> image_url) {
+  gcm::IncomingMessage CreateMessage(std::string guid,
+                                     std::string device_name,
+                                     std::string text) {
     chrome_browser_sharing::SharingMessage sharing_message;
-    sharing_message.set_sender_guid(base::GenerateGUID());
+    sharing_message.set_sender_guid(guid);
     sharing_message.set_sender_device_name(device_name);
-    if (text) {
-      sharing_message.mutable_remote_copy_message()->set_text(text.value());
-    } else if (image_url) {
-      sharing_message.mutable_remote_copy_message()->set_image_url(
-          image_url.value().possibly_invalid_spec());
-    }
+    sharing_message.mutable_remote_copy_message()->set_text(text);
 
     gcm::IncomingMessage incoming_message;
     std::string serialized_sharing_message;
@@ -94,44 +63,13 @@ class RemoteCopyBrowserTestBase : public InProcessBrowserTest {
     return incoming_message;
   }
 
-  void SendTextMessage(const std::string& device_name,
-                       const std::string& text) {
-    sharing_service_->GetFCMHandlerForTesting()->OnMessage(
-        kSharingFCMAppID,
-        CreateMessage(device_name, text, /*image_url=*/base::nullopt));
-  }
-
-  void SendImageMessage(const std::string& device_name, const GURL& image_url) {
-    base::RunLoop run_loop;
-    ClipboardObserver observer(run_loop.QuitClosure());
-    ui::ClipboardMonitor::GetInstance()->AddObserver(&observer);
-    sharing_service_->GetFCMHandlerForTesting()->OnMessage(
-        kSharingFCMAppID,
-        CreateMessage(device_name, /*text*/ base::nullopt, image_url));
-    run_loop.Run();
-    ui::ClipboardMonitor::GetInstance()->RemoveObserver(&observer);
-  }
-
-  std::vector<base::string16> GetAvailableClipboardTypes() {
-    std::vector<base::string16> types;
-    bool contains_filenames;
-    ui::Clipboard::GetForCurrentThread()->ReadAvailableTypes(
-        ui::ClipboardBuffer::kCopyPaste, &types, &contains_filenames);
-    return types;
-  }
-
-  std::string ReadClipboardText() {
+  std::string GetClipboardText() {
     base::string16 text;
     ui::Clipboard::GetForCurrentThread()->ReadText(
         ui::ClipboardBuffer::kCopyPaste, &text);
     return base::UTF16ToUTF8(text);
   }
 
-  SkBitmap ReadClipboardImage() {
-    return ui::Clipboard::GetForCurrentThread()->ReadImage(
-        ui::ClipboardBuffer::kCopyPaste);
-  }
-
   message_center::Notification GetNotification() {
     auto notifications = notification_tester_->GetDisplayedNotificationsForType(
         NotificationHandler::Type::SHARING);
@@ -147,97 +85,25 @@ class RemoteCopyBrowserTestBase : public InProcessBrowserTest {
   base::test::ScopedFeatureList feature_list_;
   std::unique_ptr<NotificationDisplayServiceTester> notification_tester_;
   SharingService* sharing_service_;
-  std::unique_ptr<net::EmbeddedTestServer> server_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(RemoteCopyBrowserTestBase);
 };
 
-class RemoteCopyDisabledBrowserTest : public RemoteCopyBrowserTestBase {
- public:
-  RemoteCopyDisabledBrowserTest() {
-    feature_list_.InitAndDisableFeature(kRemoteCopyReceiver);
-  }
-};
-
-IN_PROC_BROWSER_TEST_F(RemoteCopyDisabledBrowserTest, FeatureDisabled) {
-  // The clipboard is empty.
-  ASSERT_TRUE(GetAvailableClipboardTypes().empty());
-
-  // Send a message with text.
-  SendTextMessage(kDeviceName, kText);
-
-  // The clipboard is still empty because the feature is disabled.
-  ASSERT_TRUE(GetAvailableClipboardTypes().empty());
-}
-
 class RemoteCopyBrowserTest : public RemoteCopyBrowserTestBase {
  public:
   RemoteCopyBrowserTest() {
-    feature_list_.InitAndEnableFeature(kRemoteCopyReceiver);
+    feature_list_.InitWithFeatures({kRemoteCopyReceiver}, {});
   }
 };
 
-IN_PROC_BROWSER_TEST_F(RemoteCopyBrowserTest, Text) {
-  // The clipboard is empty.
-  ASSERT_TRUE(GetAvailableClipboardTypes().empty());
-
-  // Send a message with text.
-  SendTextMessage(kDeviceName, kText);
-
-  // The text is in the clipboard and a notification is shown.
-  std::vector<base::string16> types = GetAvailableClipboardTypes();
-  ASSERT_EQ(1u, types.size());
-  ASSERT_EQ(ui::kMimeTypeText, base::UTF16ToASCII(types[0]));
-  ASSERT_EQ(kText, ReadClipboardText());
-  ASSERT_EQ(l10n_util::GetStringFUTF16(
-                IDS_CONTENT_CONTEXT_SHARING_SHARED_CLIPBOARD_NOTIFICATION_TITLE,
-                base::ASCIIToUTF16(kDeviceName)),
-            GetNotification().title());
-}
-
-IN_PROC_BROWSER_TEST_F(RemoteCopyBrowserTest, ImageUrl) {
-  // The clipboard is empty.
-  ASSERT_TRUE(GetAvailableClipboardTypes().empty());
-
-  // Send a message with an image url.
-  SendImageMessage(kDeviceName,
-                   StartServerAndGetURL("/image_decoding/droids.jpg"));
-
-  // The image is in the clipboard and a notification is shown.
-  std::vector<base::string16> types = GetAvailableClipboardTypes();
-  ASSERT_EQ(1u, types.size());
-  ASSERT_EQ(ui::kMimeTypePNG, base::UTF16ToASCII(types[0]));
-  SkBitmap bitmap = ReadClipboardImage();
-  ASSERT_FALSE(bitmap.drawsNothing());
-  ASSERT_EQ(2560, bitmap.width());
-  ASSERT_EQ(1920, bitmap.height());
-  ASSERT_EQ(l10n_util::GetStringFUTF16(
+IN_PROC_BROWSER_TEST_F(RemoteCopyBrowserTest, HandleMessage) {
+  sharing_service_->GetFCMHandlerForTesting()->OnMessage(
+      kSharingFCMAppID,
+      CreateMessage(base::GenerateGUID(), kDeviceName, kText));
+  EXPECT_EQ(GetClipboardText(), kText);
+  EXPECT_EQ(l10n_util::GetStringFUTF16(
                 IDS_CONTENT_CONTEXT_SHARING_SHARED_CLIPBOARD_NOTIFICATION_TITLE,
                 base::ASCIIToUTF16(kDeviceName)),
             GetNotification().title());
 }
-
-IN_PROC_BROWSER_TEST_F(RemoteCopyBrowserTest, TextThenImageUrl) {
-  // The clipboard is empty.
-  ASSERT_TRUE(GetAvailableClipboardTypes().empty());
-
-  // Send a message with text.
-  SendTextMessage(kDeviceName, kText);
-
-  // The text is in the clipboard.
-  std::vector<base::string16> types = GetAvailableClipboardTypes();
-  ASSERT_EQ(1u, types.size());
-  ASSERT_EQ(ui::kMimeTypeText, base::UTF16ToASCII(types[0]));
-  ASSERT_EQ(kText, ReadClipboardText());
-
-  // Send a message with an image url.
-  SendImageMessage(kDeviceName,
-                   StartServerAndGetURL("/image_decoding/droids.jpg"));
-
-  // The image is in the clipboard and the text has been cleared.
-  types = GetAvailableClipboardTypes();
-  ASSERT_EQ(1u, types.size());
-  ASSERT_EQ(ui::kMimeTypePNG, base::UTF16ToASCII(types[0]));
-  ASSERT_EQ(std::string(), ReadClipboardText());
-}

chrome/browser/sharing/shared_clipboard/remote_copy_message_handler.cc

@@ -6,22 +6,14 @@
 
 #include <memory>
 
-#include "base/bind.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/browser/notifications/notification_display_service_factory.h"
-#include "chrome/browser/profiles/profile.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/sync/protocol/sharing_message.pb.h"
 #include "components/sync/protocol/sharing_remote_copy_message.pb.h"
-#include "net/base/load_flags.h"
-#include "net/traffic_annotation/network_traffic_annotation.h"
-#include "services/network/public/cpp/is_potentially_trustworthy.h"
-#include "services/network/public/cpp/resource_request.h"
-#include "services/network/public/cpp/shared_url_loader_factory.h"
-#include "services/network/public/cpp/simple_url_loader.h"
 #include "ui/base/clipboard/clipboard_buffer.h"
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -31,30 +23,6 @@
 #include "ui/message_center/public/cpp/notifier_id.h"
 #include "url/gurl.h"
 
-namespace {
-const net::NetworkTrafficAnnotationTag kTrafficAnnotation =
-    net::DefineNetworkTrafficAnnotation("remote_copy_message_handler",
-                                        R"(
-          semantics {
-            sender: "RemoteCopyMessageHandler"
-            description:
-              "Fetches an image from a URL specified in an FCM message."
-            trigger:
-              "The user sent an image to this device from another device that "
-              "they control."
-            data:
-              "An image URL, from a Google storage service like blobstore."
-            destination: GOOGLE_OWNED_SERVICE
-          }
-          policy {
-            cookies_allowed: NO
-            setting:
-              "Users can disable this behavior by signing out of Chrome."
-            policy_exception_justification:
-              "Can be controlled via Chrome sign-in."
-          })");
-}  // namespace
-
 RemoteCopyMessageHandler::RemoteCopyMessageHandler(Profile* profile)
     : profile_(profile) {}
 
@@ -65,101 +33,26 @@ void RemoteCopyMessageHandler::OnMessage(
     DoneCallback done_callback) {
   DCHECK(message.has_remote_copy_message());
 
-  // First cancel any pending async tasks that might otherwise overwrite the
-  // results of the more recent message.
-  url_loader_.reset();
-  ImageDecoder::Cancel(this);
-
-  device_name_ = message.sender_device_name();
-
-  switch (message.remote_copy_message().content_case()) {
-    case chrome_browser_sharing::RemoteCopyMessage::kText:
-      HandleText(message.remote_copy_message().text());
-      break;
-    case chrome_browser_sharing::RemoteCopyMessage::kImageUrl:
-      HandleImage(message.remote_copy_message().image_url());
-      break;
-    case chrome_browser_sharing::RemoteCopyMessage::CONTENT_NOT_SET:
-      NOTREACHED();
-      break;
-  }
-
-  std::move(done_callback).Run(/*response=*/nullptr);
-}
-
-void RemoteCopyMessageHandler::HandleText(const std::string& text) {
-  if (!text.empty()) {
-    {
   ui::ScopedClipboardWriter(ui::ClipboardBuffer::kCopyPaste)
-          .WriteText(base::UTF8ToUTF16(text));
-    }
-    ShowNotification();
-  }
-  Finish();
-}
-
-void RemoteCopyMessageHandler::HandleImage(const std::string& image_url) {
-  GURL url(image_url);
-  // TODO(mvanouwerkerk): Whitelist check.
-  if (!network::IsUrlPotentiallyTrustworthy(url)) {
-    Finish();
-    return;
-  }
-
-  auto request = std::make_unique<network::ResourceRequest>();
-  request->url = url;
-  // This request should be unauthenticated (no cookies), and shouldn't be
-  // stored in the cache (this URL is only fetched once, ever.)
-  request->load_flags = net::LOAD_BYPASS_CACHE | net::LOAD_DISABLE_CACHE;
-  request->credentials_mode = network::mojom::CredentialsMode::kOmit;
+      .WriteText(base::UTF8ToUTF16(message.remote_copy_message().text()));
 
-  url_loader_ =
-      network::SimpleURLLoader::Create(std::move(request), kTrafficAnnotation);
-  // TODO(mvanouwerkerk): Downloads > 1MB (kMaxBoundedStringDownloadSize).
-  url_loader_->DownloadToString(
-      profile_->GetURLLoaderFactory().get(),
-      base::BindOnce(&RemoteCopyMessageHandler::OnURLLoadComplete,
-                     base::Unretained(this)),
-      network::SimpleURLLoader::kMaxBoundedStringDownloadSize);
-}
-
-void RemoteCopyMessageHandler::OnURLLoadComplete(
-    std::unique_ptr<std::string> content) {
-  url_loader_.reset();
-  if (!content || content->empty()) {
-    Finish();
-    return;
-  }
-
-  ImageDecoder::Start(this, *content);
-}
-
-void RemoteCopyMessageHandler::OnImageDecoded(const SkBitmap& decoded_image) {
-  if (!decoded_image.drawsNothing()) {
-    {
-      ui::ScopedClipboardWriter(ui::ClipboardBuffer::kCopyPaste)
-          .WriteImage(decoded_image);
-    }
-    ShowNotification();
-  }
-  Finish();
-}
+  ShowNotification(message.sender_device_name());
 
-void RemoteCopyMessageHandler::OnDecodeImageFailed() {
-  Finish();
+  std::move(done_callback).Run(/*response=*/nullptr);
 }
 
-void RemoteCopyMessageHandler::ShowNotification() {
+void RemoteCopyMessageHandler::ShowNotification(
+    const std::string& device_name) {
   std::string notification_id = base::GenerateGUID();
 
   // TODO(mvanouwerkerk): Adjust notification text and icon once we have mocks.
   base::string16 notification_title =
-      device_name_.empty()
+      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_));
+                base::UTF8ToUTF16(device_name));
 
   message_center::Notification notification(
       message_center::NOTIFICATION_TYPE_SIMPLE, notification_id,
@@ -175,8 +68,3 @@ void RemoteCopyMessageHandler::ShowNotification() {
   NotificationDisplayServiceFactory::GetForProfile(profile_)->Display(
       NotificationHandler::Type::SHARING, notification, /*metadata=*/nullptr);
 }
-
-void RemoteCopyMessageHandler::Finish() {
-  // TODO(mvanouwerkerk): UMA logging.
-  device_name_.clear();
-}

chrome/browser/sharing/shared_clipboard/remote_copy_message_handler.h

@@ -5,22 +5,15 @@
 #ifndef CHROME_BROWSER_SHARING_SHARED_CLIPBOARD_REMOTE_COPY_MESSAGE_HANDLER_H_
 #define CHROME_BROWSER_SHARING_SHARED_CLIPBOARD_REMOTE_COPY_MESSAGE_HANDLER_H_
 
-#include <memory>
 #include <string>
 
 #include "base/macros.h"
-#include "chrome/browser/image_decoder.h"
 #include "chrome/browser/sharing/sharing_message_handler.h"
 
 class Profile;
 
-namespace network {
-class SimpleURLLoader;
-}  // namespace network
-
 // Handles incoming messages for the remote copy feature.
-class RemoteCopyMessageHandler : public SharingMessageHandler,
-                                 public ImageDecoder::ImageRequest {
+class RemoteCopyMessageHandler : public SharingMessageHandler {
  public:
   explicit RemoteCopyMessageHandler(Profile* profile);
   ~RemoteCopyMessageHandler() override;
@@ -29,20 +22,10 @@ class RemoteCopyMessageHandler : public SharingMessageHandler,
   void OnMessage(chrome_browser_sharing::SharingMessage message,
                  DoneCallback done_callback) override;
 
-  // ImageDecoder::ImageRequest implementation:
-  void OnImageDecoded(const SkBitmap& decoded_image) override;
-  void OnDecodeImageFailed() override;
-
  private:
-  void HandleText(const std::string& text);
-  void HandleImage(const std::string& image_url);
-  void OnURLLoadComplete(std::unique_ptr<std::string> content);
-  void ShowNotification();
-  void Finish();
+  void ShowNotification(const std::string& device_name);
 
   Profile* profile_ = nullptr;
-  std::unique_ptr<network::SimpleURLLoader> url_loader_;
-  std::string device_name_;
 
   DISALLOW_COPY_AND_ASSIGN(RemoteCopyMessageHandler);
 };

components/sync/protocol/sharing_remote_copy_message.proto

@@ -11,12 +11,6 @@ option optimize_for = LITE_RUNTIME;
 
 // Message to pass content for the remote copy feature.
 message RemoteCopyMessage {
-  // The content to write to the clipboard. Required.
-  oneof content {
-    // Plain text.
+  // optional
   string text = 1;
-
-    // The URL of an image to download and write to the clipboard.
-    string image_url = 2;
-  }
 }

tools/traffic_annotation/summary/annotations.xml

@@ -222,7 +222,6 @@ Refer to README.md for content description and update process.
  <item id="ranker_url_fetcher" hash_code="95682324" type="0" content_hash_code="45958626" os_list="linux,windows" file_path="components/assist_ranker/ranker_url_fetcher.cc"/>
  <item id="rappor_report" hash_code="44606780" type="0" content_hash_code="111287826" os_list="linux,windows" file_path="components/rappor/log_uploader.cc"/>
  <item id="refresh_token_annotation_request" hash_code="7433837" type="1" second_id="29188932" deprecated="2018-01-17" content_hash_code="137103383" file_path=""/>
- <item id="remote_copy_message_handler" hash_code="80255301" type="0" content_hash_code="117673331" os_list="linux,windows" file_path="chrome/browser/sharing/shared_clipboard/remote_copy_message_handler.cc"/>
  <item id="remote_suggestions_provider" hash_code="49544361" type="0" content_hash_code="126329742" os_list="linux,windows" file_path="components/ntp_snippets/remote/cached_image_fetcher.cc"/>
  <item id="render_view_context_menu" hash_code="25844439" type="0" content_hash_code="69471170" os_list="linux,windows" file_path="chrome/browser/renderer_context_menu/render_view_context_menu.cc"/>
  <item id="renderer_initiated_download" hash_code="116443055" type="0" content_hash_code="37846436" os_list="linux,windows" file_path="content/browser/frame_host/render_frame_host_impl.cc"/>

ui/base/clipboard/test/test_clipboard.cc

@@ -175,7 +175,7 @@ void TestClipboard::WriteText(const char* text_data, size_t text_len) {
   if (IsSupportedClipboardBuffer(ClipboardBuffer::kSelection))
     GetStore(ClipboardBuffer::kSelection)
         .data[ClipboardFormatType::GetPlainTextType()] = text;
-  ClipboardMonitor::GetInstance()->NotifyClipboardDataChanged();
+  ui::ClipboardMonitor::GetInstance()->NotifyClipboardDataChanged();
 }
 
 void TestClipboard::WriteHTML(const char* markup_data,
@@ -217,7 +217,6 @@ void TestClipboard::WriteBitmap(const SkBitmap& bitmap) {
     NOTREACHED() << "Unable to convert bitmap for clipboard";
     return;
   }
-  ClipboardMonitor::GetInstance()->NotifyClipboardDataChanged();
 }
 
 void TestClipboard::WriteData(const ClipboardFormatType& format,