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.bylok...@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. >
