[mojo] Reduce number of copies in AX mojom traits.

- Add MapTraits to allow serialization of a vector of pairs directly
  into a Mojo map for ui::AXNodeData.
- Update AX browsertest to use Gmock matchers for container comparison.
  This test currently depends on an ordering that is not guaranteed by
  abnything and may unexpectedly break in the future.  Using Gmock makes
  it easier to update at such a point in the future.
- Deserialize attribute maps into a base::flat_map and extract the
  underlying result directly into ui::AXNodeData.

Bug: none
Change-Id: I1db2649312ad1e68a40962e27bd5c97da29cc3fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2541745Reviewed-by: David Tseng <dtseng@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828966}

content/browser/accessibility/cross_platform_accessibility_browsertest.cc

@@ -28,6 +28,7 @@
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/shell/browser/shell.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "third_party/blink/public/common/features.h"
 #include "ui/accessibility/accessibility_features.h"
 #include "ui/accessibility/accessibility_switches.h"
@@ -40,6 +41,9 @@
 #include "ui/base/win/atl_module.h"
 #endif
 
+using ::testing::ElementsAre;
+using ::testing::Pair;
+
 #if defined(NDEBUG) && !defined(ADDRESS_SANITIZER) &&              \
     !defined(LEAK_SANITIZER) && !defined(MEMORY_SANITIZER) &&      \
     !defined(THREAD_SANITIZER) && !defined(UNDEFINED_SANITIZER) && \
@@ -274,51 +278,42 @@ IN_PROC_BROWSER_TEST_F(CrossPlatformAccessibilityBrowserTest,
   const ui::AXNode* root = tree.root();
 
   // Check properties of thet tree.
-  EXPECT_STREQ(url_str, tree.data().url.c_str());
-  EXPECT_STREQ("Accessibility Test", tree.data().title.c_str());
-  EXPECT_STREQ("html", tree.data().doctype.c_str());
-  EXPECT_STREQ("text/html", tree.data().mimetype.c_str());
+  EXPECT_EQ(url_str, tree.data().url);
+  EXPECT_EQ("Accessibility Test", tree.data().title);
+  EXPECT_EQ("html", tree.data().doctype);
+  EXPECT_EQ("text/html", tree.data().mimetype);
 
   // Check properties of the root element of the tree.
-  EXPECT_STREQ("Accessibility Test",
-               GetAttr(root, ax::mojom::StringAttribute::kName).c_str());
+  EXPECT_EQ("Accessibility Test",
+            GetAttr(root, ax::mojom::StringAttribute::kName));
   EXPECT_EQ(ax::mojom::Role::kRootWebArea, root->data().role);
 
   // Check properties of the BODY element.
   ASSERT_EQ(1u, root->GetUnignoredChildCount());
   const ui::AXNode* body = root->GetUnignoredChildAtIndex(0);
   EXPECT_EQ(ax::mojom::Role::kGenericContainer, body->data().role);
-  EXPECT_STREQ("body",
-               GetAttr(body, ax::mojom::StringAttribute::kHtmlTag).c_str());
-  EXPECT_STREQ("block",
-               GetAttr(body, ax::mojom::StringAttribute::kDisplay).c_str());
+  EXPECT_EQ("body", GetAttr(body, ax::mojom::StringAttribute::kHtmlTag));
+  EXPECT_EQ("block", GetAttr(body, ax::mojom::StringAttribute::kDisplay));
 
   // Check properties of the two children of the BODY element.
   ASSERT_EQ(2u, body->GetUnignoredChildCount());
 
   const ui::AXNode* button = body->GetUnignoredChildAtIndex(0);
   EXPECT_EQ(ax::mojom::Role::kButton, button->data().role);
-  EXPECT_STREQ("input",
-               GetAttr(button, ax::mojom::StringAttribute::kHtmlTag).c_str());
-  EXPECT_STREQ("push",
-               GetAttr(button, ax::mojom::StringAttribute::kName).c_str());
-  EXPECT_STREQ("inline-block",
-               GetAttr(button, ax::mojom::StringAttribute::kDisplay).c_str());
-  ASSERT_EQ(2U, button->data().html_attributes.size());
-  EXPECT_STREQ("type", button->data().html_attributes[0].first.c_str());
-  EXPECT_STREQ("button", button->data().html_attributes[0].second.c_str());
-  EXPECT_STREQ("value", button->data().html_attributes[1].first.c_str());
-  EXPECT_STREQ("push", button->data().html_attributes[1].second.c_str());
+  EXPECT_EQ("input", GetAttr(button, ax::mojom::StringAttribute::kHtmlTag));
+  EXPECT_EQ("push", GetAttr(button, ax::mojom::StringAttribute::kName));
+  EXPECT_EQ("inline-block",
+            GetAttr(button, ax::mojom::StringAttribute::kDisplay));
+  EXPECT_THAT(button->data().html_attributes,
+              ElementsAre(Pair("type", "button"), Pair("value", "push")));
 
   const ui::AXNode* checkbox = body->GetUnignoredChildAtIndex(1);
   EXPECT_EQ(ax::mojom::Role::kCheckBox, checkbox->data().role);
-  EXPECT_STREQ("input",
-               GetAttr(checkbox, ax::mojom::StringAttribute::kHtmlTag).c_str());
-  EXPECT_STREQ("inline-block",
-               GetAttr(checkbox, ax::mojom::StringAttribute::kDisplay).c_str());
-  ASSERT_EQ(1U, checkbox->data().html_attributes.size());
-  EXPECT_STREQ("type", checkbox->data().html_attributes[0].first.c_str());
-  EXPECT_STREQ("checkbox", checkbox->data().html_attributes[0].second.c_str());
+  EXPECT_EQ("input", GetAttr(checkbox, ax::mojom::StringAttribute::kHtmlTag));
+  EXPECT_EQ("inline-block",
+            GetAttr(checkbox, ax::mojom::StringAttribute::kDisplay));
+  EXPECT_THAT(checkbox->data().html_attributes,
+              ElementsAre(Pair("type", "checkbox")));
 }
 
 IN_PROC_BROWSER_TEST_F(CrossPlatformAccessibilityBrowserTest,

mojo/public/cpp/bindings/map_traits_stl.h

@@ -110,6 +110,37 @@ struct MapTraits<std::unordered_map<K, V>> {
   static void SetToEmpty(std::unordered_map<K, V>* output) { output->clear(); }
 };
 
+// Note: this is only used for serialization.
+template <typename K, typename V>
+struct MapTraits<std::vector<std::pair<K, V>>> {
+  using Key = K;
+  using Value = V;
+  using Container = std::vector<std::pair<K, V>>;
+  using ConstIterator = typename Container::const_iterator;
+
+  static bool IsNull(const Container& input) {
+    // std::vector<> has no built-in concept of nullness.
+    // TODO(dcheng): Why are we even calling this?
+    return false;
+  }
+
+  static size_t GetSize(const Container& input) { return input.size(); }
+
+  static ConstIterator GetBegin(const Container& input) {
+    return input.begin();
+  }
+
+  static void AdvanceIterator(ConstIterator& iterator) { iterator++; }
+
+  static const K& GetKey(const ConstIterator& iterator) {
+    return iterator->first;
+  }
+
+  static const V& GetValue(const ConstIterator& iterator) {
+    return iterator->second;
+  }
+};
+
 }  // namespace mojo
 
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_MAP_TRAITS_STL_H_

