RFR: 8237469: CssStyleHelper reuse check fixed

[Dean Wookey]

On Wed, 22 Jan 2020 15:00:14 GMT, David Grieve <dgrieve 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
> 
> modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java line 134:
> 
>> 133:             node.styleHelper.triggerStates.addAll(triggerStates[0]);
>> 134: 
>> 135:             updateParentTriggerStates(node, depth, triggerStates);
> 
> In canReuseStyleHelper, there is this check
>        // If the style maps are the same instance, we can re-use the current styleHelper if the cacheContainer is null.
>         // Under this condition, there are no styles for this node _and_ no styles inherit.
>         if (node.styleHelper.cacheContainer == null) {
>             return true;
>         }
> And later in the same function is a check for (parent == null) that returns true. In both cases, firstStyleableAncestor will still point to the old first-styleable ancestor.

Thanks. For some reason I thought all the early exits out of the method returned false. I'll push a fix.

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

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