The fix looks good to me.
Thanks,
Alexandr.
On 03/08/16 14:05, Robin Stevens wrote:
Gentle reminder: can I have a second person reviewing the latest webrev:
http://cr.openjdk.java.net/~rstevens/8161664/webrev.02/
<http://cr.openjdk.java.net/%7Erstevens/8161664/webrev.02/>
Thanks,
Robin
On Thu, Jul 28, 2016 at 10:55 AM, Sergey Bylokhov
<sergey.bylok...@oracle.com <mailto:sergey.bylok...@oracle.com>> wrote:
Thanks! Looks fine.
On 28.07.16 0:16, Robin Stevens wrote:
Here is the updated webrev which includes the comma in the
copyright and
the extra tag:
http://cr.openjdk.java.net/~rstevens/8161664/webrev.02/
<http://cr.openjdk.java.net/%7Erstevens/8161664/webrev.02/>
Robin
On Wed, Jul 27, 2016 at 12:08 AM, Sergey Bylokhov
<sergey.bylok...@oracle.com
<mailto:sergey.bylok...@oracle.com>
<mailto:sergey.bylok...@oracle.com
<mailto:sergey.bylok...@oracle.com>>> wrote:
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/
<http://cr.openjdk.java.net/%7Erstevens/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>
<mailto:sergey.bylok...@oracle.com
<mailto:sergey.bylok...@oracle.com>>
<mailto:sergey.bylok...@oracle.com
<mailto:sergey.bylok...@oracle.com>
<mailto: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/>
<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.
--
Best regards, Sergey.
