RFR: 8283551: ControlAcceleratorSupport menu items listener causes memory leak [v2]

[Andy Goryachev]

On Thu, 23 Feb 2023 09:52:08 GMT, Dean Wookey <dwookey at openjdk.org> wrote:

>> I'm not in favor of using `Private` in a method name. That is clear from the method signature and overloading methods is a valid choice  In my opinion, this is fine as is. 
>> But we could also think about naming it: `removeAcceleratorsFromSceneImpl()`
>
> I've been working on changes for a possible follow-up PR which address more bugs. For adding accelerators, there are 3 public methods, for removing there are currently 4.
> 
> I've looked at it, and it can/should be made to have 3 public remove methods that exactly mirror the public add methods (if you use a specific one to add then use the corresponding one to remove).
> 
> Further, for every private addAcceleratorsFromScene method and doAcceleratorInstall method I believe there should be a private corresponding method which reverses it. I had to rename a couple private removeAcceleratorsFromScene to doAcceleratorUninstall.
> 
> So yes, this is a very confusing class. Pairing up the add/remove methods make sense to me. Once that's done, we might want to rename some of the private ones just so it's easier to understand what each one is doing, but the public ones are fine I think.

I just want to avoid confusion when we have public and non-public methods with the same name.  but it's a minor comment, especially if there will be subsequent rework later.

-------------

PR: https://git.openjdk.org/jfx/pull/1037