Modify AccessibilityTreeFormatter to build up an internal representation of the tree

content/browser/accessibility/accessibility_tree_formatter_mac.mm

Index: content/browser/accessibility/accessibility_tree_formatter_mac.mm
diff --git a/content/browser/accessibility/accessibility_tree_formatter_mac.mm b/content/browser/accessibility/accessibility_tree_formatter_mac.mm
index f5f1964867bc3149c6f98da3f41a29d7d2abaf8b..4376e9bfeadb650787aae6bc6d424d509c13a280 100644
--- a/content/browser/accessibility/accessibility_tree_formatter_mac.mm
+++ b/content/browser/accessibility/accessibility_tree_formatter_mac.mm
@@ -7,129 +7,180 @@
 #import <Cocoa/Cocoa.h>
 
 #include "base/files/file_path.h"
+#include "base/stringprintf.h"
+#include "base/strings/sys_string_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "content/browser/accessibility/browser_accessibility_cocoa.h"
 #include "content/browser/accessibility/browser_accessibility_mac.h"
 #include "content/browser/accessibility/browser_accessibility_manager.h"
 
+using base::SysNSStringToUTF8;
+using base::SysNSStringToUTF16;
+
 namespace content {
 
 namespace {
 
-string16 Format(const char *prefix,
-                id value,
-                const char *suffix) {
-  if (value == nil)
-    return UTF8ToUTF16("");
-  NSString* format_str =
-      [NSString stringWithFormat:@"%s%%@%s", prefix, suffix];
-  NSString* tmp = [NSString stringWithFormat:format_str, value];
-  return UTF8ToUTF16([tmp cStringUsingEncoding:NSUTF8StringEncoding]);
+NSArray* ALL_ATTRIBUTES = [NSArray arrayWithObjects:
+    NSAccessibilityRoleDescriptionAttribute,
+    NSAccessibilityTitleAttribute,
+    NSAccessibilityValueAttribute,
+    NSAccessibilityMinValueAttribute,
+    NSAccessibilityMaxValueAttribute,
+    NSAccessibilityValueDescriptionAttribute,
+    NSAccessibilityDescriptionAttribute,
+    NSAccessibilityHelpAttribute,
+    @"AXInvalid",
+    NSAccessibilityDisclosingAttribute,
+    NSAccessibilityDisclosureLevelAttribute,
+    @"AXAccessKey",
+    @"AXARIAAtomic",
+    @"AXARIABusy",
+    @"AXARIALive",
+    @"AXARIARelevant",
+    NSAccessibilityEnabledAttribute,
+    NSAccessibilityFocusedAttribute,
+    @"AXLoaded",
+    @"AXLoadingProcess",
+    NSAccessibilityNumberOfCharactersAttribute,
+    NSAccessibilityOrientationAttribute,
+    @"AXRequired",
+    NSAccessibilityURLAttribute,
+    NSAccessibilityVisibleCharacterRangeAttribute,
+    @"AXVisited",
+    nil];
+
+string16 FormatString(std::string prefix,

[dmazzoni, 2013/04/08 05:46:35]

Use a const ref for string arguments, like: const std::string&

...please make sure you do that throughout.

[aboxhall, 2013/04/10 03:09:34]

I ended up deleting this method, it was silly anyway.

I went through and const-reffed anything I could in other methods.

+                      std::string& value,
+                      std::string suffix) {
+  return UTF8ToUTF16(prefix) + UTF8ToUTF16(value) + UTF8ToUTF16(suffix);
+}
+
+string16 FormatCoordinates(std::string name,
+                           std::string x_name,
+                           std::string y_name,
+                           const DictionaryValue& value) {
+  int x, y;
+  value.GetInteger(x_name, &x);
+  value.GetInteger(y_name, &y);
+  NSString* xy_str =
+    [NSString stringWithFormat:@"%s=(%d, %d)", name.c_str(), x, y];

[dmazzoni, 2013/04/08 05:46:35]

Indent 4 spaces

[aboxhall, 2013/04/10 03:09:34]

Done.

+
+  return SysNSStringToUTF16(xy_str);
 }
 
-string16 FormatPosition(BrowserAccessibility* node) {
+void PopulatePosition(BrowserAccessibility& node,
+                      DictionaryValue* position) {
   // The NSAccessibility position of an object is in global coordinates and
   // based on the lower-left corner of the object. To make this easier and less
   // confusing, convert it to local window coordinates using the top-left
   // corner when dumping the position.
-  BrowserAccessibility* root = node->manager()->GetRoot();
+  BrowserAccessibility* root = node.manager()->GetRoot();
   BrowserAccessibilityCocoa* cocoa_root = root->ToBrowserAccessibilityCocoa();
   NSPoint root_position = [[cocoa_root position] pointValue];
   NSSize root_size = [[cocoa_root size] sizeValue];
   int root_top = -static_cast<int>(root_position.y + root_size.height);
   int root_left = static_cast<int>(root_position.x);
 
-  BrowserAccessibilityCocoa* cocoa_node = node->ToBrowserAccessibilityCocoa();
+  BrowserAccessibilityCocoa* cocoa_node = node.ToBrowserAccessibilityCocoa();
   NSPoint node_position = [[cocoa_node position] pointValue];
   NSSize node_size = [[cocoa_node size] sizeValue];
 
-  NSString* position_str =
-      [NSString stringWithFormat:@"position=(%d, %d)",
-                static_cast<int>(node_position.x - root_left),
-                static_cast<int>(
-                    -node_position.y - node_size.height - root_top)];
-  return UTF8ToUTF16([position_str cStringUsingEncoding:NSUTF8StringEncoding]);
+  position->SetInteger("x", static_cast<int>(node_position.x - root_left));
+  position->SetInteger("y",
+      static_cast<int>(-node_position.y - node_size.height - root_top));
 }
 
-string16 FormatSize(BrowserAccessibility* node) {
-  BrowserAccessibilityCocoa* cocoa_node = node->ToBrowserAccessibilityCocoa();
+void PopulateSize(BrowserAccessibility& node, DictionaryValue* size) {
+  BrowserAccessibilityCocoa* cocoa_node = node.ToBrowserAccessibilityCocoa();
   NSSize node_size = [[cocoa_node size] sizeValue];
-  NSString* size_str =
-      [NSString stringWithFormat:@"size=(%d, %d)",
-                static_cast<int>(node_size.width),
-                static_cast<int>(node_size.height)];
-  return UTF8ToUTF16([size_str cStringUsingEncoding:NSUTF8StringEncoding]);
+  size->SetInteger("height", static_cast<int>(node_size.height));
+  size->SetInteger("width", static_cast<int>(node_size.width));
 }
 
 }  // namespace
 
 void AccessibilityTreeFormatter::Initialize() {}
 
-string16 AccessibilityTreeFormatter::ToString(BrowserAccessibility* node,
-                                               char* prefix) {
-  StartLine();
-  NSArray* requestedAttributes = [NSArray arrayWithObjects:
-      NSAccessibilityRoleDescriptionAttribute,
-      NSAccessibilityTitleAttribute,
-      NSAccessibilityValueAttribute,
-      NSAccessibilityMinValueAttribute,
-      NSAccessibilityMaxValueAttribute,
-      NSAccessibilityValueDescriptionAttribute,
-      NSAccessibilityDescriptionAttribute,
-      NSAccessibilityHelpAttribute,
-      @"AXInvalid",
-      NSAccessibilityDisclosingAttribute,
-      NSAccessibilityDisclosureLevelAttribute,
-      @"AXAccessKey",
-      @"AXARIAAtomic",
-      @"AXARIABusy",
-      @"AXARIALive",
-      @"AXARIARelevant",
-      NSAccessibilityEnabledAttribute,
-      NSAccessibilityFocusedAttribute,
-      @"AXLoaded",
-      @"AXLoadingProcess",
-      NSAccessibilityNumberOfCharactersAttribute,
-      NSAccessibilityOrientationAttribute,
-      @"AXRequired",
-      NSAccessibilityURLAttribute,
-      NSAccessibilityVisibleCharacterRangeAttribute,
-      @"AXVisited",
-      nil];
-
-  NSArray* defaultAttributes = [NSArray arrayWithObjects:
-      NSAccessibilityTitleAttribute,
-      NSAccessibilityValueAttribute,
-      nil];
-
+void AccessibilityTreeFormatter::AddProperties(BrowserAccessibility* node,
+                                               DictionaryValue* dict) {
   BrowserAccessibilityCocoa* cocoa_node = node->ToBrowserAccessibilityCocoa();
   NSArray* supportedAttributes = [cocoa_node accessibilityAttributeNames];
 
-  Add(true,
-      Format("", [cocoa_node accessibilityAttributeValue:
-                      NSAccessibilityRoleAttribute],
-             ""));
-  Add(false,
-      Format("subrole=", [cocoa_node accessibilityAttributeValue:
-                              NSAccessibilitySubroleAttribute],
-             ""));
-  for (NSString* requestedAttribute in requestedAttributes) {
+  std::string role = SysNSStringToUTF8(
+      [cocoa_node accessibilityAttributeValue:NSAccessibilityRoleAttribute]);
+  dict->SetString(SysNSStringToUTF8(NSAccessibilityRoleAttribute), role);
+
+  NSString* subrole =
+      [cocoa_node accessibilityAttributeValue:NSAccessibilitySubroleAttribute];
+  if (subrole != nil) {
+    dict->SetString(SysNSStringToUTF8(NSAccessibilitySubroleAttribute),
+                    SysNSStringToUTF8(subrole));
+  }
+
+  for (NSString* requestedAttribute in ALL_ATTRIBUTES) {
     if (![supportedAttributes containsObject:requestedAttribute]) {
       continue;
     }
-    NSString* methodName =
-        [cocoa_node methodNameForAttribute:requestedAttribute];
-    Add([defaultAttributes containsObject:requestedAttribute],
-        Format([[NSString stringWithFormat:@"%@='", methodName]
-                   cStringUsingEncoding:NSUTF8StringEncoding],
-               [cocoa_node accessibilityAttributeValue:
-                    requestedAttribute],
-               "'"));
+    id value = [cocoa_node accessibilityAttributeValue:requestedAttribute];
+    if (value != nil) {
+      dict->SetString(
+          SysNSStringToUTF8(requestedAttribute),
+          SysNSStringToUTF16([NSString stringWithFormat:@"%@", value]));
+    }
+  }
+  DictionaryValue* position = new DictionaryValue;
+  PopulatePosition(*node, position);
+  dict->Set("position", position);
+  DictionaryValue* size = new DictionaryValue;
+  PopulateSize(*node, size);
+  dict->Set("size", size);
+}
+
+string16 AccessibilityTreeFormatter::ToString(DictionaryValue* dict,
+                                              const string16& indent) {
+  string16 line;
+  NSArray* defaultAttributes =
+      [NSArray arrayWithObjects:NSAccessibilityTitleAttribute,
+                                NSAccessibilityValueAttribute,
+                                nil];
+  std::string s_value;
+  dict->GetString(SysNSStringToUTF8(NSAccessibilityRoleAttribute), &s_value);
+  WriteAttribute(true, FormatString("", s_value, ""), &line);
+
+  if (dict->GetString(SysNSStringToUTF8(NSAccessibilitySubroleAttribute),

[dmazzoni, 2013/04/08 05:46:35]

Put SysNSStringToUTF8(NSAccessibilitySubroleAttribute) in a local variable to
make this more readable (especially since it's used twice).

Why is "subrole" a special case? Can't it be handled as part of ALL_ATTRIBUTES
below?

[aboxhall, 2013/04/10 03:09:34]

Done.
Only because as originally written, it was formatted without enclosing single
quotes around the attribute value. Since I'm rebasing all the tests anyway, I
can make it behave the same in this change if that's better.

+                      &s_value)) {
+    WriteAttribute(
+        false,
+        FormatString((SysNSStringToUTF8(NSAccessibilitySubroleAttribute) + "="),
+                     s_value,
+                     ""),
+        &line);
+  }
+
+  for (NSString* requestedAttribute in ALL_ATTRIBUTES) {
+    if (!dict->GetString(SysNSStringToUTF8(requestedAttribute), &s_value))
+      continue;
+    NSString* prefix = [NSString stringWithFormat:@"%@='", requestedAttribute];
+    WriteAttribute([defaultAttributes containsObject:requestedAttribute],
+                   FormatString(SysNSStringToUTF8(prefix),
+                                s_value,
+                                "'"),
+                   &line);
+  }
+  DictionaryValue* d_value;
+  if (dict->GetDictionary("position", &d_value)) {
+    WriteAttribute(false,
+                   FormatCoordinates("position", "x", "y", *d_value),
+                   &line);
+  }
+  if (dict->GetDictionary("size", &d_value)) {
+    WriteAttribute(false,
+                   FormatCoordinates("size", "width", "height", *d_value),
+                   &line);
   }
-  Add(false, FormatPosition(node));
-  Add(false, FormatSize(node));
 
-  return ASCIIToUTF16(prefix) + FinishLine() + ASCIIToUTF16("\n");
+  return indent + line + ASCIIToUTF16("\n");
 }
 
 // static