Skip to content

Commit

Permalink
8275720: CommonComponentAccessibility.createWithParent isWrapped caus…
Browse files Browse the repository at this point in the history
…es mem leak

Reviewed-by: kizune
Backport-of: 574f8903ee1f74bdf7154d670d96c36d94b38b4d
  • Loading branch information
Anton Tarasov committed Nov 26, 2021
1 parent 565ec1a commit a1049e8
Show file tree
Hide file tree
Showing 10 changed files with 134 additions and 77 deletions.
Expand Up @@ -22,7 +22,7 @@
* questions.
*/

#import "CommonComponentAccessibility.h"
#import "ComponentWrapperAccessibility.h"

@interface CellAccessibility : CommonComponentAccessibility
@interface CellAccessibility : ComponentWrapperAccessibility
@end
Expand Up @@ -35,24 +35,6 @@ - (NSAccessibilityRole)accessibilityRole
return NSAccessibilityCellRole;;
}

- (NSArray *)accessibilityChildren
{
NSArray *children = [super accessibilityChildren];
if (children == NULL) {
NSString *javaRole = [self javaRole];
CommonComponentAccessibility *newChild = [CommonComponentAccessibility createWithParent:self
accessible:self->fAccessible
role:javaRole
index:self->fIndex
withEnv:[ThreadUtilities getJNIEnv]
withView:self->fView
isWrapped:NO];
return [NSArray arrayWithObject:newChild];
} else {
return children;
}
}

