tmf: Fix Marker Set action handling

When changing the active marker set, it didn't update the marker sources
when the opened trace is an experiment. The action only updated the
marker sources from the opened traces, but the marker sets were created
on each trace in the experiment's trace set, for the default
implementation of AbstractTimeGraphView.getTracesToBuild().

It wouldn't be good enough to update the traces using
TmfTraceManager.getTraceSet(), since we don't know the implementation of
getTracesToBuild() for each view. Also, calling
TmfTraceAdapterManager.getAdapters() on each opened trace could actually
create new adapter instances that weren't yet needed.

Instead the ConfigurableMarkerEventSource instances should handle the
TmfMarkerEventSourceUpdatedSignal, and reconfigure themselves.

Also, since the Marker Set menu items are radio, selecting a new marker
set first executes the action on the menu item that is being unchecked.
The action now only executes for the menu item that is being checked.

Change-Id: Ic8fd21b930ec29e8a619cbcedea712d4b7ee4d86
Signed-off-by: Patrick Tasse <patrick.tasse@gmail.com>
Reviewed-on: https://git.eclipse.org/r/93463
Reviewed-by: Hudson CI
Reviewed-by: Bernd Hufmann <bernd.hufmann@ericsson.com>
Tested-by: Bernd Hufmann <bernd.hufmann@ericsson.com>

diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSource.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSource.java
index 8da331cd37896d76268d61a649dd3825a1473b68..7497a533774d4b3f8a637858967dc344ceb344ae 100644 (file)
--- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSource.java
+++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSource.java
@@ -37,6 +37,10 @@ import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerSet;
 import org.eclipse.tracecompass.internal.tmf.core.markers.SubMarker;
 import org.eclipse.tracecompass.internal.tmf.core.markers.SubMarker.SplitMarker;
 import org.eclipse.tracecompass.internal.tmf.core.markers.SubMarker.WeightedMarker;
+import org.eclipse.tracecompass.tmf.core.signal.TmfMarkerEventSourceUpdatedSignal;
+import org.eclipse.tracecompass.tmf.core.signal.TmfSignalHandler;
+import org.eclipse.tracecompass.tmf.core.signal.TmfSignalManager;
+import org.eclipse.tracecompass.tmf.core.trace.AbstractTmfTraceAdapterFactory.IDisposableAdapter;
 import org.eclipse.tracecompass.tmf.core.trace.ICyclesConverter;
 import org.eclipse.tracecompass.tmf.core.trace.ITmfTrace;
 import org.eclipse.tracecompass.tmf.ui.colors.X11Color;
@@ -53,7 +57,7 @@ import com.google.common.collect.RangeSet;
 /**
  * Configurable marker event source.
  */
-public class ConfigurableMarkerEventSource implements IMarkerEventSource {
+public class ConfigurableMarkerEventSource implements IMarkerEventSource, IDisposableAdapter {
 
     private static final long NANO_PER_MILLI = 1000000L;
     private static final long NANO_PER_MICRO = 1000L;
@@ -74,6 +78,12 @@ public class ConfigurableMarkerEventSource implements IMarkerEventSource {
     public ConfigurableMarkerEventSource(ITmfTrace trace) {
         fMarkerEventSources = new ArrayList<>();
         fTrace = trace;
+        TmfSignalManager.register(this);
+    }
+
+    @Override
+    public void dispose() {
+        TmfSignalManager.deregister(this);
     }
 
     /**
@@ -318,4 +328,15 @@ public class ConfigurableMarkerEventSource implements IMarkerEventSource {
             return fMarker.getSubMarkers();
         }
     }
+
+    /**
+     * A marker event source has been updated
+     *
+     * @param signal
+     *            the signal
+     */
+    @TmfSignalHandler
+    public void markerEventSourceUpdated(final TmfMarkerEventSourceUpdatedSignal signal) {
+        configure(MarkerUtils.getDefaultMarkerSet());
+    }
 }

diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSourceFactory.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSourceFactory.java
index 9484a764f94e98a22403705b27c009540faff4e1..179fbd414e49fc799730e0ac8081e7bafe425676 100644 (file)
--- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSourceFactory.java
+++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/ConfigurableMarkerEventSourceFactory.java
@@ -12,8 +12,6 @@
 
 package org.eclipse.tracecompass.internal.tmf.ui.markers;
 
-import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerConfigXmlParser;
-import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerSet;
 import org.eclipse.tracecompass.tmf.core.trace.AbstractTmfTraceAdapterFactory;
 import org.eclipse.tracecompass.tmf.core.trace.ITmfTrace;
 import org.eclipse.tracecompass.tmf.ui.widgets.timegraph.model.IMarkerEventSource;
@@ -27,7 +25,7 @@ public class ConfigurableMarkerEventSourceFactory extends AbstractTmfTraceAdapte
     protected <T> T getTraceAdapter(ITmfTrace trace, Class<T> adapterType) {
         if (IMarkerEventSource.class.equals(adapterType)) {
             ConfigurableMarkerEventSource adapter = new ConfigurableMarkerEventSource(trace);
-            configure(adapter);
+            adapter.configure(MarkerUtils.getDefaultMarkerSet());
             return adapterType.cast(adapter);
         }
         return null;
@@ -39,13 +37,4 @@ public class ConfigurableMarkerEventSourceFactory extends AbstractTmfTraceAdapte
                 IMarkerEventSource.class
         };
     }
-
-    private static void configure(ConfigurableMarkerEventSource source) {
-        String defaultMarkerSetId = MarkerUtils.getDefaultMarkerSetId();
-        for (MarkerSet markerSet : MarkerConfigXmlParser.getMarkerSets()) {
-            if (markerSet.getId().equals(defaultMarkerSetId)) {
-                source.configure(markerSet);
-            }
-        }
-    }
 }

diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/MarkerUtils.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/MarkerUtils.java
index e6873e5f2a38d4b1728d20bad104b7189f96a95b..616b95eb2734f4905b94e079248560f90fa9fcae 100644 (file)
--- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/MarkerUtils.java
+++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/markers/MarkerUtils.java
@@ -12,7 +12,10 @@
 
 package org.eclipse.tracecompass.internal.tmf.ui.markers;
 
+import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.jface.dialogs.IDialogSettings;
+import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerConfigXmlParser;
+import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerSet;
 import org.eclipse.tracecompass.internal.tmf.ui.Activator;
 
 /**
@@ -22,22 +25,38 @@ public class MarkerUtils {
 
     private static final String MARKER_SET_KEY = "marker.set"; //$NON-NLS-1$
 
+    private static MarkerSet fDefaultMarkerSet = null;
+
     /**
-     * Get the default marker set id
+     * Get the default marker set
      *
-     * @return the default marker set id, or null if none is set
+     * @return the default marker set, or null if none is set
      */
-    public static String getDefaultMarkerSetId() {
-        return getDialogSettings().get(MARKER_SET_KEY);
+    public static synchronized @Nullable MarkerSet getDefaultMarkerSet() {
+        String id = getDialogSettings().get(MARKER_SET_KEY);
+        if (id == null) {
+            fDefaultMarkerSet = null;
+        } else {
+            if (fDefaultMarkerSet == null || !fDefaultMarkerSet.getId().equals(id)) {
+                for (MarkerSet markerSet : MarkerConfigXmlParser.getMarkerSets()) {
+                    if (markerSet.getId().equals(id)) {
+                        fDefaultMarkerSet = markerSet;
+                    }
+                }
+            }
+        }
+        return fDefaultMarkerSet;
     }
 
     /**
-     * Set the default marker set id
+     * Set the default marker set
      *
-     * @param id
-     *            the default marker set id, or null to set none
+     * @param markerSet
+     *            the default marker set, or null to set none
      */
-    public static void setDefaultMarkerSetId(String id) {
+    public static synchronized void setDefaultMarkerSet(@Nullable MarkerSet markerSet) {
+        fDefaultMarkerSet = markerSet;
+        String id = (markerSet == null) ? null : markerSet.getId();
         getDialogSettings().put(MARKER_SET_KEY, id);
     }
 

diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractTimeGraphView.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractTimeGraphView.java
index fcf7ef4064fdb20880f405fd8888c145300c1f7c..50f40a5cfeabb914821134f418af663a4b7351f8 100644 (file)
--- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractTimeGraphView.java
+++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractTimeGraphView.java
@@ -87,7 +87,6 @@ import org.eclipse.tracecompass.common.core.log.TraceCompassLog;
 import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerConfigXmlParser;
 import org.eclipse.tracecompass.internal.tmf.core.markers.MarkerSet;
 import org.eclipse.tracecompass.internal.tmf.ui.Activator;
-import org.eclipse.tracecompass.internal.tmf.ui.markers.ConfigurableMarkerEventSource;
 import org.eclipse.tracecompass.internal.tmf.ui.markers.MarkerUtils;
 import org.eclipse.tracecompass.tmf.core.resources.ITmfMarker;
 import org.eclipse.tracecompass.tmf.core.signal.TmfMarkerEventSourceUpdatedSignal;
@@ -1707,15 +1706,10 @@ public abstract class AbstractTimeGraphView extends TmfView implements ITmfTimeA
 
         @Override
         public void runWithEvent(Event event) {
-            MarkerUtils.setDefaultMarkerSetId(fMarkerSet == null ? null : fMarkerSet.getId());
-            for (ITmfTrace trace : TmfTraceManager.getInstance().getOpenedTraces()) {
-                for (IMarkerEventSource source : TmfTraceAdapterManager.getAdapters(trace, IMarkerEventSource.class)) {
-                    if (source instanceof ConfigurableMarkerEventSource) {
-                        ((ConfigurableMarkerEventSource) source).configure(fMarkerSet);
-                    }
-                }
+            if (isChecked()) {
+                MarkerUtils.setDefaultMarkerSet(fMarkerSet);
+                broadcast(new TmfMarkerEventSourceUpdatedSignal(AbstractTimeGraphView.this));
             }
-            broadcast(new TmfMarkerEventSourceUpdatedSignal(AbstractTimeGraphView.this));
         }
     }
 
@@ -1734,13 +1728,14 @@ public abstract class AbstractTimeGraphView extends TmfView implements ITmfTimeA
         fMarkerSetMenu.addMenuListener(new IMenuListener() {
             @Override
             public void menuAboutToShow(IMenuManager mgr) {
-                Action action = new MarkerSetAction(null);
-                String defaultMarkerSetId = MarkerUtils.getDefaultMarkerSetId();
-                action.setChecked(defaultMarkerSetId == null || defaultMarkerSetId.isEmpty());
-                mgr.add(action);
+                Action noneAction = new MarkerSetAction(null);
+                MarkerSet defaultMarkerSet = MarkerUtils.getDefaultMarkerSet();
+                String defaultMarkerSetId = (defaultMarkerSet == null) ? null : defaultMarkerSet.getId();
+                noneAction.setChecked(defaultMarkerSetId == null);
+                mgr.add(noneAction);
                 List<MarkerSet> markerSets = MarkerConfigXmlParser.getMarkerSets();
                 for (MarkerSet markerSet : markerSets) {
-                    action = new MarkerSetAction(markerSet);
+                    Action action = new MarkerSetAction(markerSet);
                     action.setChecked(markerSet.getId().equals(defaultMarkerSetId));
                     mgr.add(action);
                 }