New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 735793 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug
Team-Accessibility



Sign in to add a comment

Mac a11y: AXTextMarker always leaks

Project Member Reported by tapted@chromium.org, Jun 22 2017

Issue description

Chrome Version       : 61.0.3124.4
OS Version: OS X 10.12.5

1. Open chrome://version and enable voiceover
2. Connect to Chrome in Instruments with the leak checker
2a. (Record and stop then wait for a bit while the symbol cache loads), then:
3. Record (again) while navigating chrome://version using Ctrl+Alt+arrows


(more complex navigations might trigger more leaks)


There are 3 leaks identified (multiple times; once for each VoiceOver navigation). All stacks are under -[BrowserAccessibilityCocoa accessibilityAttributeValue:forParameter:]

1. The `new AXPositionType()` called via CreateTextMarker(position->CreatePreviousLineStartPosition()) to satisfy AXPreviousLineStartTextMarkerForTextMarker. I suspect the .release() in CreateTextMarker() is the cause of this one:

  static AXPositionInstance CreateTextPosition(int tree_id,
                                               int32_t anchor_id,
                                               int text_offset,
                                               AXTextAffinity affinity) {
    AXPositionInstance new_position(new AXPositionType());
    new_position->Initialize(AXPositionKind::TEXT_POSITION, tree_id, anchor_id,
                             INVALID_INDEX, text_offset, affinity);
    return new_position;
  }



2. and 3. are the two calls to `AXTextMarkerCreate` made to satisfy AXTextMarkerRangeForUIElement in 

id CreateTextMarkerRange(const AXPlatformRange range) {
  AXTextMarkerRef start_marker = AXTextMarkerCreate(
      kCFAllocatorDefault, reinterpret_cast<const UInt8*>(range.anchor()),
      sizeof(AXPlatformPosition));
  AXTextMarkerRef end_marker = AXTextMarkerCreate(
      kCFAllocatorDefault, reinterpret_cast<const UInt8*>(range.focus()),
      sizeof(AXPlatformPosition));
  AXTextMarkerRangeRef marker_range =
      AXTextMarkerRangeCreate(kCFAllocatorDefault, start_marker, end_marker);
  return static_cast<id>(
      base::mac::CFTypeRefToNSObjectAutorelease(marker_range));
}


For 2. and 3., it's harder to tell the source of the leak. I'll see if I can make it go away.
 
Screen Shot 2017-06-22 at 2.08.58 pm.png
156 KB View Download

Comment 1 by tapted@chromium.org, Jun 22 2017

I think I got them all - https://codereview.chromium.org/2949833006

AXTextMarkerRangeCreate does a CFRetain on each of the AXTextMarkerRef passed to it. So CreateTextMarkerRange must CFRelease them before it returns.
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ed607555484664fd396c3422d02f5c40470b9a8

commit 1ed607555484664fd396c3422d02f5c40470b9a8
Author: tapted <tapted@chromium.org>
Date: Thu Jun 22 06:30:33 2017

Mac a11y: Fix AXTextMarker leaks.

We have to rely on undocumented APIs for VoiceOver on Mac.

AXTextMarkerCreate() copies from the buffer it's provided, and doesn't
retain it.

AXTextMarkerRangeCreate() adds a retain count to the pair of
AXTextMarker it's provided, so CreatePositionFromTextMarker must do a
CFRelease.

BUG= 735793 

Review-Url: https://codereview.chromium.org/2949833006
Cr-Commit-Position: refs/heads/master@{#481463}

[modify] https://crrev.com/1ed607555484664fd396c3422d02f5c40470b9a8/content/browser/accessibility/browser_accessibility_cocoa.mm

Comment 3 by tapted@chromium.org, Jun 22 2017

Status: Fixed (was: Started)
Cc: pnangunoori@chromium.org
Labels: TE-Hardware-Dependency
As mentioned in the point# 2, we don't have any leak checker device. Hence, adding the label 'TE-Hardware-Dependency'.

Sign in to add a comment