<Swing Dev> [9] Review Request: 8161664: Memory leak in com.apple.laf.AquaProgressBarUI: removed progress bar still referenced

[Sergey Bylokhov]

Sorry I missed two small issues in the previous review:
  - The date in copyright should have a comma "Copyright (c) 2016, 
Oracle and/or its affiliates. All rights reserved."
  - The test should have "* @key headful" tag.

On 27.07.16 0:13, Robin Stevens wrote:
> Hello,
>
> thank you both for the review.
>
> Updated webrev: http://cr.openjdk.java.net/~rstevens/8161664/webrev.01/
>
> I have adjusted the test as suggested by Sergey:
>
> - I moved it to the javax/swing/JProgressBar package, as it is no longer
> specific for the Aqua look and feel
> - The test now loops over all installed look and feels. For that, I
> copied code from the test in
> javax/swing/JTab;e/7124218/SelectEditTableCell.java
> - The test now uses Util.generateOOME instead of the System.gc +
> System.finalization call
>
>
> Robin
>
> On Mon, Jul 25, 2016 at 10:32 PM, Sergey Bylokhov
> <Sergey.Bylokhov at oracle.com <mailto:Sergey.Bylokhov at oracle.com>> wrote:
>
>     Hi, Robin.
>     These calls do no guaranties that all weak references will be collected.
>     56     System.runFinalization();
>     57     System.gc();
>
>     I suggest to generate OOM explicitly. see for example:
>     javax/swing/UIDefaults/6795356/bug6795356.java
>
>     Note that on other platforms it will cover Metal L&F but probably
>     others can be tested as well. So it will be good to test all
>     installed L&F in the loop.
>
>
>     On 20.07.16 14:26, Alexandr Scherbatiy wrote:
>
>
>         The fix looks good to me.
>
>         Thanks,
>         Alexandr.
>
>         On 7/20/2016 11:37 AM, Robin Stevens wrote:
>
>             Hello,
>
>             please review this patch for issue JDK-8161664:
>
>             Bug: https://bugs.openjdk.java.net/browse/JDK-8161664
>             Patch: http://cr.openjdk.java.net/~rstevens/8161664/webrev.00/
>             <http://cr.openjdk.java.net/%7Erstevens/8161664/webrev.00/>
>
>
>             The problem is that in certain scenarios, the Timer in the
>             Animator of
>             the AquaProgressBarUI does not get stopped when the progress
>             bar is
>             removed from the Swing hierarchy.
>
>             Several approaches were discussed in another thread
>             (http://mail.openjdk.java.net/pipermail/swing-dev/2016-July/006330.html).
>             In the linked webrev, I opted to use the same approach as
>             what is done
>             in the BasicProgressBarUI class: only start the timer when the
>             progress bar is displayable.
>
>             To achieve this, I wrapped all calls to startAnimationTimer
>             with an if
>             statement that checks the displayable state of the progress bar.
>
>             There is one call to startAnimationTimer that is not
>             adjusted. That
>             call is only triggered from the paint method, which in turn
>             is only
>             triggered if the progress bar is displayable. As such, the
>             if check
>             was not needed there (imo).
>
>             The patch includes a test, which fails without the fix and
>             succeeds
>             afterwards.
>
>             Thanks,
>
>             Robin
>
>
>
>
>     --
>     Best regards, Sergey.
>
>


-- 
Best regards, Sergey.