JMC-6241: Leaking Context Menu items
Ken Dobson
kdobson at redhat.com
Tue Dec 4 20:43:46 UTC 2018
Hi all,
This patch does a couple things.
1. It fixes the leaking context menu items which resulted in the context
menu having multiple sets of the same thread activity lanes listed.
2. It adds a context menu to the Garbage Collections Page legend as there
already was one for the Java Application page.
3. Fixes a small bug that lead to the Thread Activity lane on the GC page
only refreshing if it was turned off and then back on again.
If someone could give it a review that would be great.
https://bugs.openjdk.java.net/browse/JMC-6241
Thanks,
Ken Dobson
diff -r 4629e44fd8ea
application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/common/ThreadGraphLanes.java
---
a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/common/ThreadGraphLanes.java
Thu Nov 08 12:45:05 2018 +0100
+++
b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/common/ThreadGraphLanes.java
Tue Dec 04 15:08:07 2018 -0500
@@ -99,14 +99,14 @@
this.actions = new ArrayList<>();
}
- public void openEditLanesDialog(MCContextMenuManager mm) {
+ public void openEditLanesDialog(MCContextMenuManager mm, boolean
isLegendMenu) {
// FIXME: Might there be other interesting events that don't
really have duration?
EventTypeFolderNode typeTree =
dataSourceSupplier.get().getTypeTree(ItemCollectionToolkit
.stream(dataSourceSupplier.get().getItems()).filter(this::typeWithThreadAndDuration));
laneDefs = LaneEditor.openDialog(typeTree,
laneDefs.stream().collect(Collectors.toList()),
Messages.JavaApplicationPage_EDIT_THREAD_LANES_DIALOG_TITLE,
Messages.JavaApplicationPage_EDIT_THREAD_LANES_DIALOG_MESSAGE);
- updateContextMenu(mm);
+ updateContextMenu(mm, isLegendMenu);
buildChart.run();
}
@@ -214,16 +214,26 @@
naLanes = lanesByApplicability.get(false);
return Collections.emptyList();
}
-
- private List<String> actionIdentifiers = new ArrayList<>();
+
+ //create two action identifiers to handle the chart context menu and
the legend context menu
+ private List<String> chartActionIdentifiers = new ArrayList<>();
+ private List<String> legendActionIdentifiers = new ArrayList<>();
- public void updateContextMenu(MCContextMenuManager mm) {
- for (String id : actionIdentifiers) {
- mm.remove(id);
+ public void updateContextMenu(MCContextMenuManager mm, boolean
isLegendMenu) {
+
+ if(isLegendMenu) {
+ for (String id : legendActionIdentifiers) {
+ mm.remove(id);
+ }
+ legendActionIdentifiers.clear();
+ } else {
+ for (String id : chartActionIdentifiers) {
+ mm.remove(id);
+ }
+ chartActionIdentifiers.clear();
}
- actionIdentifiers.clear();
if (mm.indexOf(EDIT_LANES) == -1) {
- IAction action = ActionToolkit.action(() ->
this.openEditLanesDialog(mm),
+ IAction action = ActionToolkit.action(() ->
this.openEditLanesDialog(mm, isLegendMenu),
Messages.JavaApplicationPage_EDIT_THREAD_LANES_ACTION,
FlightRecorderUI.getDefault().getMCImageDescriptor(ImageConstants.ICON_LANES_EDIT));
action.setId(EDIT_LANES);
@@ -245,7 +255,11 @@
};
String identifier = ld.getName() + checkAction.hashCode();
checkAction.setId(identifier);
- actionIdentifiers.add(identifier);
+ if (isLegendMenu) {
+ legendActionIdentifiers.add(identifier);
+ } else {
+ chartActionIdentifiers.add(identifier);
+ }
checkAction.setChecked(ld.isEnabled());
// FIXME: Add a tooltip here
mm.add(checkAction);
diff -r 4629e44fd8ea
application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/GarbageCollectionsPage.java
---
a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/GarbageCollectionsPage.java
Thu Nov 08 12:45:05 2018 +0100
+++
b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/GarbageCollectionsPage.java
Tue Dec 04 15:08:07 2018 -0500
@@ -54,6 +54,7 @@
import org.eclipse.jface.action.IAction;
import org.eclipse.jface.resource.ImageDescriptor;
import org.eclipse.jface.viewers.ArrayContentProvider;
+import org.eclipse.jface.viewers.CheckboxTableViewer;
import org.eclipse.jface.viewers.ColumnViewerToolTipSupport;
import org.eclipse.jface.viewers.IStructuredSelection;
import org.eclipse.jface.viewers.TableViewer;
@@ -335,8 +336,10 @@
gcInfoFolder = new CTabFolder(tableSash, SWT.NONE);
phasesList = PHASES.buildWithoutBorder(gcInfoFolder,
TableSettings.forState(state.getChild(PHASE_LIST)));
-
phasesList.getManager().getViewer().addSelectionChangedListener(
- e ->
pageContainer.showSelection(ItemCollectionToolkit.build(phasesList.getSelection().get())));
+
phasesList.getManager().getViewer().addSelectionChangedListener(e -> {
+ buildChart();
+
pageContainer.showSelection(ItemCollectionToolkit.build(phasesList.getSelection().get()));
+ });
phasesFilter =
FilterComponent.createFilterComponent(phasesList, phasesFilterState,
getDataSource().getItems().apply(JdkFilters.GC_PAUSE_PHASE),
pageContainer.getSelectionStore()::getSelections,
this::onPhasesFilterChange);
@@ -372,14 +375,14 @@
chartCanvas = new ChartCanvas(chartContainer);
chartCanvas.setLayoutData(new GridData(SWT.FILL, SWT.FILL,
true, true));
ActionToolkit.loadCheckState(state.getChild(CHART),
allChartSeriesActions.stream());
- Control chartLegend =
ActionUiToolkit.buildCheckboxControl(chartContainer,
- allChartSeriesActions.stream().filter(a ->
includeAttribute(a.getId())), true);
+ CheckboxTableViewer chartLegend =
ActionUiToolkit.buildCheckboxViewer(chartContainer,
+ allChartSeriesActions.stream().filter(a ->
includeAttribute(a.getId())));
GridData gd = new GridData(SWT.FILL, SWT.FILL, false, true);
gd.widthHint = 180;
- chartLegend.setLayoutData(gd);
+ chartLegend.getControl().setLayoutData(gd);
lanes = new ThreadGraphLanes(() -> getDataSource(), () ->
buildChart());
lanes.initializeChartConfiguration(Stream.of(state.getChildren(THREAD_LANES)));
- IAction editLanesAction = ActionToolkit.action(() ->
lanes.openEditLanesDialog(mm),
+ IAction editLanesAction = ActionToolkit.action(() ->
lanes.openEditLanesDialog(mm, false),
Messages.ThreadsPage_EDIT_LANES,
FlightRecorderUI.getDefault().getMCImageDescriptor(ImageConstants.ICON_LANES_EDIT));
form.getToolBarManager().add(editLanesAction);
@@ -404,7 +407,8 @@
phasesList.getManager().setSelectionState(phasesSelection);
metaspaceList.getManager().setSelectionState(metaspaceSelection);
mm = (MCContextMenuManager) chartCanvas.getContextMenu();
- lanes.updateContextMenu(mm);
+ lanes.updateContextMenu(mm, false);
+
lanes.updateContextMenu(MCContextMenuManager.create(chartLegend.getControl()),
true);
// Older recordings may not have thread information in pause
events.
// In those cases there is no need for the thread activity
actions.
diff -r 4629e44fd8ea
application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/JavaApplicationPage.java
---
a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/JavaApplicationPage.java
Thu Nov 08 12:45:05 2018 +0100
+++
b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/JavaApplicationPage.java
Tue Dec 04 15:08:07 2018 -0500
@@ -224,14 +224,14 @@
mm = (MCContextMenuManager) chartCanvas.getContextMenu();
// FIXME: The lanes field is initialized by
initializeChartConfiguration which is called by the super constructor. This
is too indirect for SpotBugs to resolve and should be simplified.
- lanes.updateContextMenu(mm);
-
lanes.updateContextMenu(MCContextMenuManager.create(chartLegend.getControl()));
+ lanes.updateContextMenu(mm, false);
+
lanes.updateContextMenu(MCContextMenuManager.create(chartLegend.getControl()),
true);
buildChart();
addResultActions(form);
tableFilterComponent.loadState(state.getChild(METHOD_PROFILING_TABLE_FILTER));
form.getToolBarManager()
- .add(ActionToolkit.action(() ->
lanes.openEditLanesDialog(mm), Messages.ThreadsPage_EDIT_LANES,
+ .add(ActionToolkit.action(() ->
lanes.openEditLanesDialog(mm, false), Messages.ThreadsPage_EDIT_LANES,
FlightRecorderUI.getDefault().getMCImageDescriptor(ImageConstants.ICON_LANES_EDIT)));
form.getToolBarManager().add(new Separator());
OrientationAction.installActions(form, sash);
diff -r 4629e44fd8ea
application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/ThreadsPage.java
---
a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/ThreadsPage.java
Thu Nov 08 12:45:05 2018 +0100
+++
b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/ThreadsPage.java
Tue Dec 04 15:08:07 2018 -0500
@@ -162,10 +162,10 @@
sash.setOrientation(SWT.HORIZONTAL);
mm.add(new Separator());
// FIXME: The lanes field is initialized by
initializeChartConfiguration which is called by the super constructor. This
is too indirect for SpotBugs to resolve and should be simplified.
- lanes.updateContextMenu(mm);
+ lanes.updateContextMenu(mm, false);
form.getToolBarManager()
- .add(ActionToolkit.action(() ->
lanes.openEditLanesDialog(mm), Messages.ThreadsPage_EDIT_LANES,
+ .add(ActionToolkit.action(() ->
lanes.openEditLanesDialog(mm, false), Messages.ThreadsPage_EDIT_LANES,
FlightRecorderUI.getDefault().getMCImageDescriptor(ImageConstants.ICON_LANES_EDIT)));
form.getToolBarManager().update(true);
chartLegend.getControl().dispose();
More information about the jmc-dev
mailing list