[EXTERNAL] CssStyleHelper canReuseStyleHelper

[David Grieve]

I'd create a scenegraph 

                 R 
         .-----+-----.
         A               B
   .----+----.
   C            D

Where C and D are Labels. Then I'd set a font style on A and a different font style on B. 
C and D should pick up the font style of A. Then move D to B and see if it still has A's 
font style. 

It may be necessary to have a more deeply nested scene graph. 

The potential issue is that the node that got moved could pick up the old set of styles
if the style helper is not renewed. 

Ultimately, canReuseStyleHelper will return false if the set of styles for the 
new parent (line 110 or so in CssStyleHelper.java) are different. 

> -----Original Message-----
> From: openjfx-dev <openjfx-dev-bounces at openjdk.java.net> On Behalf Of
> Dean Wookey
> Sent: Thursday, January 16, 2020 11:34 AM
> To: openjfx-dev at openjdk.java.net Mailing <openjfx-
> dev at openjdk.java.net>; David Grieve <david.grieve at oracle.com>
> Cc: aghaisas at openjdk.org
> Subject: [EXTERNAL] CssStyleHelper canReuseStyleHelper
> 
> Hi Ajit, David,
> 
> I came accross a potential issue introduced by JDK-8090462 (
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs
> .openjdk.java.net%2Fbrowse%2FJDK-
> 8090462&data=02%7C01%7Cdavid.grieve%40microsoft.com%7C235930d
> fab7f4f7f15ba08d79aa21774%7C72f988bf86f141af91ab2d7cd011db47%7C1%7
> C0%7C637147893881930150&sdata=awsMTPYkp7GmSrYsoy9I6M%2F6uj
> X7thgZhBVY9UKcjpU%3D&reserved=0), (
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fopenjdk%2Fjfx%2Fcommit%2F834b0d05e7a2a8351403ec4a121ea
> b312d80fd24%23diff-
> 9ec098280fa1aeed53c70243347e76ab&data=02%7C01%7Cdavid.grieve%
> 40microsoft.com%7C235930dfab7f4f7f15ba08d79aa21774%7C72f988bf86f141
> af91ab2d7cd011db47%7C1%7C0%7C637147893881930150&sdata=ZhWS
> dmlJ9rLzIkqnohWTacJKPb8FNrG5yApF0fwP8g4%3D&reserved=0).
> The issue is in the canReuseStyleHelper method. canReuseStyleHelper is
> called as a result of changes to the scene graph and for other reasons. The
> original code found the parent helper in the following way:
> 
>         Styleable parent = node.getStyleableParent();
>         while (parent != null) {
>             if (parent instanceof Node) {
>                 parentHelper = ((Node) parent).styleHelper;
>                 if (parentHelper != null) break;
>             }
>             parent = parent.getStyleableParent();
>         }
> 
> This gets the parent helper for the new parent of the node. The code now (
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fopenjdk%2Fjfx%2Fblob%2F20325e1c3ec4c4e81af74d3d43bf3a803
> dbe1a51%2Fmodules%2Fjavafx.graphics%2Fsrc%2Fmain%2Fjava%2Fjavafx%
> 2Fscene%2FCssStyleHelper.java%23L322&data=02%7C01%7Cdavid.griev
> e%40microsoft.com%7C235930dfab7f4f7f15ba08d79aa21774%7C72f988bf86f
> 141af91ab2d7cd011db47%7C1%7C0%7C637147893881930150&sdata=nJR
> 3ocKaIsf6HWNjHtseEbqLbEvbW7%2FzQS1B%2F39GVyI%3D&reserved=
> 0
> ):
> 
>         CssStyleHelper parentHelper =
> getStyleHelper(node.styleHelper.firstStyleableAncestor);
> 
> gets the helper of the previous parent of the node since
> firstStyleableAncestor hasn't been updated to reflect the current state of the
> scene graph yet. This means if you move a node, it could keep its
> CssStyleHelper even if it's under completely new parents.
> 
> I'm not sure if this is actually causing any problems. It would be helpful if I
> knew the kind of things that could go wrong if you reuse a style helper so
> that I could design a test that highlighted the issue.
> 
> Can you perhaps think of cases where this could go badly?
> 
> Dean