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.bylok...@oracle.com <mailto: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.
--
Best regards, Sergey.
