RFR: 8237469: CssStyleHelper reuse check fixed

Kevin Rushforth kcr at openjdk.java.net
Fri Jan 17 18:42:45 UTC 2020

On Fri, 17 Jan 2020 18:40:34 GMT, Kevin Rushforth <kcr at 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

More information about the openjfx-dev mailing list