JDK-8173796 : Overriden method, requestLayout(), by subclass of Parent didn't get call when process layout
  • Type: Bug
  • Component: javafx
  • Sub-Component: scenegraph
  • Affected Version: 9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2017-02-02
  • Updated: 2017-02-09
  • Resolved: 2017-02-09
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9Fixed
Related Reports
Relates :  
Relates :  
Relates :  
Description
The fix to JDK-8137252 has introduced an unintended side effect (bug) that causes subclasses of Parent no longer able to override requestLayout() and expect proper behavior. We have a number of classes such as HBox, VBox, StackPane, GridPane and TextFlow override requestLayout() with extra logic.
Comments
Changeset: eef2a50f5bdd Author: ckyang Date: 2017-02-09 13:44 -0800 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/eef2a50f5bdd
09-02-2017

I think this change should be performance neutral (or nearly so). Prior to the refactoring in the proposed patch for this bug we were skipping this overridden requestLayout method entirely, and forcing a layout, in those cases where the short-circuit would have taken effect. So really, the performance concern, which is a valid one, was already present after the fix for JDK-8137252. After the refactoring for this bug (JDK-8173796), the short-circuit needed to be removed from StackPane (and VBox and Hbox) to avoid a functional regression that would have reintroduced JDK-8137252. Still, as Jonathan mentioned, we need to keep eyes open to make sure there isn't some performance hit that we don't anticipate. +1
09-02-2017

This is one of those patches that worries me about going into the repo at this stage of development. In particular, I worry about the implications of removing the code from StackPane, etc - whether we will hit performance or functional regressions (most probably performance ones though - it seems feasible that more layouts may be requested now with this change). Having said that, I don't see any issues with the proposed changeset, it's more that we just need to be vigilant in the next few weeks to keep our finger on the pulse of performance issues, and to keep this changeset in mind. +1
08-02-2017

Please review the proposed fix. The change in Parent is to fix what this bug has reported. However to fix it correctly without introducing regression to the fix to JDK-8137252, I need to fix a flaw in StackPane's layout optimization logic. Similar fix also applied to VBox and HBox. GridPane has the same logic flaw but because its logic is more complex I will file a separate JIRA to fix it separately. http://cr.openjdk.java.net/~ckyang/JDK-8173796/webrev.00/
08-02-2017