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}

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) {

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_);

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) {

mojo/services/clipboard/clipboard_standalone_impl.cc

-// 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

mojo/services/clipboard/clipboard_standalone_impl.h

-// 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];

mojo/services/clipboard/clipboard_standalone_unittest.cc

-// 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());
 

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());
 }

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