Make AccessibilityNodeData more compact.

content/browser/accessibility/browser_accessibility.h

Index: content/browser/accessibility/browser_accessibility.h
diff --git a/content/browser/accessibility/browser_accessibility.h b/content/browser/accessibility/browser_accessibility.h
index 8fda68fc07908b80322804ff4215a29a89f7a652..0be7eaa076bc9d3d7c053b014c04c867b64b589a 100644
--- a/content/browser/accessibility/browser_accessibility.h
+++ b/content/browser/accessibility/browser_accessibility.h
@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/strings/string16.h"
 #include "build/build_config.h"
 #include "content/common/accessibility_node_data.h"
 #include "content/common/content_export.h"
@@ -26,12 +27,6 @@ class BrowserAccessibilityWin;
 class BrowserAccessibilityGtk;
 #endif
 
-typedef std::map<AccessibilityNodeData::BoolAttribute, bool> BoolAttrMap;
-typedef std::map<AccessibilityNodeData::FloatAttribute, float> FloatAttrMap;
-typedef std::map<AccessibilityNodeData::IntAttribute, int> IntAttrMap;
-typedef std::map<AccessibilityNodeData::StringAttribute, string16>
-    StringAttrMap;
-
 ////////////////////////////////////////////////////////////////////////////////
 //
 // BrowserAccessibility
@@ -136,49 +131,21 @@ class CONTENT_EXPORT BrowserAccessibility {
   // Accessors
   //
 
-  const BoolAttrMap& bool_attributes() const {
-    return bool_attributes_;
-  }
-
-  const FloatAttrMap& float_attributes() const {
-    return float_attributes_;
-  }
-
-  const IntAttrMap& int_attributes() const {
-    return int_attributes_;
-  }
-
-  const StringAttrMap& string_attributes() const {
-    return string_attributes_;
-  }
-
   const std::vector<BrowserAccessibility*>& children() const {
     return children_;
   }
-  const std::vector<std::pair<string16, string16> >& html_attributes() const {
+  const std::vector<std::pair<std::string, std::string> >&
+  html_attributes() const {
     return html_attributes_;
   }
   int32 index_in_parent() const { return index_in_parent_; }
-  const std::vector<int32>& indirect_child_ids() const {
-    return indirect_child_ids_;
-  }
-  const std::vector<int32>& line_breaks() const {
-    return line_breaks_;
-  }
-  const std::vector<int32>& cell_ids() const {
-    return cell_ids_;
-  }
-  const std::vector<int32>& unique_cell_ids() const {
-    return unique_cell_ids_;
-  }
   gfx::Rect location() const { return location_; }
   BrowserAccessibilityManager* manager() const { return manager_; }
-  const string16& name() const { return name_; }
+  const std::string& name() const { return name_; }
   int32 renderer_id() const { return renderer_id_; }
   int32 role() const { return role_; }
-  const string16& role_name() const { return role_name_; }
   int32 state() const { return state_; }
-  const string16& value() const { return value_; }
+  const std::string& value() const { return value_; }
   bool instance_active() const { return instance_active_; }
 
 #if defined(OS_MACOSX) && __OBJC__
@@ -208,10 +175,23 @@ class CONTENT_EXPORT BrowserAccessibility {
   // returns true if found.
   bool GetStringAttribute(
       AccessibilityNodeData::StringAttribute attribute, string16* value) const;
+  bool GetStringAttribute(
+      AccessibilityNodeData::StringAttribute attribute, std::string* value)
+          const;
+
+  void SetStringAttribute(
+      AccessibilityNodeData::StringAttribute attribute,
+      const std::string& value);
+
+  // Retrieve a reference an integer list attribute from the integer list
+  // attribute map, or a reference to an empty vector if not found.
+  const std::vector<int32>& GetIntListAttribute(

[aboxhall, 2013/07/31 18:34:47]

Why does this accessor return the value when others assign it to an out
parameter?

[dmazzoni, 2013/08/06 17:36:34]

The distinction is whether you need to know whether the attribute
is present or not. As an example, there's a difference between
VALUE_FOR_RANGE being present and it being zero.

It's worth improving this too while I'm at it. I made all of the
attribute accessors have two variants - one that returns the
value or a default value if it's missing, and another that takes
a pointer and returns a bool indicating if the attribute was
present.

That made it possible to save a few lines of code in some places
where we don't care whether the attribute is present or not.

+      AccessibilityNodeData::IntListAttribute attribute) const;
 
   // Retrieve the value of a html attribute from the attribute map and
   // returns true if found.
   bool GetHtmlAttribute(const char* attr, string16* value) const;
+  bool GetHtmlAttribute(const char* attr, std::string* value) const;
 
   // Utility method to handle special cases for ARIA booleans, tristates and
   // booleans which have a "mixed" state.
@@ -236,7 +216,7 @@ class CONTENT_EXPORT BrowserAccessibility {
   bool IsEditableText() const;
 
   // Append the text from this node and its children.
-  string16 GetTextRecursive() const;
+  std::string GetTextRecursive() const;
 
  protected:
   // Perform platform specific initialization. This can be called multiple times
@@ -264,21 +244,23 @@ class CONTENT_EXPORT BrowserAccessibility {
   std::vector<BrowserAccessibility*> children_;
 
   // Accessibility metadata from the renderer
-  string16 name_;
-  string16 value_;
-  BoolAttrMap bool_attributes_;
-  IntAttrMap int_attributes_;
-  FloatAttrMap float_attributes_;
-  StringAttrMap string_attributes_;
-  std::vector<std::pair<string16, string16> > html_attributes_;
+  std::string name_;
+  std::string value_;
+  std::vector<std::pair<

[aboxhall, 2013/07/31 18:34:47]

Could we add using statements for each attribute type to make this a bit more
readable?

[dmazzoni, 2013/08/06 17:36:34]

That's generally frowned-upon in header files.

+      AccessibilityNodeData::BoolAttribute, bool> > bool_attributes_;

[aboxhall, 2013/07/31 18:34:47]

Could we add using statements for AccessibilityNodeData::FooAttribute to make
these a little more readable?

[dmazzoni, 2013/08/06 17:36:34]

Same.

However, a longer-term plan is to move these enums to
Blink, and we can come up with more compact names at
that time.

+  std::vector<std::pair<
+      AccessibilityNodeData::FloatAttribute, float> > float_attributes_;
+  std::vector<std::pair<
+      AccessibilityNodeData::IntAttribute, int> > int_attributes_;
+  std::vector<std::pair<
+      AccessibilityNodeData::StringAttribute, std::string> > string_attributes_;
+  std::vector<std::pair<
+      AccessibilityNodeData::IntListAttribute, std::vector<int32> > >
+          intlist_attributes_;
+  std::vector<std::pair<std::string, std::string> > html_attributes_;
   int32 role_;
   int32 state_;
-  string16 role_name_;
   gfx::Rect location_;
-  std::vector<int32> indirect_child_ids_;
-  std::vector<int32> line_breaks_;
-  std::vector<int32> cell_ids_;
-  std::vector<int32> unique_cell_ids_;
 
   // BrowserAccessibility objects are reference-counted on some platforms.
   // When we're done with this object and it's removed from our accessibility