diff options
author | erg <erg@chromium.org> | 2014-10-16 11:45:11 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-10-16 18:45:31 +0000 |
commit | 00a8f30e140e0e88a23fb1f14bfa74680cb4bd54 (patch) | |
tree | 969fe45524a875b8108b3750ff72d98c2d31bd81 /mojo | |
parent | da4b9e9cff5906d104aaf92379528eec91c16d15 (diff) | |
download | chromium_src-00a8f30e140e0e88a23fb1f14bfa74680cb4bd54.zip chromium_src-00a8f30e140e0e88a23fb1f14bfa74680cb4bd54.tar.gz chromium_src-00a8f30e140e0e88a23fb1f14bfa74680cb4bd54.tar.bz2 |
mojo: Switch the clipboard interface over to using map<>.
BUG=413863
Review URL: https://codereview.chromium.org/646773005
Cr-Commit-Position: refs/heads/master@{#299942}
Diffstat (limited to 'mojo')
-rw-r--r-- | mojo/public/cpp/bindings/lib/map_internal.h | 26 | ||||
-rw-r--r-- | mojo/public/cpp/bindings/map.h | 5 | ||||
-rw-r--r-- | mojo/public/cpp/bindings/tests/map_unittest.cc | 71 | ||||
-rw-r--r-- | mojo/services/clipboard/clipboard_standalone_impl.cc | 63 | ||||
-rw-r--r-- | mojo/services/clipboard/clipboard_standalone_impl.h | 7 | ||||
-rw-r--r-- | mojo/services/clipboard/clipboard_standalone_unittest.cc | 42 | ||||
-rw-r--r-- | mojo/services/html_viewer/webclipboard_impl.cc | 36 | ||||
-rw-r--r-- | mojo/services/public/interfaces/clipboard/clipboard.mojom | 11 |
8 files changed, 142 insertions, 119 deletions
diff --git a/mojo/public/cpp/bindings/lib/map_internal.h b/mojo/public/cpp/bindings/lib/map_internal.h index aa0fe7e..f006cd0 100644 --- a/mojo/public/cpp/bindings/lib/map_internal.h +++ b/mojo/public/cpp/bindings/lib/map_internal.h @@ -57,7 +57,7 @@ struct MapTraits<Key, Value, false> { static inline ValueRefType at(std::map<KeyStorageType, ValueStorageType>* m, KeyForwardType key) { // We don't have C++11 library support yet, so we have to emulate the crash - // on a non-existant key. + // on a non-existent key. auto it = m->find(key); MOJO_CHECK(it != m->end()); return it->second; @@ -66,11 +66,17 @@ struct MapTraits<Key, Value, false> { const std::map<KeyStorageType, ValueStorageType>* m, KeyForwardType key) { // We don't have C++11 library support yet, so we have to emulate the crash - // on a non-existant key. + // on a non-existent key. auto it = m->find(key); MOJO_CHECK(it != m->end()); return it->second; } + static inline ValueRefType GetOrInsert( + std::map<KeyStorageType, ValueStorageType>* m, + KeyForwardType key) { + // This is the backing for the index operator (operator[]). + return (*m)[key]; + } static inline void Insert(std::map<KeyStorageType, ValueStorageType>* m, KeyForwardType key, ValueForwardType value) { @@ -146,7 +152,7 @@ struct MapTraits<Key, Value, true> { static inline ValueRefType at(std::map<KeyStorageType, ValueStorageType>* m, KeyForwardType key) { // We don't have C++11 library support yet, so we have to emulate the crash - // on a non-existant key. + // on a non-existent key. auto it = m->find(key); MOJO_CHECK(it != m->end()); return GetValue(it); @@ -155,11 +161,23 @@ struct MapTraits<Key, Value, true> { const std::map<KeyStorageType, ValueStorageType>* m, KeyForwardType key) { // We don't have C++11 library support yet, so we have to emulate the crash - // on a non-existant key. + // on a non-existent key. auto it = m->find(key); MOJO_CHECK(it != m->end()); return GetValue(it); } + static inline ValueRefType GetOrInsert( + std::map<KeyStorageType, ValueStorageType>* m, + KeyForwardType key) { + // This is the backing for the index operator (operator[]). + auto it = m->find(key); + if (it == m->end()) { + it = m->insert(std::make_pair(key, ValueStorageType())).first; + new (it->second.buf) Value(); + } + + return GetValue(it); + } static inline void Insert(std::map<KeyStorageType, ValueStorageType>* m, KeyForwardType key, ValueRefType value) { diff --git a/mojo/public/cpp/bindings/map.h b/mojo/public/cpp/bindings/map.h index f09937a..7aa7b6f 100644 --- a/mojo/public/cpp/bindings/map.h +++ b/mojo/public/cpp/bindings/map.h @@ -88,6 +88,11 @@ class Map { return Traits::at(&map_, key); } + ValueRefType operator[](KeyForwardType key) { + is_null_ = false; + return Traits::GetOrInsert(&map_, key); + } + void Swap(Map<Key, Value>* other) { std::swap(is_null_, other->is_null_); map_.swap(other->map_); diff --git a/mojo/public/cpp/bindings/tests/map_unittest.cc b/mojo/public/cpp/bindings/tests/map_unittest.cc index 159afb9..6f7db16 100644 --- a/mojo/public/cpp/bindings/tests/map_unittest.cc +++ b/mojo/public/cpp/bindings/tests/map_unittest.cc @@ -50,6 +50,49 @@ TEST_F(MapTest, InsertWorks) { } } +TEST_F(MapTest, TestIndexOperator) { + Map<String, int> map; + for (size_t i = 0; i < kStringIntDataSize; ++i) + map[kStringIntData[i].string_data] = kStringIntData[i].int_data; + + for (size_t i = 0; i < kStringIntDataSize; ++i) { + EXPECT_EQ(kStringIntData[i].int_data, + map.at(kStringIntData[i].string_data)); + } +} + +TEST_F(MapTest, TestIndexOperatorAsRValue) { + Map<String, int> map; + for (size_t i = 0; i < kStringIntDataSize; ++i) + map.insert(kStringIntData[i].string_data, kStringIntData[i].int_data); + + for (size_t i = 0; i < kStringIntDataSize; ++i) { + EXPECT_EQ(kStringIntData[i].int_data, map[kStringIntData[i].string_data]); + } +} + +TEST_F(MapTest, TestIndexOperatorMoveOnly) { + ASSERT_EQ(0u, MoveOnlyType::num_instances()); + mojo::Map<mojo::String, mojo::Array<int32_t>> map; + std::vector<MoveOnlyType*> value_ptrs; + + for (size_t i = 0; i < kStringIntDataSize; ++i) { + const char* key = kStringIntData[i].string_data; + Array<int32_t> array(1); + array[0] = kStringIntData[i].int_data; + map[key] = array.Pass(); + EXPECT_TRUE(map); + } + + // We now read back that data, to test the behavior of operator[]. + for (size_t i = 0; i < kStringIntDataSize; ++i) { + auto it = map.find(kStringIntData[i].string_data); + ASSERT_TRUE(it != map.end()); + ASSERT_EQ(1u, it.GetValue().size()); + EXPECT_EQ(kStringIntData[i].int_data, it.GetValue()[0]); + } +} + TEST_F(MapTest, ConstructedFromArray) { Array<String> keys(kStringIntDataSize); Array<int> values(kStringIntDataSize); @@ -153,7 +196,33 @@ TEST_F(MapTest, Insert_MoveOnly) { // a lot more boring. map.reset(); - EXPECT_EQ(0u, CopyableType::num_instances()); + EXPECT_EQ(0u, MoveOnlyType::num_instances()); +} + +TEST_F(MapTest, IndexOperator_MoveOnly) { + ASSERT_EQ(0u, MoveOnlyType::num_instances()); + mojo::Map<mojo::String, MoveOnlyType> map; + std::vector<MoveOnlyType*> value_ptrs; + + for (size_t i = 0; i < kStringIntDataSize; ++i) { + const char* key = kStringIntData[i].string_data; + MoveOnlyType value; + value_ptrs.push_back(value.ptr()); + map[key] = value.Pass(); + ASSERT_EQ(i + 1, map.size()); + ASSERT_EQ(i + 1, value_ptrs.size()); + EXPECT_EQ(map.size() + 1, MoveOnlyType::num_instances()); + EXPECT_TRUE(map.at(key).moved()); + EXPECT_EQ(value_ptrs[i], map.at(key).ptr()); + map.at(key).ResetMoved(); + EXPECT_TRUE(map); + } + + // std::map doesn't have a capacity() method like std::vector so this test is + // a lot more boring. + + map.reset(); + EXPECT_EQ(0u, MoveOnlyType::num_instances()); } TEST_F(MapTest, STLToMojo) { diff --git a/mojo/services/clipboard/clipboard_standalone_impl.cc b/mojo/services/clipboard/clipboard_standalone_impl.cc index e1c7e2c..404eded 100644 --- a/mojo/services/clipboard/clipboard_standalone_impl.cc +++ b/mojo/services/clipboard/clipboard_standalone_impl.cc @@ -1,12 +1,12 @@ -// Copyright (c) 2014 The Chromium Authors. All rights reserved. +// Copyright 2014 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 "mojo/services/clipboard/clipboard_standalone_impl.h" -namespace mojo { +#include <string.h> -typedef std::vector<uint8_t> ByteVector; +namespace mojo { // ClipboardData contains data copied to the Clipboard for a variety of formats. // It mostly just provides APIs to cleanly access and manipulate this data. @@ -15,35 +15,25 @@ class ClipboardStandaloneImpl::ClipboardData { ClipboardData() {} ~ClipboardData() {} - std::vector<std::string> GetMimeTypes() const { - std::vector<std::string> types; - for (std::map<std::string, ByteVector>::const_iterator it = - data_types_.begin(); - it != data_types_.end(); - ++it) { - types.push_back(it->first); - } - - return types; - } + Array<String> GetMimeTypes() const { + Array<String> types(data_types_.size()); + int i = 0; + for (auto it = data_types_.begin(); it != data_types_.end(); ++it, ++i) + types[i] = it.GetKey(); - void SetData(std::map<std::string, ByteVector>* data) { - std::swap(data_types_, *data); + return types.Pass(); } - bool GetData(const std::string& mime_type, ByteVector* data) const { - std::map<std::string, ByteVector>::const_iterator it = - data_types_.find(mime_type); - if (it != data_types_.end()) { - *data = it->second; - return true; - } + void SetData(Map<String, Array<uint8_t>> data) { data_types_ = data.Pass(); } - return false; + void GetData(const String& mime_type, mojo::Array<uint8_t>* data) const { + auto it = data_types_.find(mime_type); + if (it != data_types_.end()) + *data = it.GetValue().Clone(); } private: - std::map<std::string, ByteVector> data_types_; + Map<String, Array<uint8_t>> data_types_; DISALLOW_COPY_AND_ASSIGN(ClipboardData); }; @@ -67,34 +57,23 @@ void ClipboardStandaloneImpl::GetSequenceNumber( void ClipboardStandaloneImpl::GetAvailableMimeTypes( Clipboard::Type clipboard_type, const mojo::Callback<void(mojo::Array<mojo::String>)>& callback) { - mojo::Array<mojo::String> types = mojo::Array<mojo::String>::From( - clipboard_state_[clipboard_type]->GetMimeTypes()); - callback.Run(types.Pass()); + callback.Run(clipboard_state_[clipboard_type]->GetMimeTypes().Pass()); } void ClipboardStandaloneImpl::ReadMimeType( Clipboard::Type clipboard_type, const mojo::String& mime_type, const mojo::Callback<void(mojo::Array<uint8_t>)>& callback) { - ByteVector mime_data; - if (clipboard_state_[clipboard_type]->GetData( - mime_type.To<std::string>(), &mime_data)) { - callback.Run(mojo::Array<uint8_t>::From(mime_data).Pass()); - return; - } - - callback.Run(mojo::Array<uint8_t>().Pass()); + mojo::Array<uint8_t> mime_data; + clipboard_state_[clipboard_type]->GetData(mime_type, &mime_data); + callback.Run(mime_data.Pass()); } void ClipboardStandaloneImpl::WriteClipboardData( Clipboard::Type clipboard_type, - mojo::Array<MimeTypePairPtr> data) { - std::map<std::string, ByteVector> mime_data; - for (size_t i = 0; i < data.size(); ++i) - mime_data[data[i]->mime_type] = data[i]->data; - + mojo::Map<mojo::String, mojo::Array<uint8_t>> data) { sequence_number_[clipboard_type]++; - clipboard_state_[clipboard_type]->SetData(&mime_data); + clipboard_state_[clipboard_type]->SetData(data.Pass()); } } // namespace mojo diff --git a/mojo/services/clipboard/clipboard_standalone_impl.h b/mojo/services/clipboard/clipboard_standalone_impl.h index 09d7424..ccb8c54 100644 --- a/mojo/services/clipboard/clipboard_standalone_impl.h +++ b/mojo/services/clipboard/clipboard_standalone_impl.h @@ -1,4 +1,4 @@ -// Copyright (c) 2014 The Chromium Authors. All rights reserved. +// Copyright 2014 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. @@ -38,8 +38,9 @@ class ClipboardStandaloneImpl : public InterfaceImpl<mojo::Clipboard> { Clipboard::Type clipboard_type, const mojo::String& mime_type, const mojo::Callback<void(mojo::Array<uint8_t>)>& callback) override; - virtual void WriteClipboardData(Clipboard::Type clipboard_type, - mojo::Array<MimeTypePairPtr> data) override; + virtual void WriteClipboardData( + Clipboard::Type clipboard_type, + mojo::Map<mojo::String, mojo::Array<uint8_t>> data) override; private: uint64_t sequence_number_[kNumClipboards]; diff --git a/mojo/services/clipboard/clipboard_standalone_unittest.cc b/mojo/services/clipboard/clipboard_standalone_unittest.cc index c297b94..cdf0087 100644 --- a/mojo/services/clipboard/clipboard_standalone_unittest.cc +++ b/mojo/services/clipboard/clipboard_standalone_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2014 The Chromium Authors. All rights reserved. +// Copyright 2014 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. @@ -89,11 +89,8 @@ class ClipboardStandaloneTest : public testing::Test { } void SetStringText(const std::string& data) { - Array<MimeTypePairPtr> mime_data; - MimeTypePairPtr text_data(MimeTypePair::New()); - text_data->mime_type = mojo::Clipboard::MIME_TYPE_TEXT; - text_data->data = Array<uint8_t>::From(data).Pass(); - mime_data.push_back(text_data.Pass()); + Map<String, Array<uint8_t>> mime_data; + mime_data[mojo::Clipboard::MIME_TYPE_TEXT] = Array<uint8_t>::From(data); clipboard_->WriteClipboardData(mojo::Clipboard::TYPE_COPY_PASTE, mime_data.Pass()); } @@ -127,16 +124,11 @@ TEST_F(ClipboardStandaloneTest, CanReadBackText) { } TEST_F(ClipboardStandaloneTest, CanSetMultipleDataTypesAtOnce) { - Array<MimeTypePairPtr> mime_data; - MimeTypePairPtr text_data(MimeTypePair::New()); - text_data->mime_type = mojo::Clipboard::MIME_TYPE_TEXT; - text_data->data = Array<uint8_t>::From(std::string(kPlainTextData)).Pass(); - mime_data.push_back(text_data.Pass()); - - MimeTypePairPtr html_data(MimeTypePair::New()); - html_data->mime_type = mojo::Clipboard::MIME_TYPE_HTML; - html_data->data = Array<uint8_t>::From(std::string(kHtmlData)).Pass(); - mime_data.push_back(html_data.Pass()); + Map<String, Array<uint8_t>> mime_data; + mime_data[mojo::Clipboard::MIME_TYPE_TEXT] = + Array<uint8_t>::From(std::string(kPlainTextData)); + mime_data[mojo::Clipboard::MIME_TYPE_HTML] = + Array<uint8_t>::From(std::string(kHtmlData)); clipboard_->WriteClipboardData(mojo::Clipboard::TYPE_COPY_PASTE, mime_data.Pass()); @@ -150,22 +142,6 @@ TEST_F(ClipboardStandaloneTest, CanSetMultipleDataTypesAtOnce) { EXPECT_EQ(kHtmlData, data); } -TEST_F(ClipboardStandaloneTest, CanClearClipboardWithNull) { - std::string data; - SetStringText(kPlainTextData); - EXPECT_EQ(1ul, GetSequenceNumber()); - - EXPECT_TRUE(GetDataOfType(mojo::Clipboard::MIME_TYPE_TEXT, &data)); - EXPECT_EQ(kPlainTextData, data); - - Array<MimeTypePairPtr> mime_data; - clipboard_->WriteClipboardData(mojo::Clipboard::TYPE_COPY_PASTE, - mime_data.Pass()); - - EXPECT_EQ(2ul, GetSequenceNumber()); - EXPECT_FALSE(GetDataOfType(mojo::Clipboard::MIME_TYPE_TEXT, &data)); -} - TEST_F(ClipboardStandaloneTest, CanClearClipboardWithZeroArray) { std::string data; SetStringText(kPlainTextData); @@ -174,7 +150,7 @@ TEST_F(ClipboardStandaloneTest, CanClearClipboardWithZeroArray) { EXPECT_TRUE(GetDataOfType(mojo::Clipboard::MIME_TYPE_TEXT, &data)); EXPECT_EQ(kPlainTextData, data); - Array<MimeTypePairPtr> mime_data(0); + Map<String, Array<uint8_t>> mime_data; clipboard_->WriteClipboardData(mojo::Clipboard::TYPE_COPY_PASTE, mime_data.Pass()); diff --git a/mojo/services/html_viewer/webclipboard_impl.cc b/mojo/services/html_viewer/webclipboard_impl.cc index 94b16ca..9b7ebff 100644 --- a/mojo/services/html_viewer/webclipboard_impl.cc +++ b/mojo/services/html_viewer/webclipboard_impl.cc @@ -168,11 +168,8 @@ blink::WebString WebClipboardImpl::readCustomData( } void WebClipboardImpl::writePlainText(const blink::WebString& text) { - Array<MimeTypePairPtr> data; - MimeTypePairPtr text_data(MimeTypePair::New()); - text_data->mime_type = mojo::Clipboard::MIME_TYPE_TEXT; - text_data->data = Array<uint8_t>::From(text).Pass(); - data.push_back(text_data.Pass()); + Map<String, Array<uint8_t>> data; + data[mojo::Clipboard::MIME_TYPE_TEXT] = Array<uint8_t>::From(text); clipboard_->WriteClipboardData(mojo::Clipboard::TYPE_COPY_PASTE, data.Pass()); } @@ -181,28 +178,13 @@ void WebClipboardImpl::writeHTML(const blink::WebString& htmlText, const blink::WebURL& url, const blink::WebString& plainText, bool writeSmartPaste) { - Array<MimeTypePairPtr> data; - MimeTypePairPtr text_data(MimeTypePair::New()); - text_data->mime_type = mojo::Clipboard::MIME_TYPE_TEXT; - text_data->data = Array<uint8_t>::From(plainText).Pass(); - data.push_back(text_data.Pass()); - - MimeTypePairPtr html_data(MimeTypePair::New()); - text_data->mime_type = mojo::Clipboard::MIME_TYPE_HTML; - text_data->data = Array<uint8_t>::From(htmlText).Pass(); - data.push_back(html_data.Pass()); - - MimeTypePairPtr url_data(MimeTypePair::New()); - url_data->mime_type = mojo::Clipboard::MIME_TYPE_URL; - url_data->data = Array<uint8_t>::From(url.string()).Pass(); - data.push_back(url_data.Pass()); - - if (writeSmartPaste) { - MimeTypePairPtr smart_paste(MimeTypePair::New()); - url_data->mime_type = kMimeTypeWebkitSmartPaste; - url_data->data = Array<uint8_t>::From(blink::WebString()).Pass(); - data.push_back(smart_paste.Pass()); - } + Map<String, Array<uint8_t>> data; + data[mojo::Clipboard::MIME_TYPE_TEXT] = Array<uint8_t>::From(plainText); + data[mojo::Clipboard::MIME_TYPE_HTML] = Array<uint8_t>::From(htmlText); + data[mojo::Clipboard::MIME_TYPE_URL] = Array<uint8_t>::From(url.string()); + + if (writeSmartPaste) + data[kMimeTypeWebkitSmartPaste] = Array<uint8_t>::From(blink::WebString()); clipboard_->WriteClipboardData(mojo::Clipboard::TYPE_COPY_PASTE, data.Pass()); } diff --git a/mojo/services/public/interfaces/clipboard/clipboard.mojom b/mojo/services/public/interfaces/clipboard/clipboard.mojom index eca47b9..92ef822 100644 --- a/mojo/services/public/interfaces/clipboard/clipboard.mojom +++ b/mojo/services/public/interfaces/clipboard/clipboard.mojom @@ -4,13 +4,6 @@ module mojo { -// A wrapper type which is just a Key/Value pair. Workaround until we get -// proper maps in mojom. -struct MimeTypePair { - string mime_type; - array<uint8> data; -}; - interface Clipboard { enum Type { COPY_PASTE = 0, @@ -44,9 +37,9 @@ interface Clipboard { ReadMimeType(Type clipboard_type, string mime_type) => (array<uint8>? data); // Writes a set of mime types to the clipboard. This will increment the - // sequence number. In the case of an empty or NULL list, this will just + // sequence number. In the case of an empty or null map, this will just // clear the clipboard. - WriteClipboardData(Type clipboard_type, array<MimeTypePair>? data); + WriteClipboardData(Type clipboard_type, map<string, array<uint8>>? data); }; } // module mojo |