On Fri, 17 Jan 2020 18:40:34 GMT, Kevin Rushforth <k...@openjdk.org> wrote:

>> Everything passes with the fix and 5 of the new tests fail without the fix.
>> 
>> removingThenAddingNodeToDifferentBranchGetsNewFontStyleTest
>> movingBranchToDifferentBranchGetsNewCssVariableTest
>> removingThenAddingNodeToDifferentBranchGetsCorrectInheritedValue
>> removingThenAddingNodeToDifferentBranchGetsIneritableStyle
>> movingNodeToDifferentBranchGetsNewFontStyleTest
>> 
>> I doubt this will cause a significant performance decrease. I think it's 
>> worth it for correctness. The only other thing is that I kept the fix to the 
>> minimum, but technically canReuseHelper now has a side effect. I could try 
>> rewrite some things if necessary, or maybe rename the method? Else leave it 
>> as is?
>> 
>> Originally introduced here: 
>> https://bugs.openjdk.java.net/browse/JDK-8090462/ 
>> https://github.com/openjdk/jfx/commit/834b0d05e7a2a8351403ec4a121eab312d80fd24#diff-9ec098280fa1aeed53c70243347e76ab
> 
> I mentioned this in the JBS bug as well: I suspect that 
> [JDK-8234877](https://bugs.openjdk.java.net/browse/JDK-8234877) has the same 
> root cause as this one. Can you run the HelloCSS test program as described in 
> that bug with your patch from this PR and see if it also fixes that bug?

This will need (at least) two reviewers. I request @aghaisas and @dsgrieve to 
review (I'll review it as well next week, once I'm done with the more urgent 
reviews).

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

PR: https://git.openjdk.java.net/jfx/pull/87

Reply via email to