ui/accessibility/mojom/ax_node_data_mojom_traits.cc

@@ -3,82 +3,13 @@
 // found in the LICENSE file.
 
 #include "ui/accessibility/mojom/ax_node_data_mojom_traits.h"
+
+#include "base/containers/flat_map.h"
 #include "ui/accessibility/mojom/ax_relative_bounds.mojom-shared.h"
 #include "ui/accessibility/mojom/ax_relative_bounds_mojom_traits.h"
 
 namespace mojo {
 
-// static
-std::unordered_map<ax::mojom::StringAttribute, std::string>
-StructTraits<ax::mojom::AXNodeDataDataView, ui::AXNodeData>::string_attributes(
-    const ui::AXNodeData& p) {
-  std::unordered_map<ax::mojom::StringAttribute, std::string> result;
-  for (const auto& iter : p.string_attributes)
-    result[iter.first] = iter.second;
-  return result;
-}
-
-// static
-std::unordered_map<ax::mojom::IntAttribute, int32_t>
-StructTraits<ax::mojom::AXNodeDataDataView, ui::AXNodeData>::int_attributes(
-    const ui::AXNodeData& p) {
-  std::unordered_map<ax::mojom::IntAttribute, int32_t> result;
-  for (const auto& iter : p.int_attributes)
-    result[iter.first] = iter.second;
-  return result;
-}
-
-// static
-std::unordered_map<ax::mojom::FloatAttribute, float>
-StructTraits<ax::mojom::AXNodeDataDataView, ui::AXNodeData>::float_attributes(
-    const ui::AXNodeData& p) {
-  std::unordered_map<ax::mojom::FloatAttribute, float> result;
-  for (const auto& iter : p.float_attributes)
-    result[iter.first] = iter.second;
-  return result;
-}
-
-// static
-std::unordered_map<ax::mojom::BoolAttribute, bool>
-StructTraits<ax::mojom::AXNodeDataDataView, ui::AXNodeData>::bool_attributes(
-    const ui::AXNodeData& p) {
-  std::unordered_map<ax::mojom::BoolAttribute, bool> result;
-  for (const auto& iter : p.bool_attributes)
-    result[iter.first] = iter.second;
-  return result;
-}
-
-// static
-std::unordered_map<ax::mojom::IntListAttribute, std::vector<int32_t>>
-StructTraits<ax::mojom::AXNodeDataDataView, ui::AXNodeData>::intlist_attributes(
-    const ui::AXNodeData& p) {
-  std::unordered_map<ax::mojom::IntListAttribute, std::vector<int32_t>> result;
-  for (const auto& iter : p.intlist_attributes)
-    result[iter.first] = iter.second;
-  return result;
-}
-
-// static
-std::unordered_map<ax::mojom::StringListAttribute, std::vector<std::string>>
-StructTraits<ax::mojom::AXNodeDataDataView,
-             ui::AXNodeData>::stringlist_attributes(const ui::AXNodeData& p) {
-  std::unordered_map<ax::mojom::StringListAttribute, std::vector<std::string>>
-      result;
-  for (const auto& iter : p.stringlist_attributes)
-    result[iter.first] = iter.second;
-  return result;
-}
-
-// static
-std::unordered_map<std::string, std::string>
-StructTraits<ax::mojom::AXNodeDataDataView, ui::AXNodeData>::html_attributes(
-    const ui::AXNodeData& p) {
-  std::unordered_map<std::string, std::string> result;
-  for (const auto& iter : p.html_attributes)
-    result[iter.first] = iter.second;
-  return result;
-}
-
 // static
 bool StructTraits<ax::mojom::AXNodeDataDataView, ui::AXNodeData>::Read(
     ax::mojom::AXNodeDataDataView data,
@@ -88,49 +19,44 @@ bool StructTraits<ax::mojom::AXNodeDataDataView, ui::AXNodeData>::Read(
   out->state = data.state();
   out->actions = data.actions();
 
-  std::unordered_map<ax::mojom::StringAttribute, std::string> string_attributes;
+  // TODO(dcheng): AXNodeData should probably just switch over to absl's
+  // flat_hash_map for simplicity at some point.
+  base::flat_map<ax::mojom::StringAttribute, std::string> string_attributes;
   if (!data.ReadStringAttributes(&string_attributes))
     return false;
-  for (const auto& iter : string_attributes)
-    out->AddStringAttribute(iter.first, iter.second);
+  out->string_attributes = std::move(string_attributes).extract();
 
-  std::unordered_map<ax::mojom::IntAttribute, int32_t> int_attributes;
+  base::flat_map<ax::mojom::IntAttribute, int32_t> int_attributes;
   if (!data.ReadIntAttributes(&int_attributes))
     return false;
-  for (const auto& iter : int_attributes)
-    out->AddIntAttribute(iter.first, iter.second);
+  out->int_attributes = std::move(int_attributes).extract();
 
-  std::unordered_map<ax::mojom::FloatAttribute, float> float_attributes;
+  base::flat_map<ax::mojom::FloatAttribute, float> float_attributes;
   if (!data.ReadFloatAttributes(&float_attributes))
     return false;
-  for (const auto& iter : float_attributes)
-    out->AddFloatAttribute(iter.first, iter.second);
+  out->float_attributes = std::move(float_attributes).extract();
 
-  std::unordered_map<ax::mojom::BoolAttribute, bool> bool_attributes;
+  base::flat_map<ax::mojom::BoolAttribute, bool> bool_attributes;
   if (!data.ReadBoolAttributes(&bool_attributes))
     return false;
-  for (const auto& iter : bool_attributes)
-    out->AddBoolAttribute(iter.first, iter.second);
+  out->bool_attributes = std::move(bool_attributes).extract();
 
-  std::unordered_map<ax::mojom::IntListAttribute, std::vector<int32_t>>
+  base::flat_map<ax::mojom::IntListAttribute, std::vector<int32_t>>
       intlist_attributes;
   if (!data.ReadIntlistAttributes(&intlist_attributes))
     return false;
-  for (const auto& iter : intlist_attributes)
-    out->AddIntListAttribute(iter.first, iter.second);
+  out->intlist_attributes = std::move(intlist_attributes).extract();
 
-  std::unordered_map<ax::mojom::StringListAttribute, std::vector<std::string>>
+  base::flat_map<ax::mojom::StringListAttribute, std::vector<std::string>>
       stringlist_attributes;
   if (!data.ReadStringlistAttributes(&stringlist_attributes))
     return false;
-  for (const auto& iter : stringlist_attributes)
-    out->AddStringListAttribute(iter.first, iter.second);
+  out->stringlist_attributes = std::move(stringlist_attributes).extract();
 
-  std::unordered_map<std::string, std::string> html_attributes;
+  base::flat_map<std::string, std::string> html_attributes;
   if (!data.ReadHtmlAttributes(&html_attributes))
     return false;
-  for (const auto& iter : html_attributes)
-    out->html_attributes.push_back(std::make_pair(iter.first, iter.second));
+  out->html_attributes = std::move(html_attributes).extract();
 
   if (!data.ReadChildIds(&out->child_ids))
     return false;

ui/accessibility/mojom/ax_node_data_mojom_traits.h

@@ -17,22 +17,37 @@ struct StructTraits<ax::mojom::AXNodeDataDataView, ui::AXNodeData> {
   static ax::mojom::Role role(const ui::AXNodeData& p) { return p.role; }
   static uint32_t state(const ui::AXNodeData& p) { return p.state; }
   static uint64_t actions(const ui::AXNodeData& p) { return p.actions; }
-  static std::unordered_map<ax::mojom::StringAttribute, std::string>
-  string_attributes(const ui::AXNodeData& p);
-  static std::unordered_map<ax::mojom::IntAttribute, int32_t> int_attributes(
-      const ui::AXNodeData& p);
-  static std::unordered_map<ax::mojom::FloatAttribute, float> float_attributes(
-      const ui::AXNodeData& p);
-  static std::unordered_map<ax::mojom::BoolAttribute, bool> bool_attributes(
-      const ui::AXNodeData& p);
-  static std::unordered_map<ax::mojom::IntListAttribute, std::vector<int32_t>>
-  intlist_attributes(const ui::AXNodeData& p);
-  static std::unordered_map<ax::mojom::StringListAttribute,
-                            std::vector<std::string>>
-  stringlist_attributes(const ui::AXNodeData& p);
-  static std::unordered_map<std::string, std::string> html_attributes(
-      const ui::AXNodeData& p);
-  static std::vector<int32_t> child_ids(const ui::AXNodeData& p) {
+  static const std::vector<std::pair<ax::mojom::StringAttribute, std::string>>&
+  string_attributes(const ui::AXNodeData& p) {
+    return p.string_attributes;
+  }
+  static const std::vector<std::pair<ax::mojom::IntAttribute, int32_t>>&
+  int_attributes(const ui::AXNodeData& p) {
+    return p.int_attributes;
+  }
+  static const std::vector<std::pair<ax::mojom::FloatAttribute, float>>&
+  float_attributes(const ui::AXNodeData& p) {
+    return p.float_attributes;
+  }
+  static const std::vector<std::pair<ax::mojom::BoolAttribute, bool>>&
+  bool_attributes(const ui::AXNodeData& p) {
+    return p.bool_attributes;
+  }
+  static const std::vector<
+      std::pair<ax::mojom::IntListAttribute, std::vector<int32_t>>>&
+  intlist_attributes(const ui::AXNodeData& p) {
+    return p.intlist_attributes;
+  }
+  static const std::vector<
+      std::pair<ax::mojom::StringListAttribute, std::vector<std::string>>>&
+  stringlist_attributes(const ui::AXNodeData& p) {
+    return p.stringlist_attributes;
+  }
+  static const std::vector<std::pair<std::string, std::string>>&
+  html_attributes(const ui::AXNodeData& p) {
+    return p.html_attributes;
+  }
+  static const std::vector<int32_t>& child_ids(const ui::AXNodeData& p) {
     return p.child_ids;
   }
   static ui::AXRelativeBounds relative_bounds(const ui::AXNodeData& p) {

ui/accessibility/mojom/ax_tree_update_mojom_traits.h

@@ -33,7 +33,7 @@ struct StructTraits<ax::mojom::AXTreeUpdateDataView, ui::AXTreeUpdate> {
   static ax::mojom::EventFrom event_from(const ui::AXTreeUpdate& p) {
     return p.event_from;
   }
-  static std::vector<ui::AXEventIntent> event_intents(
+  static const std::vector<ui::AXEventIntent>& event_intents(
       const ui::AXTreeUpdate& p) {
     return p.event_intents;
   }