RFR: 8263384: IGV: Outline should highlight the Graph that has focus [v2]

[Roberto Castañeda Lozano]

On Thu, 16 Jun 2022 06:42:24 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> Roberto Castañeda Lozano has updated the pull request incrementally with three additional commits since the last revision:
>> 
>>  - Highlight active graph when the Outline window is re-opened
>>  - Avoid unnecessary setting of 'result' to null
>>  - Wait for last graph update before highlighting it
>
> src/utils/IdealGraphVisualizer/Coordinator/src/main/java/com/sun/hotspot/igv/coordinator/OutlineTopComponent.java line 221:
> 
>> 219:     public void resultChanged(LookupEvent lookupEvent) {
>> 220:         // Highlight the focused graph, if available, in the outline.
>> 221:         if (result.allItems().isEmpty()) {
> 
> It looks like you are confident that result is not NULL when resultChanged() is called.  I believe framework calls it after `componentOpened()`.  if so, is it possible to remove result = null in `componentClosed()`?

Right, see e.g. the code listed in https://urldefense.com/v3/__https://netbeans.apache.org/tutorials/nbm-selection-1.html*_creating_a_context_sensitive_topcomponent__;Iw!!ACWV5N9M2RV99hQ!LiR13Y2Xu04_5jtu5YCN05ErEp6Ycm-xoItlMNKfVDkCqSl-udQ49QbUQT-v6xlynX1AkbMGgnDePZWC1S_2pTVneHBaMbcWdq-lLQ$  which follows the same pattern.

`result = null` is just added for consistency with other TopComponent classes in the project, e.g:

https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/b2a58bec4a4f70a06b23013cc4c351b36a413521/src/utils/IdealGraphVisualizer/ControlFlow/src/main/java/com/sun/hotspot/igv/controlflow/ControlFlowTopComponent.java*L134__;Iw!!ACWV5N9M2RV99hQ!LiR13Y2Xu04_5jtu5YCN05ErEp6Ycm-xoItlMNKfVDkCqSl-udQ49QbUQT-v6xlynX1AkbMGgnDePZWC1S_2pTVneHBaMbcVWFwXsg$ 

But you are right, it does not seem necessary and I just removed it.

> src/utils/IdealGraphVisualizer/Coordinator/src/main/java/com/sun/hotspot/igv/coordinator/OutlineTopComponent.java line 229:
> 
>> 227:         }
>> 228:         try {
>> 229:             manager.setSelectedNodes(new GraphNode[]{FolderNode.getGraphNode(p.getGraph())});
> 
> Do we need to consider that FolderNode.getGraphNode() returns null? `setSelectedNodes` will throw `IllegalArgumentException` if input is null?

`FolderNode.getGraphNode()` should never return null, so if that happens I think the best option is to warn the user and log the `IllegalArgumentException` exception thrown by `setSelectedNodes()`, which is what `Exceptions.printStackTrace()` does. See https://bits.netbeans.org/12.6/javadoc/org-openide-explorer/org/openide/explorer/ExplorerManager.html#setSelectedNodes-org.openide.nodes.Node:A- and https://bits.netbeans.org/12.6/javadoc/org-openide-util/org/openide/util/Exceptions.html#printStackTrace-java.lang.Throwable-.

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

PR: https://git.openjdk.org/jdk/pull/9167