Readability review for Obj-C for feldstein

Readibility review code using the cocoa browser accessibility stuff for
objective c

Review URL: http://codereview.chromium.org/2951011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@53723 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 90 insertions, 43 deletions

diff --git a/chrome/browser/cocoa/browser_accessibility.mm b/chrome/browser/cocoa/browser_accessibility.mm
index 6dbf3ce..cea970a 100644
--- a/chrome/browser/cocoa/browser_accessibility.mm
+++ b/chrome/browser/cocoa/browser_accessibility.mm
@@ -4,9 +4,10 @@
 
 #include <execinfo.h>
 
+#import "chrome/browser/cocoa/browser_accessibility.h"
+
 #include "base/string16.h"
 #include "base/sys_string_conversions.h"
-#include "chrome/browser/cocoa/browser_accessibility.h"
 #include "chrome/browser/renderer_host/render_widget_host_view_mac.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebRect.h"
 
@@ -16,9 +17,9 @@ namespace {
 
 // Returns an autoreleased copy of the WebAccessibility's attribute.
 NSString* NSStringForWebAccessibilityAttribute(
-    std::map<int32, string16>& attributes,
+    const std::map<int32, string16>& attributes,
     WebAccessibility::Attribute attribute) {
-  std::map<int32, string16>::iterator iter =
+  std::map<int32, string16>::const_iterator iter =
       attributes.find(attribute);
   NSString* returnValue = @"";
   if (iter != attributes.end()) {
@@ -54,15 +55,17 @@ static const RoleEntry roles[] = {
   { WebAccessibility::ROLE_WEBCORE_LINK, NSAccessibilityLinkRole},
 };
 
+// GetState checks the bitmask used in webaccessibility.h to check
+// if the given state was set on the accessibility object.
 bool GetState(WebAccessibility accessibility, int state) {
   return ((accessibility.state >> state) & 1);
 }
 
-} // anonymous namespace
+} // namespace
 
 @implementation BrowserAccessibility
 
-- (id)initWithObject:(const WebAccessibility)accessibility
+- (id)initWithObject:(const WebAccessibility&)accessibility
             delegate:(id<BrowserAccessibilityDelegate>)delegate
               parent:(id)parent {
   if ((self = [super init])) {
@@ -73,6 +76,8 @@ bool GetState(WebAccessibility accessibility, int state) {
   return self;
 }
 
+// Returns an array of BrowserAccessibility objects, representing the
+// accessibility children of this object.
 - (NSArray*)children {
   if (!children_.get()) {
     const std::vector<WebAccessibility>& accessibilityChildren =
@@ -83,28 +88,33 @@ bool GetState(WebAccessibility accessibility, int state) {
     std::vector<WebAccessibility>::const_iterator iterator;
     for (iterator = accessibilityChildren.begin();
          iterator != accessibilityChildren.end();
-         iterator++) {
-      BrowserAccessibility* child =
+         ++iterator) {
+      scoped_nsobject<BrowserAccessibility> child(
           [[BrowserAccessibility alloc]
               initWithObject:*iterator
                     delegate:delegate_
-                      parent:self];
-      [child autorelease];
+                      parent:self]);
       [children_ addObject:child];
     }
   }
   return children_;
 }
 
+// Returns whether or not this node should be ignored in the
+// accessibility tree.
 - (BOOL)isIgnored {
   return webAccessibility_.role == WebAccessibility::ROLE_IGNORED;
 }
 
+// The origin of this accessibility object in the page's document.
+// This is relative to webkit's top-left origin, not Cocoa's
+// bottom-left origin.
 - (NSPoint)origin {
   return NSMakePoint(webAccessibility_.location.x,
                      webAccessibility_.location.y);
 }
 
+// Returns a string indicating the role of this object.
 - (NSString*)role {
   NSString* role = NSAccessibilityUnknownRole;
   WebAccessibility::Role value = webAccessibility_.role;
@@ -118,46 +128,59 @@ bool GetState(WebAccessibility accessibility, int state) {
   return role;
 }
 
+// Returns the size of this object.
 - (NSSize)size {
   return NSMakeSize(webAccessibility_.location.width,
                     webAccessibility_.location.height);
 }
 
-- (id)accessibilityAttributeValue:(NSString *)attribute {
+// Returns the accessibility value for the given attribute.  If the value isn't
+// supported this will return nil.
+- (id)accessibilityAttributeValue:(NSString*)attribute {
   if ([attribute isEqualToString:NSAccessibilityRoleAttribute]) {
     return [self role];
-  } else if ([attribute isEqualToString:NSAccessibilityDescriptionAttribute]) {
+  }
+  if ([attribute isEqualToString:NSAccessibilityDescriptionAttribute]) {
     return NSStringForWebAccessibilityAttribute(webAccessibility_.attributes,
         WebAccessibility::ATTR_DESCRIPTION);
-  } else if ([attribute isEqualToString:NSAccessibilityPositionAttribute]) {
+  }
+  if ([attribute isEqualToString:NSAccessibilityPositionAttribute]) {
     return [NSValue valueWithPoint:[delegate_ accessibilityPointInScreen:self]];
-  } else if ([attribute isEqualToString:NSAccessibilitySizeAttribute]) {
+  }
+  if ([attribute isEqualToString:NSAccessibilitySizeAttribute]) {
     return [NSValue valueWithSize:[self size]];
-  } else if (
-      [attribute isEqualToString:NSAccessibilityTopLevelUIElementAttribute] ||
+  }
+  if ([attribute isEqualToString:NSAccessibilityTopLevelUIElementAttribute] ||
       [attribute isEqualToString:NSAccessibilityWindowAttribute]) {
     return [delegate_ window];
-  } else if ([attribute isEqualToString:NSAccessibilityChildrenAttribute]) {
+  }
+  if ([attribute isEqualToString:NSAccessibilityChildrenAttribute]) {
     return [self children];
-  } else if ([attribute isEqualToString:NSAccessibilityParentAttribute]) {
+  }
+  if ([attribute isEqualToString:NSAccessibilityParentAttribute]) {
     if (parent_) {
       return NSAccessibilityUnignoredAncestor(parent_);
     }
-  } else if ([attribute isEqualToString:NSAccessibilityTitleAttribute]) {
+  }
+  if ([attribute isEqualToString:NSAccessibilityTitleAttribute]) {
     return base::SysUTF16ToNSString(webAccessibility_.name);
-  } else if ([attribute isEqualToString:NSAccessibilityHelpAttribute]) {
+  }
+  if ([attribute isEqualToString:NSAccessibilityHelpAttribute]) {
     return NSStringForWebAccessibilityAttribute(webAccessibility_.attributes,
                                                 WebAccessibility::ATTR_HELP);
-  } else if ([attribute isEqualToString:NSAccessibilityValueAttribute]) {
+  }
+  if ([attribute isEqualToString:NSAccessibilityValueAttribute]) {
     return base::SysUTF16ToNSString(webAccessibility_.value);
-  } else if (
-      [attribute isEqualToString:NSAccessibilityRoleDescriptionAttribute]) {
+  }
+  if ([attribute isEqualToString:NSAccessibilityRoleDescriptionAttribute]) {
     return NSAccessibilityRoleDescription([self role], nil);
-  } else if ([attribute isEqualToString:NSAccessibilityFocusedAttribute]) {
+  }
+  if ([attribute isEqualToString:NSAccessibilityFocusedAttribute]) {
     NSNumber* ret = [NSNumber numberWithBool:
         GetState(webAccessibility_, WebAccessibility::STATE_FOCUSED)];
     return ret;
-  } else if ([attribute isEqualToString:NSAccessibilityEnabledAttribute] ||
+  }
+  if ([attribute isEqualToString:NSAccessibilityEnabledAttribute] ||
       [attribute isEqualToString:@"AXVisited"] ||
       [attribute isEqualToString:@"AXLoaded"]) {
     return [NSNumber numberWithBool:YES];
@@ -165,14 +188,21 @@ bool GetState(WebAccessibility accessibility, int state) {
   return nil;
 }
 
-- (NSArray *)accessibilityActionNames {
+// Returns an array of action names that this object will respond to.
+- (NSArray*)accessibilityActionNames {
   return [NSArray arrayWithObjects:
       NSAccessibilityPressAction, NSAccessibilityShowMenuAction, nil];
 }
 
-- (NSArray *)accessibilityArrayAttributeValues:(NSString *)attribute
-                                         index:(NSUInteger)index
-                                      maxCount:(NSUInteger)maxCount {
+// Returns a sub-array of values for the given attribute value, starting at
+// index, with up to maxCount items.  If the given index is out of bounds,
+// or there are no values for the given attribute, it will return nil.
+// This method is used for querying subsets of values, without having to
+// return a large set of data, such as elements with a large number of
+// children.
+- (NSArray*)accessibilityArrayAttributeValues:(NSString*)attribute
+                                        index:(NSUInteger)index
+                                     maxCount:(NSUInteger)maxCount {
   NSArray* fullArray = [self accessibilityAttributeValue:attribute];
   if (!fullArray)
     return nil;
@@ -188,34 +218,39 @@ bool GetState(WebAccessibility accessibility, int state) {
   return [fullArray subarrayWithRange:subRange];
 }
 
-- (NSUInteger)accessibilityArrayAttributeCount:(NSString *)attribute {
+// Returns the count of the specified accessibility array attribute.
+- (NSUInteger)accessibilityArrayAttributeCount:(NSString*)attribute {
   NSArray* fullArray = [self accessibilityAttributeValue:attribute];
   return [fullArray count];
 }
 
-- (NSArray *)accessibilityAttributeNames {
+// Returns the list of accessibility attributes that this object supports.
+- (NSArray*)accessibilityAttributeNames {
   return [NSArray arrayWithObjects:
-      NSAccessibilityRoleAttribute,
-      NSAccessibilityRoleDescriptionAttribute,
       NSAccessibilityChildrenAttribute,
+      NSAccessibilityDescriptionAttribute,
+      NSAccessibilityEnabledAttribute,
+      NSAccessibilityFocusedAttribute,
       NSAccessibilityHelpAttribute,
       NSAccessibilityParentAttribute,
       NSAccessibilityPositionAttribute,
+      NSAccessibilityRoleAttribute,
+      NSAccessibilityRoleDescriptionAttribute,
       NSAccessibilitySizeAttribute,
       NSAccessibilityTitleAttribute,
-      NSAccessibilityDescriptionAttribute,
+      NSAccessibilityTopLevelUIElementAttribute,
       NSAccessibilityValueAttribute,
-      NSAccessibilityFocusedAttribute,
-      NSAccessibilityEnabledAttribute,
       NSAccessibilityWindowAttribute,
-      NSAccessibilityTopLevelUIElementAttribute,
       nil];
 }
 
+// Returns the deepest child that has user focus.
+// TODO(feldstein): This does not work yet.
 - (id)accessibilityFocusedUIElement {
   return self;
 }
 
+// Returns the index of the child in this objects array of children.
 - (NSUInteger)accessibilityIndexOfChild:(id)child {
   NSUInteger index = 0;
   for (BrowserAccessibility* childToCheck in [self children]) {
@@ -227,34 +262,43 @@ bool GetState(WebAccessibility accessibility, int state) {
   return NSNotFound;
 }
 
-- (BOOL)accessibilityIsAttributeSettable:(NSString *)attribute {
-  if ([attribute isEqualToString:NSAccessibilityFocusedAttribute]) {
+// Returns whether or not the specified attribute can be set by the
+// accessibility API via |accessibilitySetValue:forAttribute:|.
+- (BOOL)accessibilityIsAttributeSettable:(NSString*)attribute {
+  if ([attribute isEqualToString:NSAccessibilityFocusedAttribute])
     return GetState(webAccessibility_, WebAccessibility::STATE_FOCUSABLE);
-  } else if ([attribute isEqualToString:NSAccessibilityValueAttribute]) {
+  if ([attribute isEqualToString:NSAccessibilityValueAttribute])
     return !GetState(webAccessibility_, WebAccessibility::STATE_READONLY);
-  }
   return NO;
 }
 
+// Returns whether or not this object should be ignored in the accessibilty
+// tree.
 - (BOOL)accessibilityIsIgnored {
   return [self isIgnored];
 }
 
-- (void)accessibilityPerformAction:(NSString *)action {
+// Performs the given accessibilty action on the webkit accessibility object
+// that backs this object.
+- (void)accessibilityPerformAction:(NSString*)action {
   // TODO(feldstein): Support more actions.
   [delegate_ doDefaultAction:webAccessibility_.id];
 }
 
+// Returns the description of the given action.
 - (NSString*)accessibilityActionDescription:(NSString*)action {
   return NSAccessibilityActionDescription(action);
 }
 
+// Sets an override value for a specific accessibility attribute.
+// This class does not support this.
 - (BOOL)accessibilitySetOverrideValue:(id)value
-                         forAttribute:(NSString *)attribute {
+                         forAttribute:(NSString*)attribute {
   return NO;
 }
 
-- (void)accessibilitySetValue:(id)value forAttribute:(NSString *)attribute {
+// Sets the value for an accessibility attribute via the accessibility API.
+- (void)accessibilitySetValue:(id)value forAttribute:(NSString*)attribute {
   if ([attribute isEqualToString:NSAccessibilityFocusedAttribute]) {
     NSNumber* focusedNumber = value;
     BOOL focused = [focusedNumber intValue];
@@ -263,6 +307,9 @@ bool GetState(WebAccessibility accessibility, int state) {
   }
 }
 
+// Returns the deepest accessibility child that should not be ignored.
+// It is assumed that the hit test has been narrowed down to this object
+// or one of its children, so this will never return nil.
 - (id)accessibilityHitTest:(NSPoint)point {
   id hit = self;
   for (id child in [self children]) {