[ozone/x11] Enabled drag and drop.

This is the first step towards enabling drag and drop in Ozone/X11 using the components extracted in previous patches. Here the core part of the functionality is enabled. As Ozone allows selecting the platform at run time, it is necessary for it to select the proper OS exchange data provider at run time too. In this CL the new platform method is introduced that creates the provider, so the provider factory now asks the platform to instantiate a new provider. The XDragDropClient is modified so it can now be aggregated; it is used by the X11Window to handle the XDND events. Subsequent CLs will add rich drag and drop features such as drag image, mouse pointer shapes reflecting the current operation, etc. Bug: 1014860 Change-Id: I1a32f880fb9ac4fe8748507142d227c6cbafddbd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2033149 Reviewed-by: Robert Kroeger <rjkroege@chromium.org> Reviewed-by: Thomas Anderson <thomasanderson@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Darwin Huang <huangdarwin@chromium.org> Commit-Queue: Alexander Dunaev <adunaev@igalia.com> Cr-Commit-Position: refs/heads/master@{#762232}
FairyWorld · Apr 24, 2020 · 13e6e25 · 13e6e25
1 parent ae97855
commit 13e6e25
Show file tree

Hide file tree

Showing 63 changed files with 957 additions and 376 deletions.
diff --git a/ash/drag_drop/drag_drop_controller.cc b/ash/drag_drop/drag_drop_controller.cc
@@ -30,6 +30,7 @@
 #include "ui/base/clipboard/custom_data_helper.h"
 #include "ui/base/dragdrop/drag_drop_types.h"
 #include "ui/base/dragdrop/os_exchange_data.h"
+#include "ui/base/dragdrop/os_exchange_data_provider.h"
 #include "ui/base/hit_test.h"
 #include "ui/base/mojom/cursor_type.mojom-shared.h"
 #include "ui/events/event.h"
@@ -166,7 +167,7 @@ int DragDropController::StartDragAndDrop(
   if (!enabled_ || IsDragDropInProgress())
     return 0;
 
-  const ui::OSExchangeData::Provider* provider = &data->provider();
+  const ui::OSExchangeDataProvider* provider = &data->provider();
   // We do not support touch drag/drop without a drag image.
   if (source == ui::DragDropTypes::DRAG_EVENT_SOURCE_TOUCH &&
       provider->GetDragImage().size().IsEmpty())

diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
@@ -590,6 +590,7 @@ jumbo_static_library("ui") {
     "//third_party/zlib",
     "//ui/accessibility",
     "//ui/base",
+    "//ui/base:data_exchange",
     "//ui/base/clipboard",
     "//ui/base/ime",
     "//ui/compositor",

diff --git a/chrome/browser/ui/bookmarks/bookmark_browsertest.cc b/chrome/browser/ui/bookmarks/bookmark_browsertest.cc
@@ -237,8 +237,7 @@ IN_PROC_BROWSER_TEST_F(BookmarkBrowsertest, DragSingleBookmark) {
         GURL url;
         base::string16 title;
         EXPECT_TRUE(drag_data->provider().GetURLAndTitle(
-            ui::OSExchangeData::FilenameToURLPolicy::DO_NOT_CONVERT_FILENAMES,
-            &url, &title));
+            ui::DO_NOT_CONVERT_FILENAMES, &url, &title));
         EXPECT_EQ(page_url, url);
         EXPECT_EQ(page_title, title);
 #if !defined(OS_WIN)
@@ -289,8 +288,7 @@ IN_PROC_BROWSER_TEST_F(BookmarkBrowsertest, DragMultipleBookmarks) {
         // On Mac 10.11 and 10.12, this returns true, even though we set no url.
         // See https://crbug.com/893432.
         EXPECT_FALSE(drag_data->provider().GetURLAndTitle(
-            ui::OSExchangeData::FilenameToURLPolicy::DO_NOT_CONVERT_FILENAMES,
-            &url, &title));
+            ui::DO_NOT_CONVERT_FILENAMES, &url, &title));
 #endif
 #if !defined(OS_WIN)
         // On Windows, GetDragImage() is a NOTREACHED() as the Windows

diff --git a/chrome/browser/ui/views/frame/browser_root_view.cc b/chrome/browser/ui/views/frame/browser_root_view.cc
@@ -86,8 +86,7 @@ void OnFindURLMimeType(const GURL& url,
 bool GetURLForDrop(const ui::DropTargetEvent& event, GURL* url) {
   DCHECK(url);
   base::string16 title;
-  return event.data().GetURLAndTitle(ui::OSExchangeData::CONVERT_FILENAMES, url,
-                                     &title) &&
+  return event.data().GetURLAndTitle(ui::CONVERT_FILENAMES, url, &title) &&
          url->is_valid();
 }
 
@@ -148,7 +147,7 @@ bool BrowserRootView::CanDrop(const ui::OSExchangeData& data) {
     return false;
 
   // If there is a URL, we'll allow the drop.
-  if (data.HasURL(ui::OSExchangeData::CONVERT_FILENAMES))
+  if (data.HasURL(ui::CONVERT_FILENAMES))
     return true;
 
   // If there isn't a URL, see if we can 'paste and go'.

diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
@@ -53,6 +53,7 @@
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
 #include "ui/base/dragdrop/drag_drop_types.h"
 #include "ui/base/dragdrop/os_exchange_data.h"
+#include "ui/base/dragdrop/os_exchange_data_provider.h"
 #include "ui/base/ime/input_method.h"
 #include "ui/base/ime/input_method_keyboard_controller.h"
 #include "ui/base/ime/text_edit_commands.h"
@@ -1806,11 +1807,10 @@ int OmniboxViewViews::OnDrop(const ui::OSExchangeData& data) {
     return ui::DragDropTypes::DRAG_NONE;
 
   base::string16 text;
-  if (data.HasURL(ui::OSExchangeData::CONVERT_FILENAMES)) {
+  if (data.HasURL(ui::CONVERT_FILENAMES)) {
     GURL url;
     base::string16 title;
-    if (data.GetURLAndTitle(ui::OSExchangeData::CONVERT_FILENAMES, &url,
-                            &title)) {
+    if (data.GetURLAndTitle(ui::CONVERT_FILENAMES, &url, &title)) {
       text = StripJavascriptSchemas(base::UTF8ToUTF16(url.spec()));
     }
   } else if (data.HasString() && data.GetString(&text)) {

diff --git a/chrome/browser/ui/views/toolbar/home_button.cc b/chrome/browser/ui/views/toolbar/home_button.cc
@@ -160,7 +160,7 @@ bool HomeButton::GetDropFormats(
 }
 
 bool HomeButton::CanDrop(const OSExchangeData& data) {
-  return data.HasURL(ui::OSExchangeData::CONVERT_FILENAMES);
+  return data.HasURL(ui::CONVERT_FILENAMES);
 }
 
 int HomeButton::OnDragUpdated(const ui::DropTargetEvent& event) {
@@ -170,8 +170,8 @@ int HomeButton::OnDragUpdated(const ui::DropTargetEvent& event) {
 int HomeButton::OnPerformDrop(const ui::DropTargetEvent& event) {
   GURL new_homepage_url;
   base::string16 title;
-  if (event.data().GetURLAndTitle(
-          ui::OSExchangeData::CONVERT_FILENAMES, &new_homepage_url, &title) &&
+  if (event.data().GetURLAndTitle(ui::CONVERT_FILENAMES, &new_homepage_url,
+                                  &title) &&
       new_homepage_url.is_valid()) {
     PrefService* prefs = browser_->profile()->GetPrefs();
     bool old_is_ntp = prefs->GetBoolean(prefs::kHomePageIsNewTabPage);

diff --git a/components/bookmarks/browser/BUILD.gn b/components/bookmarks/browser/BUILD.gn
@@ -145,6 +145,7 @@ source_set("unit_tests") {
     "//components/prefs:test_support",
     "//testing/gtest",
     "//ui/base",
+    "//ui/base:data_exchange",
     "//ui/base/clipboard",
     "//url",
   ]

diff --git a/components/bookmarks/browser/bookmark_node_data_unittest.cc b/components/bookmarks/browser/bookmark_node_data_unittest.cc
@@ -18,6 +18,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/clipboard/clipboard.h"
 #include "ui/base/dragdrop/os_exchange_data.h"
+#include "ui/base/dragdrop/os_exchange_data_provider.h"
 #include "url/gurl.h"
 
 using base::ASCIIToUTF16;
@@ -62,7 +63,7 @@ class BookmarkNodeDataTest : public testing::Test {
 
 namespace {
 
-std::unique_ptr<ui::OSExchangeData::Provider> CloneProvider(
+std::unique_ptr<ui::OSExchangeDataProvider> CloneProvider(
     const ui::OSExchangeData& data) {
   return data.provider().Clone();
 }
@@ -144,8 +145,8 @@ TEST_F(BookmarkNodeDataTest, URL) {
   // Writing should also put the URL and title on the clipboard.
   GURL read_url;
   base::string16 read_title;
-  EXPECT_TRUE(data2.GetURLAndTitle(
-      ui::OSExchangeData::CONVERT_FILENAMES, &read_url, &read_title));
+  EXPECT_TRUE(
+      data2.GetURLAndTitle(ui::CONVERT_FILENAMES, &read_url, &read_title));
   EXPECT_EQ(url, read_url);
   EXPECT_EQ(title, read_title);
 }

diff --git a/components/bookmarks/browser/bookmark_node_data_views.cc b/components/bookmarks/browser/bookmark_node_data_views.cc
@@ -59,8 +59,7 @@ bool BookmarkNodeData::Read(const ui::OSExchangeData& data) {
     // See if there is a URL on the clipboard.
     GURL url;
     base::string16 title;
-    if (data.GetURLAndTitle(
-            ui::OSExchangeData::CONVERT_FILENAMES, &url, &title))
+    if (data.GetURLAndTitle(ui::CONVERT_FILENAMES, &url, &title))
       ReadFromTuple(url, title);
   }
 

diff --git a/content/browser/BUILD.gn b/content/browser/BUILD.gn
@@ -217,6 +217,7 @@ jumbo_source_set("browser") {
     "//ui/accessibility:ax_enums_mojo",
     "//ui/base",
     "//ui/base:buildflags",
+    "//ui/base:data_exchange",
     "//ui/base/clipboard",
     "//ui/base/cursor",
     "//ui/base/idle",

diff --git a/content/browser/web_contents/web_contents_view_aura.cc b/content/browser/web_contents/web_contents_view_aura.cc
@@ -156,7 +156,7 @@ class WebDragSourceAura : public NotificationObserver {
 // Fill out the OSExchangeData with a file contents, synthesizing a name if
 // necessary.
 void PrepareDragForFileContents(const DropData& drop_data,
-                                ui::OSExchangeData::Provider* provider) {
+                                ui::OSExchangeDataProvider* provider) {
   base::Optional<base::FilePath> filename =
       drop_data.GetSafeFilenameForImageFileContents();
   if (filename)
@@ -165,10 +165,9 @@ void PrepareDragForFileContents(const DropData& drop_data,
 #endif
 
 #if defined(OS_WIN)
-void PrepareDragForDownload(
-    const DropData& drop_data,
-    ui::OSExchangeData::Provider* provider,
-    WebContentsImpl* web_contents) {
+void PrepareDragForDownload(const DropData& drop_data,
+                            ui::OSExchangeDataProvider* provider,
+                            WebContentsImpl* web_contents) {
   const GURL& page_url = web_contents->GetLastCommittedURL();
   const std::string& page_encoding = web_contents->GetEncoding();
 
@@ -215,8 +214,8 @@ void PrepareDragForDownload(
       download_path, base::File(), download_url,
       Referrer(page_url, drop_data.referrer_policy), page_encoding,
       web_contents);
-  ui::OSExchangeData::DownloadFileInfo file_download(base::FilePath(),
-                                                     std::move(download_file));
+  ui::DownloadFileInfo file_download(base::FilePath(),
+                                     std::move(download_file));
   provider->SetDownloadFileInfo(&file_download);
 }
 #endif  // defined(OS_WIN)
@@ -230,7 +229,7 @@ const ui::ClipboardFormatType& GetFileSystemFileFormatType() {
 
 // Utility to fill a ui::OSExchangeDataProvider object from DropData.
 void PrepareDragData(const DropData& drop_data,
-                     ui::OSExchangeData::Provider* provider,
+                     ui::OSExchangeDataProvider* provider,
                      WebContentsImpl* web_contents) {
   provider->MarkOriginatedFromRenderer();
 #if defined(OS_WIN)
@@ -315,8 +314,7 @@ void PrepareDropData(DropData* drop_data, const ui::OSExchangeData& data) {
 
   GURL url;
   base::string16 url_title;
-  data.GetURLAndTitle(
-      ui::OSExchangeData::DO_NOT_CONVERT_FILENAMES, &url, &url_title);
+  data.GetURLAndTitle(ui::DO_NOT_CONVERT_FILENAMES, &url, &url_title);
   if (url.is_valid()) {
     drop_data->url = url;
     drop_data->url_title = url_title;
@@ -1071,7 +1069,7 @@ void WebContentsViewAura::StartDragging(
   ui::TouchSelectionController* selection_controller = GetSelectionController();
   if (selection_controller)
     selection_controller->HideAndDisallowShowingAutomatically();
-  std::unique_ptr<ui::OSExchangeData::Provider> provider =
+  std::unique_ptr<ui::OSExchangeDataProvider> provider =
       ui::OSExchangeDataProviderFactory::CreateProvider();
   PrepareDragData(drop_data, provider.get(), web_contents_);
 

diff --git a/content/browser/web_contents/web_contents_view_aura_unittest.cc b/content/browser/web_contents/web_contents_view_aura_unittest.cc
@@ -39,7 +39,10 @@ namespace content {
 namespace {
 constexpr gfx::Rect kBounds = gfx::Rect(0, 0, 20, 20);
 constexpr gfx::PointF kClientPt = {5, 10};
+
+#if !defined(USE_OZONE) || defined(OS_WIN) || defined(USE_X11)
 constexpr gfx::PointF kScreenPt = {17, 3};
+#endif
 
 // Runs a specified callback when a ui::MouseEvent is received.
 class RunCallbackOnActivation : public WebContentsDelegate {
@@ -198,6 +201,20 @@ TEST_F(WebContentsViewAuraTest, OccludeView) {
   EXPECT_EQ(web_contents()->GetVisibility(), Visibility::VISIBLE);
 }
 
+#if !defined(USE_OZONE) && !defined(OS_CHROMEOS)
+// TODO(crbug.com/1070483): Enable this test on Ozone.
+//
+// The expectations for the X11 implementation differ from other ones because
+// the GetString() method of the X11 data provider returns an empty string
+// if file data is also present, which is not the same for other
+// implementations.  (See the code under USE_X11 in the body of the test.)
+//
+// Ozone spawns the platform at run time, which would require this test to query
+// Ozone about the current platform, which would pull the Ozone platform as the
+// dependency here.
+//
+// Another solution could be fixing the X11 platform implementation so it
+// would behave the same way as other Ozone platforms do.
 TEST_F(WebContentsViewAuraTest, DragDropFiles) {
   WebContentsViewAura* view = GetView();
   auto data = std::make_unique<ui::OSExchangeData>();
@@ -280,6 +297,8 @@ TEST_F(WebContentsViewAuraTest, DragDropFiles) {
   }
 }
 
+#endif  // !defined(USE_OZONE) && !defined(OS_CHROMEOS)
+
 #if defined(OS_WIN) || defined(USE_X11)
 TEST_F(WebContentsViewAuraTest, DragDropFilesOriginateFromRenderer) {
   WebContentsViewAura* view = GetView();

diff --git a/ui/base/BUILD.gn b/ui/base/BUILD.gn
@@ -99,7 +99,6 @@ jumbo_component("base") {
     "default_style.h",
     "device_form_factor.h",
     "device_form_factor_desktop.cc",
-    "dragdrop/download_file_interface.h",
     "dragdrop/drag_drop_types.h",
     "dragdrop/drop_target_event.cc",
     "dragdrop/drop_target_event.h",
@@ -441,6 +440,7 @@ jumbo_component("base") {
     "//third_party/modp_b64",
     "//third_party/zlib:zlib",
     "//third_party/zlib/google:compression_utils",
+    "//ui/base:data_exchange",
     "//ui/base/clipboard:clipboard_types",
     "//ui/display",
     "//ui/events",
@@ -654,6 +654,28 @@ jumbo_component("base") {
   }
 }
 
+component("data_exchange") {
+  defines = [ "IS_UI_BASE_DATA_EXCHANGE_IMPL" ]
+
+  sources = [
+    "dragdrop/download_file_info.cc",
+    "dragdrop/download_file_info.h",
+    "dragdrop/download_file_interface.h",
+    "dragdrop/os_exchange_data_provider.h",
+    "dragdrop/os_exchange_data_provider_factory_ozone.cc",
+    "dragdrop/os_exchange_data_provider_factory_ozone.h",
+  ]
+
+  deps = [
+    "//base",
+    "//ui/base/clipboard:clipboard_types",
+    "//ui/base/dragdrop/file_info",
+    "//ui/gfx",
+    "//ui/gfx/geometry",
+    "//url",
+  ]
+}
+
 component("features") {
   output_name = "ui_base_features"
 
@@ -940,6 +962,7 @@ test("ui_base_unittests") {
     "//third_party/icu",
     "//third_party/zlib/google:compression_utils",
     "//ui/base",
+    "//ui/base:data_exchange",
     "//ui/base:test_support",
     "//ui/base:ui_data_pack",
     "//ui/base/clipboard:clipboard_test",

diff --git a/ui/base/dragdrop/DEPS b/ui/base/dragdrop/DEPS
@@ -1,3 +1,4 @@
 include_rules = [
+  "+ui/ozone/public/ozone_platform.h",
   "+third_party/mozilla",
 ]
diff --git a/ui/base/dragdrop/download_file_info.cc b/ui/base/dragdrop/download_file_info.cc
@@ -0,0 +1,18 @@
+// Copyright (c) 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/base/dragdrop/download_file_info.h"
+
+#include <utility>
+
+namespace ui {
+
+DownloadFileInfo::DownloadFileInfo(
+    const base::FilePath& filename,
+    std::unique_ptr<DownloadFileProvider> downloader)
+    : filename(filename), downloader(std::move(downloader)) {}
+
+DownloadFileInfo::~DownloadFileInfo() = default;
+
+}  // namespace ui
diff --git a/ui/base/dragdrop/download_file_info.h b/ui/base/dragdrop/download_file_info.h
@@ -0,0 +1,28 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef UI_BASE_DRAGDROP_DOWNLOAD_FILE_INFO_H_
+#define UI_BASE_DRAGDROP_DOWNLOAD_FILE_INFO_H_
+
+#include <memory>
+
+#include "base/component_export.h"
+#include "base/files/file_path.h"
+#include "ui/base/dragdrop/download_file_interface.h"
+
+namespace ui {
+
+// Encapsulates the info about a file to be downloaded.
+struct COMPONENT_EXPORT(UI_BASE_DATA_EXCHANGE) DownloadFileInfo {
+  DownloadFileInfo(const base::FilePath& filename,
+                   std::unique_ptr<DownloadFileProvider> downloader);
+  ~DownloadFileInfo();
+
+  base::FilePath filename;
+  std::unique_ptr<DownloadFileProvider> downloader;
+};
+
+}  // namespace ui
+
+#endif  // UI_BASE_DRAGDROP_DOWNLOAD_FILE_INFO_H_
diff --git a/ui/base/dragdrop/download_file_interface.h b/ui/base/dragdrop/download_file_interface.h
@@ -7,10 +7,9 @@
 
 #include "build/build_config.h"
 
+#include "base/component_export.h"
 #include "base/memory/ref_counted.h"
 
-#include "ui/base/ui_base_export.h"
-
 #if defined(OS_WIN)
 #include <objidl.h>
 #endif
@@ -22,7 +21,7 @@ class FilePath;
 namespace ui {
 
 // Defines the interface to observe the status of file download.
-class UI_BASE_EXPORT DownloadFileObserver
+class COMPONENT_EXPORT(UI_BASE_DATA_EXCHANGE) DownloadFileObserver
     : public base::RefCountedThreadSafe<DownloadFileObserver> {
  public:
   virtual void OnDownloadCompleted(const base::FilePath& file_path) = 0;
@@ -34,7 +33,7 @@ class UI_BASE_EXPORT DownloadFileObserver
 };
 
 // Defines the interface to control how a file is downloaded.
-class UI_BASE_EXPORT DownloadFileProvider {
+class COMPONENT_EXPORT(UI_BASE_DATA_EXCHANGE) DownloadFileProvider {
  public:
   virtual ~DownloadFileProvider() = default;