Commit 18450925 authored by Alexander Dunaev's avatar Alexander Dunaev Committed by Commit Bot

[ozone/linux] Enabled file contents in drag data on Linux.

Historically, there were a few discrepancies in behaviour of file
drag and drop on different platforms.  With introduction of Ozone and
implementation of X11 platform it became so complicated that a few tests
had been disabled.

The core difference between Linux and non-Linux was in how string and
URI contents are handled together.  Various Linux file managers pass
both string and URI together, for which there was a workaround in the
X11 implementation but not in Wayland one.

Another difference was that drags started in the renderer did not take
files (due to security reasons) but the non-backed data provider used
in ChromeOS and Wayland didn't recognise this scenario.

This CL unifies the behaviour on Linux platforms and makes the
non-backed data provider aware of drags originated from renderer, and
enables the tests that validate this behaviour.

Bug: 1121928, 1070483
Change-Id: If64857c60df295b37f1f5d4abe11338912134f5c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2461372Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Alexander Dunaev <adunaev@igalia.com>
Cr-Commit-Position: refs/heads/master@{#816195}
parent 5c7ae321
...@@ -153,7 +153,7 @@ class WebDragSourceAura : public NotificationObserver { ...@@ -153,7 +153,7 @@ class WebDragSourceAura : public NotificationObserver {
DISALLOW_COPY_AND_ASSIGN(WebDragSourceAura); DISALLOW_COPY_AND_ASSIGN(WebDragSourceAura);
}; };
#if defined(USE_X11) || defined(OS_WIN) #if defined(OS_LINUX) || defined(OS_WIN)
// Fill out the OSExchangeData with a file contents, synthesizing a name if // Fill out the OSExchangeData with a file contents, synthesizing a name if
// necessary. // necessary.
void PrepareDragForFileContents(const DropData& drop_data, void PrepareDragForFileContents(const DropData& drop_data,
...@@ -239,15 +239,11 @@ void PrepareDragData(const DropData& drop_data, ...@@ -239,15 +239,11 @@ void PrepareDragData(const DropData& drop_data,
if (!drop_data.download_metadata.empty()) if (!drop_data.download_metadata.empty())
PrepareDragForDownload(drop_data, provider, web_contents); PrepareDragForDownload(drop_data, provider, web_contents);
#endif #endif
#if defined(USE_X11) || defined(OS_WIN) #if defined(OS_LINUX) || defined(OS_WIN)
bool should_check_file_contents = true;
#if defined(USE_X11)
should_check_file_contents = !features::IsUsingOzonePlatform();
#endif
// We set the file contents before the URL because the URL also sets file // We set the file contents before the URL because the URL also sets file
// contents (to a .URL shortcut). We want to prefer file content data over // contents (to a .URL shortcut). We want to prefer file content data over
// a shortcut so we add it first. // a shortcut so we add it first.
if (should_check_file_contents && !drop_data.file_contents.empty()) if (!drop_data.file_contents.empty())
PrepareDragForFileContents(drop_data, provider); PrepareDragForFileContents(drop_data, provider);
#endif #endif
// Call SetString() before SetURL() when we actually have a custom string. // Call SetString() before SetURL() when we actually have a custom string.
......
...@@ -35,17 +35,13 @@ ...@@ -35,17 +35,13 @@
#include "ui/base/dragdrop/os_exchange_data_provider_win.h" #include "ui/base/dragdrop/os_exchange_data_provider_win.h"
#endif #endif
#if defined(USE_X11)
#include "ui/base/ui_base_features.h"
#endif
namespace content { namespace content {
namespace { namespace {
constexpr gfx::Rect kBounds = gfx::Rect(0, 0, 20, 20); constexpr gfx::Rect kBounds = gfx::Rect(0, 0, 20, 20);
constexpr gfx::PointF kClientPt = {5, 10}; constexpr gfx::PointF kClientPt = {5, 10};
#if !defined(USE_OZONE) || defined(OS_WIN) || defined(USE_X11) #if defined(OS_LINUX) || defined(OS_WIN)
constexpr gfx::PointF kScreenPt = {17, 3}; constexpr gfx::PointF kScreenPt = {17, 3};
#endif #endif
...@@ -204,20 +200,7 @@ TEST_F(WebContentsViewAuraTest, OccludeView) { ...@@ -204,20 +200,7 @@ TEST_F(WebContentsViewAuraTest, OccludeView) {
EXPECT_EQ(web_contents()->GetVisibility(), Visibility::VISIBLE); EXPECT_EQ(web_contents()->GetVisibility(), Visibility::VISIBLE);
} }
#if !defined(USE_OZONE) && !defined(OS_CHROMEOS) #if defined(OS_LINUX) && !defined(OS_CHROMEOS) || defined(OS_WIN)
// 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) { TEST_F(WebContentsViewAuraTest, DragDropFiles) {
WebContentsViewAura* view = GetView(); WebContentsViewAura* view = GetView();
auto data = std::make_unique<ui::OSExchangeData>(); auto data = std::make_unique<ui::OSExchangeData>();
...@@ -253,17 +236,14 @@ TEST_F(WebContentsViewAuraTest, DragDropFiles) { ...@@ -253,17 +236,14 @@ TEST_F(WebContentsViewAuraTest, DragDropFiles) {
view->OnDragEntered(event); view->OnDragEntered(event);
ASSERT_NE(nullptr, view->current_drop_data_); ASSERT_NE(nullptr, view->current_drop_data_);
#if defined(USE_X11) #if defined(OS_LINUX)
// By design, OSExchangeDataProviderX11::GetString returns an empty string // By design, Linux implementations return an empty string if file data
// if file data is also present. // is also present.
if (!features::IsUsingOzonePlatform()) { EXPECT_TRUE(!view->current_drop_data_->text ||
EXPECT_TRUE(!view->current_drop_data_->text || view->current_drop_data_->text->empty());
view->current_drop_data_->text->empty()); #else
} else EXPECT_EQ(string_data, view->current_drop_data_->text);
#endif #endif
{
EXPECT_EQ(string_data, view->current_drop_data_->text);
}
std::vector<ui::FileInfo> retrieved_file_infos = std::vector<ui::FileInfo> retrieved_file_infos =
view->current_drop_data_->filenames; view->current_drop_data_->filenames;
...@@ -287,17 +267,14 @@ TEST_F(WebContentsViewAuraTest, DragDropFiles) { ...@@ -287,17 +267,14 @@ TEST_F(WebContentsViewAuraTest, DragDropFiles) {
CheckDropData(view); CheckDropData(view);
#if defined(USE_X11) #if defined(OS_LINUX)
// By design, OSExchangeDataProviderX11::GetString returns an empty string // By design, Linux implementations returns an empty string if file data
// if file data is also present. // is also present.
if (!features::IsUsingOzonePlatform()) { EXPECT_TRUE(!drop_complete_data_->drop_data.text ||
EXPECT_TRUE(!drop_complete_data_->drop_data.text || drop_complete_data_->drop_data.text->empty());
drop_complete_data_->drop_data.text->empty()); #else
} else EXPECT_EQ(string_data, drop_complete_data_->drop_data.text);
#endif #endif
{
EXPECT_EQ(string_data, drop_complete_data_->drop_data.text);
}
retrieved_file_infos = drop_complete_data_->drop_data.filenames; retrieved_file_infos = drop_complete_data_->drop_data.filenames;
ASSERT_EQ(test_file_infos.size(), retrieved_file_infos.size()); ASSERT_EQ(test_file_infos.size(), retrieved_file_infos.size());
...@@ -308,15 +285,10 @@ TEST_F(WebContentsViewAuraTest, DragDropFiles) { ...@@ -308,15 +285,10 @@ TEST_F(WebContentsViewAuraTest, DragDropFiles) {
} }
} }
#endif // !defined(USE_OZONE) && !defined(OS_CHROMEOS) #endif // defined(OS_LINUX) && !defined(OS_CHROMEOS) || defined(OS_WIN)
#if defined(OS_WIN) || defined(USE_X11) #if defined(OS_LINUX) || defined(OS_WIN)
TEST_F(WebContentsViewAuraTest, DragDropFilesOriginateFromRenderer) { TEST_F(WebContentsViewAuraTest, DragDropFilesOriginateFromRenderer) {
#if defined(USE_X11)
// TODO(https://crbug.com/1109695): enable for Ozone/Linux.
if (features::IsUsingOzonePlatform())
return;
#endif
WebContentsViewAura* view = GetView(); WebContentsViewAura* view = GetView();
auto data = std::make_unique<ui::OSExchangeData>(); auto data = std::make_unique<ui::OSExchangeData>();
...@@ -355,9 +327,9 @@ TEST_F(WebContentsViewAuraTest, DragDropFilesOriginateFromRenderer) { ...@@ -355,9 +327,9 @@ TEST_F(WebContentsViewAuraTest, DragDropFilesOriginateFromRenderer) {
view->OnDragEntered(event); view->OnDragEntered(event);
ASSERT_NE(nullptr, view->current_drop_data_); ASSERT_NE(nullptr, view->current_drop_data_);
#if defined(USE_X11) #if defined(OS_LINUX)
// By design, OSExchangeDataProviderX11::GetString returns an empty string // By design, Linux implementations return an empty string if file data
// if file data is also present. // is also present.
EXPECT_TRUE(!view->current_drop_data_->text || EXPECT_TRUE(!view->current_drop_data_->text ||
view->current_drop_data_->text->empty()); view->current_drop_data_->text->empty());
#else #else
...@@ -379,9 +351,9 @@ TEST_F(WebContentsViewAuraTest, DragDropFilesOriginateFromRenderer) { ...@@ -379,9 +351,9 @@ TEST_F(WebContentsViewAuraTest, DragDropFilesOriginateFromRenderer) {
CheckDropData(view); CheckDropData(view);
#if defined(USE_X11) #if defined(OS_LINUX)
// By design, OSExchangeDataProviderX11::GetString returns an empty string // By design, Linux implementations returns an empty string if file data is
// if file data is also present. // also present.
EXPECT_TRUE(!drop_complete_data_->drop_data.text || EXPECT_TRUE(!drop_complete_data_->drop_data.text ||
drop_complete_data_->drop_data.text->empty()); drop_complete_data_->drop_data.text->empty());
#else #else
......
...@@ -71,7 +71,7 @@ class COMPONENT_EXPORT(UI_BASE_DATA_EXCHANGE) OSExchangeDataProvider { ...@@ -71,7 +71,7 @@ class COMPONENT_EXPORT(UI_BASE_DATA_EXCHANGE) OSExchangeDataProvider {
virtual bool HasFile() const = 0; virtual bool HasFile() const = 0;
virtual bool HasCustomFormat(const ClipboardFormatType& format) const = 0; virtual bool HasCustomFormat(const ClipboardFormatType& format) const = 0;
#if defined(USE_X11) || defined(OS_WIN) #if defined(OS_LINUX) || defined(OS_WIN)
virtual void SetFileContents(const base::FilePath& filename, virtual void SetFileContents(const base::FilePath& filename,
const std::string& file_contents) = 0; const std::string& file_contents) = 0;
#endif #endif
......
...@@ -40,12 +40,11 @@ std::unique_ptr<OSExchangeDataProvider> OSExchangeDataProviderNonBacked::Clone() ...@@ -40,12 +40,11 @@ std::unique_ptr<OSExchangeDataProvider> OSExchangeDataProviderNonBacked::Clone()
} }
void OSExchangeDataProviderNonBacked::MarkOriginatedFromRenderer() { void OSExchangeDataProviderNonBacked::MarkOriginatedFromRenderer() {
// TODO(dcheng): Currently unneeded because ChromeOS Aura correctly separates originated_from_renderer_ = true;
// URL and filename metadata, and does not implement the DownloadURL protocol.
} }
bool OSExchangeDataProviderNonBacked::DidOriginateFromRenderer() const { bool OSExchangeDataProviderNonBacked::DidOriginateFromRenderer() const {
return false; return originated_from_renderer_;
} }
void OSExchangeDataProviderNonBacked::SetString(const base::string16& data) { void OSExchangeDataProviderNonBacked::SetString(const base::string16& data) {
...@@ -85,6 +84,15 @@ void OSExchangeDataProviderNonBacked::SetPickledData( ...@@ -85,6 +84,15 @@ void OSExchangeDataProviderNonBacked::SetPickledData(
} }
bool OSExchangeDataProviderNonBacked::GetString(base::string16* data) const { bool OSExchangeDataProviderNonBacked::GetString(base::string16* data) const {
#if defined(OS_LINUX)
if (HasFile()) {
// Various Linux file managers both pass a list of file:// URIs and set the
// string representation to the URI. We explicitly don't want to return use
// this representation.
return false;
}
#endif // defined(OS_LINUX)
if ((formats_ & OSExchangeData::STRING) == 0) if ((formats_ & OSExchangeData::STRING) == 0)
return false; return false;
*data = string_; *data = string_;
...@@ -160,11 +168,11 @@ bool OSExchangeDataProviderNonBacked::HasCustomFormat( ...@@ -160,11 +168,11 @@ bool OSExchangeDataProviderNonBacked::HasCustomFormat(
return base::Contains(pickle_data_, format); return base::Contains(pickle_data_, format);
} }
#if defined(USE_X11) #if defined(OS_LINUX)
void OSExchangeDataProviderNonBacked::SetFileContents( void OSExchangeDataProviderNonBacked::SetFileContents(
const base::FilePath& filename, const base::FilePath& filename,
const std::string& file_contents) { const std::string& file_contents) {
NOTREACHED(); NOTIMPLEMENTED();
} }
#endif #endif
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/component_export.h" #include "base/component_export.h"
#include "base/pickle.h" #include "base/pickle.h"
#include "build/build_config.h"
#include "ui/base/dragdrop/file_info/file_info.h" #include "ui/base/dragdrop/file_info/file_info.h"
#include "ui/base/dragdrop/os_exchange_data_provider.h" #include "ui/base/dragdrop/os_exchange_data_provider.h"
#include "ui/gfx/geometry/vector2d.h" #include "ui/gfx/geometry/vector2d.h"
...@@ -58,7 +59,7 @@ class COMPONENT_EXPORT(UI_BASE) OSExchangeDataProviderNonBacked ...@@ -58,7 +59,7 @@ class COMPONENT_EXPORT(UI_BASE) OSExchangeDataProviderNonBacked
bool HasURL(FilenameToURLPolicy policy) const override; bool HasURL(FilenameToURLPolicy policy) const override;
bool HasFile() const override; bool HasFile() const override;
bool HasCustomFormat(const ClipboardFormatType& format) const override; bool HasCustomFormat(const ClipboardFormatType& format) const override;
#if defined(USE_X11) #if defined(OS_LINUX)
void SetFileContents(const base::FilePath& filename, void SetFileContents(const base::FilePath& filename,
const std::string& file_contents) override; const std::string& file_contents) override;
#endif #endif
...@@ -104,6 +105,8 @@ class COMPONENT_EXPORT(UI_BASE) OSExchangeDataProviderNonBacked ...@@ -104,6 +105,8 @@ class COMPONENT_EXPORT(UI_BASE) OSExchangeDataProviderNonBacked
// For HTML format // For HTML format
base::string16 html_; base::string16 html_;
GURL base_url_; GURL base_url_;
bool originated_from_renderer_ = false;
}; };
} // namespace ui } // namespace ui
......
...@@ -367,7 +367,6 @@ bool XOSExchangeDataProvider::HasCustomFormat( ...@@ -367,7 +367,6 @@ bool XOSExchangeDataProvider::HasCustomFormat(
return !requested_types.empty(); return !requested_types.empty();
} }
#if defined(USE_X11)
void XOSExchangeDataProvider::SetFileContents( void XOSExchangeDataProvider::SetFileContents(
const base::FilePath& filename, const base::FilePath& filename,
const std::string& file_contents) { const std::string& file_contents) {
...@@ -398,7 +397,6 @@ void XOSExchangeDataProvider::SetFileContents( ...@@ -398,7 +397,6 @@ void XOSExchangeDataProvider::SetFileContents(
scoped_refptr<base::RefCountedMemory>( scoped_refptr<base::RefCountedMemory>(
base::RefCountedString::TakeString(&file_contents_copy))); base::RefCountedString::TakeString(&file_contents_copy)));
} }
#endif
void XOSExchangeDataProvider::SetHtml(const base::string16& html, void XOSExchangeDataProvider::SetHtml(const base::string16& html,
const GURL& base_url) { const GURL& base_url) {
......
...@@ -82,10 +82,8 @@ class COMPONENT_EXPORT(UI_BASE_X) XOSExchangeDataProvider ...@@ -82,10 +82,8 @@ class COMPONENT_EXPORT(UI_BASE_X) XOSExchangeDataProvider
bool HasURL(FilenameToURLPolicy policy) const override; bool HasURL(FilenameToURLPolicy policy) const override;
bool HasFile() const override; bool HasFile() const override;
bool HasCustomFormat(const ClipboardFormatType& format) const override; bool HasCustomFormat(const ClipboardFormatType& format) const override;
#if defined(USE_X11)
void SetFileContents(const base::FilePath& filename, void SetFileContents(const base::FilePath& filename,
const std::string& file_contents) override; const std::string& file_contents) override;
#endif
void SetHtml(const base::string16& html, const GURL& base_url) override; void SetHtml(const base::string16& html, const GURL& base_url) override;
bool GetHtml(base::string16* html, GURL* base_url) const override; bool GetHtml(base::string16* html, GURL* base_url) const override;
......
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