- (NSInteger)accessibilityIndex
{
return self->fIndex;
Expand Down
Expand Up @@ -65,20 +65,16 @@

+ (void) initializeRolesMap;

+ (CommonComponentAccessibility* _Nullable) getComponentAccessibility:(NSString* _Nonnull)role;
+ (CommonComponentAccessibility * _Nullable) getComponentAccessibility:(NSString * _Nonnull)role andParent:(CommonComponentAccessibility * _Nonnull)parent;
+ (Class _Nonnull) getComponentAccessibilityClass:(NSString* _Nonnull)role;
+ (Class _Nonnull) getComponentAccessibilityClass:(NSString * _Nonnull)role andParent:(CommonComponentAccessibility * _Nonnull)parent;

+ (NSArray* _Nullable)childrenOfParent:(CommonComponentAccessibility* _Nonnull)parent withEnv:(JNIEnv _Nonnull * _Nonnull)env withChildrenCode:(NSInteger)whichChildren allowIgnored:(BOOL)allowIgnored;
+ (NSArray* _Nullable)childrenOfParent:(CommonComponentAccessibility* _Nonnull)parent withEnv:(JNIEnv _Nonnull * _Nonnull)env withChildrenCode:(NSInteger)whichChildren allowIgnored:(BOOL)allowIgnored recursive:(BOOL)recursive;
+ (CommonComponentAccessibility* _Nullable) createWithParent:(CommonComponentAccessibility* _Nullable)parent accessible:(jobject _Nonnull)jaccessible role:(NSString* _Nonnull)javaRole index:(jint)index withEnv:(JNIEnv _Nonnull * _Nonnull)env withView:(NSView* _Nonnull)view;
+ (CommonComponentAccessibility* _Nullable) createWithParent:(CommonComponentAccessibility* _Nullable)parent withClass:(Class _Nonnull)classType accessible:(jobject _Nonnull)jaccessible role:(NSString* _Nonnull)javaRole index:(jint)index withEnv:(JNIEnv _Nonnull * _Nonnull)env withView:(NSView* _Nonnull)view;
+ (CommonComponentAccessibility* _Nullable) createWithAccessible:(jobject _Nonnull)jaccessible role:(NSString* _Nonnull)role index:(jint)index withEnv:(JNIEnv _Nonnull * _Nonnull)env withView:(NSView* _Nonnull)view;
+ (CommonComponentAccessibility* _Nullable) createWithAccessible:(jobject _Nonnull)jaccessible withEnv:(JNIEnv _Nonnull * _Nonnull)env withView:(NSView* _Nonnull)view;

// If the isWraped parameter is true, then the object passed as a parent was created based on the same java component,
// but performs a different NSAccessibilityRole of a table cell, or a list row, or tree row,
// and we need to create an element whose role corresponds to the role in Java.
+ (CommonComponentAccessibility* _Nullable) createWithParent:(CommonComponentAccessibility* _Nullable)parent accessible:(jobject _Nonnull)jaccessible role:(NSString* _Nonnull)javaRole index:(jint)index withEnv:(JNIEnv _Nonnull * _Nonnull)env withView:(NSView* _Nonnull)view isWrapped:(BOOL)wrapped;

// The current parameter is used to bypass the check for an item's index on the parent so that the item is created. This is necessary,
// for example, for AccessibleJTreeNode, whose currentComponent has index -1
+ (CommonComponentAccessibility* _Nullable) createWithAccessible:(jobject _Nonnull)jaccessible withEnv:(JNIEnv _Nonnull * _Nonnull)env withView:(NSView* _Nonnull)view isCurrent:(BOOL)current;
Expand Down
Expand Up @@ -223,9 +223,9 @@ + (void) initializeRolesMap {

/*
* If new implementation of the accessible component peer for the given role exists
* return the allocated class otherwise return nil to let old implementation being initialized
* return the component's class otherwise return CommonComponentAccessibility
*/
+ (CommonComponentAccessibility *) getComponentAccessibility:(NSString *)role
+ (Class) getComponentAccessibilityClass:(NSString *)role
{
AWT_ASSERT_APPKIT_THREAD;
if (rolesMap == nil) {
Expand All @@ -234,22 +234,22 @@ + (CommonComponentAccessibility *) getComponentAccessibility:(NSString *)role

NSString *className = [rolesMap objectForKey:role];
if (className != nil) {
return [NSClassFromString(className) alloc];
return NSClassFromString(className);
}
return [CommonComponentAccessibility alloc];
return [CommonComponentAccessibility class];
}

+ (CommonComponentAccessibility *) getComponentAccessibility:(NSString *)role andParent:(CommonComponentAccessibility *)parent
+ (Class) getComponentAccessibilityClass:(NSString *)role andParent:(CommonComponentAccessibility *)parent
{
AWT_ASSERT_APPKIT_THREAD;
if (rolesMap == nil) {
[self initializeRolesMap];
}
NSString *className = [rowRolesMapForParent objectForKey:[[parent class] className]];
if (className == nil) {
return [CommonComponentAccessibility getComponentAccessibility:role];
return [CommonComponentAccessibility getComponentAccessibilityClass:role];
}
return [NSClassFromString(className) alloc];
return NSClassFromString(className);
}

- (id)initWithParent:(NSObject *)parent withEnv:(JNIEnv *)env withAccessible:(jobject)accessible withIndex:(jint)index withView:(NSView *)view withJavaRole:(NSString *)javaRole
Expand Down Expand Up @@ -537,10 +537,11 @@ + (CommonComponentAccessibility *) createWithAccessible:(jobject)jaccessible rol

+ (CommonComponentAccessibility *) createWithParent:(CommonComponentAccessibility *)parent accessible:(jobject)jaccessible role:(NSString *)javaRole index:(jint)index withEnv:(JNIEnv *)env withView:(NSView *)view
{
return [CommonComponentAccessibility createWithParent:parent accessible:jaccessible role:javaRole index:index withEnv:env withView:view isWrapped:NO];
Class classType = [self getComponentAccessibilityClass:javaRole andParent:parent];
return [CommonComponentAccessibility createWithParent:parent withClass:classType accessible:jaccessible role:javaRole index:index withEnv:env withView:view];
}

+ (CommonComponentAccessibility *) createWithParent:(CommonComponentAccessibility *)parent accessible:(jobject)jaccessible role:(NSString *)javaRole index:(jint)index withEnv:(JNIEnv *)env withView:(NSView *)view isWrapped:(BOOL)wrapped
+ (CommonComponentAccessibility *) createWithParent:(CommonComponentAccessibility *)parent withClass:(Class)classType accessible:(jobject)jaccessible role:(NSString *)javaRole index:(jint)index withEnv:(JNIEnv *)env withView:(NSView *)view
{
GET_CACCESSIBLE_CLASS_RETURN(NULL);
DECLARE_FIELD_RETURN(jf_ptr, sjc_CAccessible, "ptr", "J", NULL);
Expand All @@ -553,11 +554,8 @@ + (CommonComponentAccessibility *) createWithParent:(CommonComponentAccessibilit
return [[value retain] autorelease];
}

// otherwise, create a new instance
CommonComponentAccessibility *newChild = [CommonComponentAccessibility getComponentAccessibility:javaRole andParent:parent];

// must init freshly -alloc'd object
[newChild initWithParent:parent withEnv:env withAccessible:jCAX withIndex:index withView:view withJavaRole:javaRole]; // must init new instance
CommonComponentAccessibility *newChild =
[[classType alloc] initWithParent:parent withEnv:env withAccessible:jCAX withIndex:index withView:view withJavaRole:javaRole];

// If creating a JPopupMenu (not a combobox popup list) need to fire menuOpened.
// This is the only way to know if the menu is opening; visible state change
Expand Down
@@ -0,0 +1,48 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, JetBrains s.r.o.. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
#import "CommonComponentAccessibility.h"

/**
* Some native a11y elements do not have direct peers in Java, like list rows and cells.
* However, these elements are required by Cocoa in order for a11y to work properly.
* The ComponentWrapperAccessibility interface provides a concept of wrapping an element
* originated from java (like a list item, or a table element) with a component
* which has a11y role required Cocoa (like NSAccessibilityRowRole, or NSAccessibilityCellRole)
* but does not have peer in java.
*
* The wrapping component becomes a parent of the wrapped child in the a11y hierarchy.
* The child component is created automatically on demand with the same set of arguments,
* except that it has a11y role of its java peer.
*
* It is important that only the wrapping component is linked with sun.lwawt.macosx.CAccessible
* and thus its lifecycle depends on the java accessible. So when the same java accessible is passed
* to create a native peer, the wrapping component is retrieved in case it has already been
* created (see [CommonComponentAccessibility createWithParent]). When the wrapping component is
* deallocated (as triggered from the java side) it releases the wrapped child.
*/
@interface ComponentWrapperAccessibility : CommonComponentAccessibility

@property (nonatomic, retain) CommonComponentAccessibility *wrappedChild;

@end
@@ -0,0 +1,57 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, JetBrains s.r.o.. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
#import "ComponentWrapperAccessibility.h"
#import "ThreadUtilities.h"

@implementation ComponentWrapperAccessibility

@synthesize wrappedChild;

- (NSAccessibilityRole)accessibilityRole {
@throw [NSException exceptionWithName:NSInternalInconsistencyException
reason:[NSString stringWithFormat:@"You must override -(NSAccessibilityRole)accessibilityRole in a subclass"]
userInfo:nil];
}

- (NSArray *)accessibilityChildren {
if (!wrappedChild) {
wrappedChild =
[[CommonComponentAccessibility alloc] initWithParent:self
withEnv:[ThreadUtilities getJNIEnv]
withAccessible:fAccessible
withIndex:0
withView:fView
withJavaRole:fJavaRole];
}
return [NSArray arrayWithObject:wrappedChild];
}

- (void)dealloc {
if (wrappedChild) {
[wrappedChild release];
}
[super dealloc];
}

@end
Expand Up @@ -22,7 +22,7 @@
* questions.
*/

#import "CommonComponentAccessibility.h"
#import "ComponentWrapperAccessibility.h"

@interface ListRowAccessibility : CommonComponentAccessibility <NSAccessibilityRow>
@interface ListRowAccessibility : ComponentWrapperAccessibility <NSAccessibilityRow>
@end
Expand Up @@ -38,25 +38,6 @@ - (NSAccessibilityRole)accessibilityRole
return NSAccessibilityRowRole;
}

- (NSArray *)accessibilityChildren
{
NSArray *children = [super accessibilityChildren];
if (children == NULL) {

// Since the row element has already been created, we should no create it again, but just retrieve it by a pointer, that's why isWrapped is set to YES.
CommonComponentAccessibility *newChild = [CommonComponentAccessibility createWithParent:self
accessible:self->fAccessible
role:self->fJavaRole
index:self->fIndex
withEnv:[ThreadUtilities getJNIEnv]
withView:self->fView
isWrapped:YES];
return [NSArray arrayWithObject:newChild];
} else {
return children;
}
}

- (NSInteger)accessibilityIndex
{
return [[self accessibilityParent] accessibilityIndexOfChild:self];
Expand Down
Expand Up @@ -72,14 +72,7 @@ - (NSArray *)accessibilityChildren
return children;
}
}

return [NSArray arrayWithObject:[CommonComponentAccessibility createWithParent:self
accessible:self->fAccessible
role:self->fJavaRole
index:self->fIndex
withEnv:env
withView:self->fView
isWrapped:YES]];
return [super accessibilityChildren];
}

- (NSInteger)accessibilityDisclosureLevel
Expand Down
Expand Up @@ -84,13 +84,15 @@ - (NSArray *)accessibilityChildren
(*env)->DeleteLocalRef(env, jkey);
}

CellAccessibility *child = [[CellAccessibility alloc] initWithParent:self
withEnv:env
withAccessible:jchild
withIndex:childIndex
withView:self->fView
withJavaRole:childJavaRole];
[children addObject:[[child retain] autorelease]];
CellAccessibility *child = (CellAccessibility *)
[CommonComponentAccessibility createWithParent:self
withClass:[CellAccessibility class]
accessible:jchild
role:childJavaRole
index:childIndex
withEnv:env
withView:self->fView];
[children addObject:child];

(*env)->DeleteLocalRef(env, jchild);
(*env)->DeleteLocalRef(env, jchildJavaRole);
Expand Down

1 comment on commit a1049e8

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.