Skip to content

Commit

Permalink
Reland "Implement ui::ClipboardMus."
Browse files Browse the repository at this point in the history
Should fix size_t conversion issues.

Original issue's description:
> Implement ui::ClipboardMus.
>
> This lets chrome in mash read and write from the mus server's clipboard.
>
> BUG=581460
>
> Committed: https://crrev.com/bbc764eb7a7c1937d26f43f6087604ebc2d048ad
> Cr-Commit-Position: refs/heads/master@{#398351}

TBR=dcheng@chromium.org,brettw@chromium.org,sky@chromium.org
BUG=581460

Review-Url: https://codereview.chromium.org/2044973002
Cr-Commit-Position: refs/heads/master@{#398413}
  • Loading branch information
eglaysher authored and Commit bot committed Jun 7, 2016
1 parent 9efd9d5 commit e163bd7
Show file tree
Hide file tree
Showing 33 changed files with 783 additions and 130 deletions.
5 changes: 5 additions & 0 deletions base/threading/thread_restrictions.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ class WindowResizeHelperMac;
}

namespace views {
class ClipboardMus;
class ScreenMus;
}

Expand Down Expand Up @@ -235,6 +236,10 @@ class BASE_EXPORT ThreadRestrictions {
#if !defined(OFFICIAL_BUILD)
friend class content::SoftwareOutputDeviceMus; // Interim non-production code
#endif
// In the non-mus case, we called blocking OS functions in the ui::Clipboard
// implementation which weren't caught by threading restrictions. Our
// blocking calls to mus, however, are.
friend class views::ClipboardMus;
friend class views::ScreenMus;
// END USAGE THAT NEEDS TO BE FIXED.

Expand Down
1 change: 1 addition & 0 deletions components/mus/clipboard/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ source_set("lib") {
deps = [
"//base",
"//components/mus/public/interfaces",
"//mojo/common",
"//mojo/public/cpp/bindings",
"//mojo/public/cpp/bindings:callback",
"//services/shell/public/cpp",
Expand Down
16 changes: 6 additions & 10 deletions components/mus/clipboard/clipboard_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <utility>

#include "base/macros.h"
#include "mojo/common/common_type_converters.h"
#include "mojo/public/cpp/bindings/array.h"
#include "mojo/public/cpp/bindings/callback.h"
#include "mojo/public/cpp/bindings/string.h"
Expand Down Expand Up @@ -84,20 +85,15 @@ void ClipboardImpl::GetAvailableMimeTypes(
clipboard_state_[clipboard_num]->GetMimeTypes());
}

void ClipboardImpl::ReadMimeType(
uint64_t sequence,
void ClipboardImpl::ReadClipboardData(
Clipboard::Type clipboard_type,
const String& mime_type,
const ReadMimeTypeCallback& callback) {
const ReadClipboardDataCallback& callback) {
int clipboard_num = static_cast<int>(clipboard_type);
Array<uint8_t> mime_data(nullptr);
bool valid_sequence_number =
clipboard_state_[clipboard_num]->sequence_number() == sequence;

if (valid_sequence_number)
clipboard_state_[clipboard_num]->GetData(mime_type, &mime_data);

callback.Run(valid_sequence_number, std::move(mime_data));
uint64_t sequence = clipboard_state_[clipboard_num]->sequence_number();
clipboard_state_[clipboard_num]->GetData(mime_type, &mime_data);
callback.Run(sequence, std::move(mime_data));
}

void ClipboardImpl::WriteClipboardData(
Expand Down
8 changes: 3 additions & 5 deletions components/mus/clipboard/clipboard_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,9 @@ class ClipboardImpl : public mojom::Clipboard {
void GetAvailableMimeTypes(
mojom::Clipboard::Type clipboard_types,
const GetAvailableMimeTypesCallback& callback) override;
void ReadMimeType(
uint64_t sequence,
mojom::Clipboard::Type clipboard_type,
const mojo::String& mime_type,
const ReadMimeTypeCallback& callback) override;
void ReadClipboardData(mojom::Clipboard::Type clipboard_type,
const mojo::String& mime_type,
const ReadClipboardDataCallback& callback) override;
void WriteClipboardData(
mojom::Clipboard::Type clipboard_type,
mojo::Map<mojo::String, mojo::Array<uint8_t>> data,
Expand Down
31 changes: 15 additions & 16 deletions components/mus/clipboard/clipboard_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,22 +58,21 @@ class ClipboardAppTest : public shell::test::ShellTest {
return types.To<std::vector<std::string>>();
}

bool GetDataOfType(uint64_t sequence_number, const std::string& mime_type,
std::string* data) {
bool GetDataOfType(const std::string& mime_type, std::string* data) {
bool valid = false;
Array<uint8_t> raw_data;
clipboard_->ReadMimeType(
sequence_number, Clipboard::Type::COPY_PASTE, mime_type,
&valid, &raw_data);
valid = valid && !raw_data.is_null();
uint64_t sequence_number = 0;
clipboard_->ReadClipboardData(Clipboard::Type::COPY_PASTE, mime_type,
&sequence_number, &raw_data);
valid = !raw_data.is_null();
*data = raw_data.is_null() ? "" : raw_data.To<std::string>();
return valid;
}

void SetStringText(const std::string& data) {
uint64_t sequence_number;
Map<String, Array<uint8_t>> mime_data;
mime_data[Clipboard::MIME_TYPE_TEXT] = Array<uint8_t>::From(data);
mime_data[mojom::kMimeTypeText] = Array<uint8_t>::From(data);
clipboard_->WriteClipboardData(Clipboard::Type::COPY_PASTE,
std::move(mime_data),
&sequence_number);
Expand All @@ -89,26 +88,26 @@ TEST_F(ClipboardAppTest, EmptyClipboardOK) {
EXPECT_EQ(0ul, GetSequenceNumber());
EXPECT_TRUE(GetAvailableFormatMimeTypes().empty());
std::string data;
EXPECT_FALSE(GetDataOfType(0ul, Clipboard::MIME_TYPE_TEXT, &data));
EXPECT_FALSE(GetDataOfType(mojom::kMimeTypeText, &data));
}

TEST_F(ClipboardAppTest, CanReadBackText) {
std::string data;
EXPECT_EQ(0ul, GetSequenceNumber());
EXPECT_FALSE(GetDataOfType(0ul, Clipboard::MIME_TYPE_TEXT, &data));
EXPECT_FALSE(GetDataOfType(mojom::kMimeTypeText, &data));

SetStringText(kPlainTextData);
EXPECT_EQ(1ul, GetSequenceNumber());

EXPECT_TRUE(GetDataOfType(1ul, Clipboard::MIME_TYPE_TEXT, &data));
EXPECT_TRUE(GetDataOfType(mojom::kMimeTypeText, &data));
EXPECT_EQ(kPlainTextData, data);
}

TEST_F(ClipboardAppTest, CanSetMultipleDataTypesAtOnce) {
Map<String, Array<uint8_t>> mime_data;
mime_data[Clipboard::MIME_TYPE_TEXT] =
mime_data[mojom::kMimeTypeText] =
Array<uint8_t>::From(std::string(kPlainTextData));
mime_data[Clipboard::MIME_TYPE_HTML] =
mime_data[mojom::kMimeTypeHTML] =
Array<uint8_t>::From(std::string(kHtmlData));

uint64_t sequence_num = 0;
Expand All @@ -118,9 +117,9 @@ TEST_F(ClipboardAppTest, CanSetMultipleDataTypesAtOnce) {
EXPECT_EQ(1ul, sequence_num);

std::string data;
EXPECT_TRUE(GetDataOfType(1ul, Clipboard::MIME_TYPE_TEXT, &data));
EXPECT_TRUE(GetDataOfType(mojom::kMimeTypeText, &data));
EXPECT_EQ(kPlainTextData, data);
EXPECT_TRUE(GetDataOfType(1ul, Clipboard::MIME_TYPE_HTML, &data));
EXPECT_TRUE(GetDataOfType(mojom::kMimeTypeHTML, &data));
EXPECT_EQ(kHtmlData, data);
}

Expand All @@ -129,7 +128,7 @@ TEST_F(ClipboardAppTest, CanClearClipboardWithZeroArray) {
SetStringText(kPlainTextData);
EXPECT_EQ(1ul, GetSequenceNumber());

EXPECT_TRUE(GetDataOfType(1ul, Clipboard::MIME_TYPE_TEXT, &data));
EXPECT_TRUE(GetDataOfType(mojom::kMimeTypeText, &data));
EXPECT_EQ(kPlainTextData, data);

Map<String, Array<uint8_t>> mime_data;
Expand All @@ -139,7 +138,7 @@ TEST_F(ClipboardAppTest, CanClearClipboardWithZeroArray) {
&sequence_num);

EXPECT_EQ(2ul, sequence_num);
EXPECT_FALSE(GetDataOfType(2ul, Clipboard::MIME_TYPE_TEXT, &data));
EXPECT_FALSE(GetDataOfType(mojom::kMimeTypeText, &data));
}

} // namespace clipboard
Expand Down
22 changes: 11 additions & 11 deletions components/mus/public/interfaces/clipboard.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,21 @@

module mus.mojom;

const string kMimeTypeHTML = "text/html";
const string kMimeTypeMozillaURL = "text/x-moz-url";
const string kMimeTypePNG = "image/png";
const string kMimeTypeRTF = "text/rtf";
const string kMimeTypeText = "text/plain";
const string kMimeTypeURIList = "text/uri-list";
const string kMimeTypeURL = "text/url";

interface Clipboard {
enum Type {
COPY_PASTE = 0,
SELECTION = 1,
DRAG = 2
};

// Mime type constants
const string MIME_TYPE_TEXT = "text/plain";
const string MIME_TYPE_HTML = "text/html";
const string MIME_TYPE_URL = "text/url";

// Returns a sequence number which uniquely identifies clipboard state.
// Clients are able to assume that the clipboard contents are unchanged as
// long as this number has not changed. This number is monotonically
Expand All @@ -29,17 +32,14 @@ interface Clipboard {
GetAvailableMimeTypes(Type clipboard_types)
=> (uint64 sequence, array<string> types);

// If the current sequence number is still |sequence|, returns true and the
// current data associated with the requested Mime type. When the sequence
// number has changed (because new data has been written to the clipboard),
// returns false and null for |data|.
// Returns the current data associated with the requested Mime type.
//
// We don't want to provide one API to return the entire clipboard state
// because the combined size of the clipboard can be megabytes, especially
// when image data is involved.
[Sync]
ReadMimeType(uint64 sequence, Type clipboard_type, string mime_type)
=> (bool sequence_valid, array<uint8>? data);
ReadClipboardData(Type clipboard_type, string mime_type)
=> (uint64 sequence, array<uint8>? data);

// Writes a set of mime types to the clipboard. This will increment the
// sequence number and return that. In the case of an empty or null map,
Expand Down
8 changes: 8 additions & 0 deletions mojo/common/common_type_converters.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ Array<uint8_t> TypeConverter<Array<uint8_t>, std::string>::Convert(
return result;
}

Array<uint8_t> TypeConverter<Array<uint8_t>, base::StringPiece>::Convert(
const base::StringPiece& input) {
Array<uint8_t> result(input.size());
if (!input.empty())
memcpy(&result.front(), input.data(), input.size());
return result;
}

base::string16 TypeConverter<base::string16, Array<uint8_t>>::Convert(
const Array<uint8_t>& input) {
if (input.is_null() || input.empty())
Expand Down
5 changes: 5 additions & 0 deletions mojo/common/common_type_converters.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ struct MOJO_COMMON_EXPORT TypeConverter<Array<uint8_t>, std::string> {
static Array<uint8_t> Convert(const std::string& input);
};

template <>
struct MOJO_COMMON_EXPORT TypeConverter<Array<uint8_t>, base::StringPiece> {
static Array<uint8_t> Convert(const base::StringPiece& input);
};

template <>
struct MOJO_COMMON_EXPORT TypeConverter<base::string16, Array<uint8_t>> {
static base::string16 Convert(const Array<uint8_t>& input);
Expand Down
1 change: 0 additions & 1 deletion ui/base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ component("base") {
"clipboard/clipboard_util_win.h",
"clipboard/clipboard_win.cc",
"clipboard/clipboard_win.h",
"clipboard/custom_data_helper_linux.cc",
"clipboard/custom_data_helper_mac.mm",
"cocoa/animation_utils.h",
"cocoa/appkit_utils.h",
Expand Down
60 changes: 40 additions & 20 deletions ui/base/clipboard/clipboard.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <memory>

#include "base/logging.h"
#include "base/memory/ptr_util.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/gfx/geometry/size.h"

Expand All @@ -32,32 +33,32 @@ void Clipboard::SetAllowedThreads(
}

// static
Clipboard* Clipboard::GetForCurrentThread() {
void Clipboard::SetClipboardForCurrentThread(
std::unique_ptr<Clipboard> platform_clipboard) {
base::AutoLock lock(clipboard_map_lock_.Get());
base::PlatformThreadId id = Clipboard::GetAndValidateThreadID();

base::PlatformThreadId id = base::PlatformThread::CurrentId();

AllowedThreadsVector* allowed_threads = allowed_threads_.Pointer();
if (!allowed_threads->empty()) {
bool found = false;
for (AllowedThreadsVector::const_iterator it = allowed_threads->begin();
it != allowed_threads->end(); ++it) {
if (*it == id) {
found = true;
break;
}
}

DCHECK(found);
ClipboardMap* clipboard_map = clipboard_map_.Pointer();
ClipboardMap::const_iterator it = clipboard_map->find(id);
if (it != clipboard_map->end()) {
// This shouldn't happen. The clipboard should not already exist.
NOTREACHED();
}
clipboard_map->insert(std::make_pair(id, std::move(platform_clipboard)));
}

// static
Clipboard* Clipboard::GetForCurrentThread() {
base::AutoLock lock(clipboard_map_lock_.Get());
base::PlatformThreadId id = GetAndValidateThreadID();

ClipboardMap* clipboard_map = clipboard_map_.Pointer();
ClipboardMap::const_iterator it = clipboard_map->find(id);
if (it != clipboard_map->end())
return it->second;
return it->second.get();

Clipboard* clipboard = Clipboard::Create();
clipboard_map->insert(std::make_pair(id, clipboard));
clipboard_map->insert(std::make_pair(id, base::WrapUnique(clipboard)));
return clipboard;
}

Expand All @@ -67,10 +68,8 @@ void Clipboard::DestroyClipboardForCurrentThread() {
ClipboardMap* clipboard_map = clipboard_map_.Pointer();
base::PlatformThreadId id = base::PlatformThread::CurrentId();
ClipboardMap::iterator it = clipboard_map->find(id);
if (it != clipboard_map->end()) {
delete it->second;
if (it != clipboard_map->end())
clipboard_map->erase(it);
}
}

void Clipboard::DispatchObject(ObjectType type, const ObjectMapParams& params) {
Expand Down Expand Up @@ -132,4 +131,25 @@ void Clipboard::DispatchObject(ObjectType type, const ObjectMapParams& params) {
}
}

base::PlatformThreadId Clipboard::GetAndValidateThreadID() {
base::PlatformThreadId id = base::PlatformThread::CurrentId();
#ifndef NDEBUG
AllowedThreadsVector* allowed_threads = allowed_threads_.Pointer();
if (!allowed_threads->empty()) {
bool found = false;
for (AllowedThreadsVector::const_iterator it = allowed_threads->begin();
it != allowed_threads->end(); ++it) {
if (*it == id) {
found = true;
break;
}
}

DCHECK(found);
}
#endif

return id;
}

} // namespace ui
Loading

0 comments on commit e163bd7

Please sign in to comment.