Hi Anton, Why did you change CAccessible.dispose() from protected to public? Shouldn't it be left protected? -Pete
On 12/22/15 3:45 PM, Anton Tarasov wrote: > Hi Pete, > > Thanks for the review! > >> On 22 Dec 2015, at 23:07, Pete Brunet <peter.bru...@oracle.com >> <mailto:peter.bru...@oracle.com>> wrote: >> >> Hi Anton, Some comments/questions: >> - Some copyright dates need updating > > Indeed, I’ll update them. > >> - Line 1112 of JavaComponentAccessibility: does the release of >> jaccessible cause a release of jparent? > > As I can see, jparent there is only a ref to jComponent, which in its > turn is a JNIGlobalRef (or anyway is a class field). So, I don’t see > the need to delete it… (or did I miss something?) > >> - Line 7155 of Component.java: is that the only place where this >> means is needed? > > If you mean to call AC.dispose() than - yes, I think so. We rather > don’t want to dispose the context until the Component goes out of the > UI hierarchy, which is when Component.removeNotify() is _always_ > getting called (for hw & lw components). > > Anton. > >> >> Pete >> >> On 12/22/15 8:10 AM, Anton Tarasov wrote: >>> Hi guys! >>> >>> Could you please review the problem I’ve filed and the suggested fix? >>> >>> bug: JDK-8145984 >>> <https://bugs.openjdk.java.net/browse/JDK-8145984> >>> sun.lwawt.macosx.CAccessible >>> leaks >>> webrev: http://cr.openjdk.java.net/~ant/JDK-8145984/jdk9/webrev.0 >>> <http://cr.openjdk.java.net/%7Eant/JDK-8145984/jdk9/webrev.0> >>> >>> (This is to be addressed in 8u/9. The webrev for 8u is in JIRA, it’s >>> identical except the paths.) >>> >>> Please, find the details in JIRA. >>> >>> Thanks! >>> Anton. >> >
