Bug 448968 - Add diagnostic logging to help identify the root cause.

Signed-off-by: Stefan Xenos <sxenos@gmail.com>
Change-Id: I390e9c5a7d9357d9e27dcc7048ec9568efef8975

7 files changed, 132 insertions, 10 deletions

diff --git a/bundles/org.eclipse.compare/.options b/bundles/org.eclipse.compare/.options
new file mode 100644
index 000000000..63b49716d
--- /dev/null
+++ b/bundles/org.eclipse.compare/.options
@@ -0,0 +1,7 @@
+# Debugging options for the org.eclipse.compare plugin.
+
+# Turn on debugging for the org.eclipse.compare plugin.
+org.eclipse.compare/debug=false
+
+# Turn on debugging for the ContentMergeViewer
+org.eclipse.compare/content_merge_viewer=false
diff --git a/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/ContentMergeViewer.java b/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/ContentMergeViewer.java
index 1958bf4eb..837b89c14 100644
--- a/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/ContentMergeViewer.java
+++ b/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/ContentMergeViewer.java
@@ -8,6 +8,7 @@
  * Contributors:
  *     IBM Corporation - initial API and implementation
  *     Alex Blewitt <alex.blewitt@gmail.com> - replace new Boolean with Boolean.valueOf - https://bugs.eclipse.org/470344
+ *     Stefan Xenos <sxenos@gmail.com> (Google) - bug 448968 - Add diagnostic logging
  *******************************************************************************/
 
 package org.eclipse.compare.contentmergeviewer;
@@ -30,6 +31,7 @@ import org.eclipse.compare.internal.ICompareUIConstants;
 import org.eclipse.compare.internal.IFlushable2;
 import org.eclipse.compare.internal.ISavingSaveable;
 import org.eclipse.compare.internal.MergeViewerContentProvider;
+import org.eclipse.compare.internal.Policy;
 import org.eclipse.compare.internal.Utilities;
 import org.eclipse.compare.internal.ViewerSwitchingCancelled;
 import org.eclipse.compare.structuremergeviewer.Differencer;
@@ -121,11 +123,19 @@ public abstract class ContentMergeViewer extends ContentViewer
 		public void layout(Composite composite, boolean force) {
 			
 			if (fLeftLabel == null) {
+				if (composite.isDisposed()) {
+					CompareUIPlugin.log(new IllegalArgumentException("Attempted to perform a layout on a disposed composite")); //$NON-NLS-1$
+				}
+				if (Policy.debugContentMergeViewer) {
+					logTrace("found bad label. fComposite.isDisposed() = " + fComposite.isDisposed() //$NON-NLS-1$//$NON-NLS-2$
+							+ composite.isDisposed());
+					logStackTrace();
+				}
 				// Help to find out the cause for bug 449558
-				NullPointerException npe= new NullPointerException("fLeftLabel is 'null';fLeftLabelSet is " + fLeftLabelSet + ";fComposite.isDisposed() is " + fComposite.isDisposed());
+				NullPointerException npe= new NullPointerException("fLeftLabel is 'null';fLeftLabelSet is " + fLeftLabelSet + ";fComposite.isDisposed() is " + fComposite.isDisposed()); //$NON-NLS-1$ //$NON-NLS-2$
 
 				// Allow to test whether doing nothing helps
-				if (Boolean.getBoolean("ContentMergeViewer.DEBUG")) {
+				if (Boolean.getBoolean("ContentMergeViewer.DEBUG")) { //$NON-NLS-1$
 					CompareUIPlugin.log(npe);
 					return;
 				}
@@ -354,6 +364,11 @@ public abstract class ContentMergeViewer extends ContentViewer
 	 */
 	protected ContentMergeViewer(int style, ResourceBundle bundle, CompareConfiguration cc) {
 		
+		if (Policy.debugContentMergeViewer) {
+			logTrace("constructed (fLeftLabel == null)"); //$NON-NLS-1$
+			logStackTrace();
+		}
+
 		fStyles= style & ~(SWT.LEFT_TO_RIGHT | SWT.RIGHT_TO_LEFT);	// remove BIDI direction bits
 		fBundle= bundle;
 		
@@ -375,7 +390,7 @@ public abstract class ContentMergeViewer extends ContentViewer
 			fCompareConfiguration = new CompareConfiguration();
 		else
 			fCompareConfiguration= cc;
-		fPropertyChangeListener= new IPropertyChangeListener() {
+			fPropertyChangeListener= new IPropertyChangeListener() {
 			public void propertyChange(PropertyChangeEvent event) {
 				ContentMergeViewer.this.handlePropertyChangeEvent(event);
 			}
@@ -386,8 +401,17 @@ public abstract class ContentMergeViewer extends ContentViewer
 		fIsRightDirty = false;
 	}
 
-	//---- hooks ---------------------
+	private void logStackTrace() {
+		new Exception("<Fake exception> in " + getClass().getName()).printStackTrace(System.out); //$NON-NLS-1$
+	}
+
+	private void logTrace(String string) {
+		System.out.println("ContentMergeViewer " + System.identityHashCode(this) + ": " + string);   //$NON-NLS-1$//$NON-NLS-2$
+	}
 	
+	//---- hooks ---------------------
+
+
 	/**
 	 * Returns the viewer's name.
 	 *
@@ -801,8 +825,13 @@ public abstract class ContentMergeViewer extends ContentViewer
 		
 		int style= SWT.SHADOW_OUT;
 		fAncestorLabel= new CLabel(fComposite, style | Window.getDefaultOrientation());
-		
+
 		fLeftLabel= new CLabel(fComposite, style | Window.getDefaultOrientation());
+		if (Policy.debugContentMergeViewer) {
+			logTrace("fLeftLabel initialized");
+			logStackTrace();
+		}
+		
 		fLeftLabelSet= true;
 		new Resizer(fLeftLabel, VERTICAL);
 		
@@ -982,6 +1011,10 @@ public abstract class ContentMergeViewer extends ContentViewer
 
 		fAncestorLabel= null;
 		fLeftLabel= null;
+		if (Policy.debugContentMergeViewer) {
+			logTrace("handleDispose(...) - fLeftLabel = null");
+			logStackTrace();
+		}
 		fDirectionLabel= null;
 		fRightLabel= null;
 		fCenter= null;
diff --git a/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/CompareUIPlugin.java b/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/CompareUIPlugin.java
index 80136c899..2138a38ce 100644
--- a/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/CompareUIPlugin.java
+++ b/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/CompareUIPlugin.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2014 IBM Corporation and others.
+ * Copyright (c) 2000, 2015 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -8,6 +8,7 @@
  * Contributors:
  *     IBM Corporation - initial API and implementation
  *     Carsten Pfeiffer <carsten.pfeiffer@gebit.de> - CompareUIPlugin.getCommonType() returns null if left or right side is not available - https://bugs.eclipse.org/311843
+ *     Stefan Xenos <sxenos@gmail.com> (Google) - bug 448968 - Add diagnostic logging
  *******************************************************************************/
 package org.eclipse.compare.internal;
 
@@ -65,6 +66,8 @@ import org.eclipse.jface.resource.ImageDescriptor;
 import org.eclipse.jface.util.IPropertyChangeListener;
 import org.eclipse.jface.util.PropertyChangeEvent;
 import org.eclipse.jface.viewers.Viewer;
+import org.eclipse.osgi.service.debug.DebugOptions;
+import org.eclipse.osgi.service.debug.DebugOptionsListener;
 import org.eclipse.swt.graphics.Image;
 import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Display;
@@ -82,6 +85,7 @@ import org.eclipse.ui.PlatformUI;
 import org.eclipse.ui.model.IWorkbenchAdapter;
 import org.eclipse.ui.plugin.AbstractUIPlugin;
 import org.osgi.framework.BundleContext;
+import org.osgi.framework.ServiceRegistration;
 
 /**
  * The Compare UI plug-in defines the entry point to initiate a configurable
@@ -253,6 +257,8 @@ public final class CompareUIPlugin extends AbstractUIPlugin {
 	private CompareResourceFilter fFilter;
 	private IPropertyChangeListener fPropertyChangeListener;
 
+	private ServiceRegistration debugRegistration;
+
 	/**
 	 * Creates the <code>CompareUIPlugin</code> object and registers all
 	 * structure creators, content merge viewers, and structure merge viewers
@@ -270,6 +276,11 @@ public final class CompareUIPlugin extends AbstractUIPlugin {
 	public void start(BundleContext context) throws Exception {
 		super.start(context);
 
+		Hashtable properties = new Hashtable(2);
+		properties.put(DebugOptions.LISTENER_SYMBOLICNAME, PLUGIN_ID);
+		debugRegistration = context.registerService(DebugOptionsListener.class, Policy.DEBUG_OPTIONS_LISTENER,
+				properties);
+
 		ComparePlugin.getDefault().setCappingDisabled(
 				getPreferenceStore().getBoolean(
 						ComparePreferencePage.CAPPING_DISABLED));
@@ -295,6 +306,11 @@ public final class CompareUIPlugin extends AbstractUIPlugin {
 			}
 			fgImages= null;
 		}
+		
+		if (debugRegistration != null) {
+			debugRegistration.unregister();
+			debugRegistration = null;
+		}
 	}
 		
 	/**
diff --git a/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/Policy.java b/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/Policy.java
new file mode 100644
index 000000000..d51bff8b5
--- /dev/null
+++ b/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/Policy.java
@@ -0,0 +1,30 @@
+/*******************************************************************************
+ * Copyright (c) 2015 Google, Inc and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Stefan Xenos <sxenos@gmail.com> (Google) - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.compare.internal;
+
+import org.eclipse.osgi.service.debug.DebugOptions;
+import org.eclipse.osgi.service.debug.DebugOptionsListener;
+
+/**
+ * Policy implements NLS convenience methods for the plugin and
+ * makes progress monitor policy decisions
+ */
+public class Policy {
+	// Debug flags
+	public static boolean debugContentMergeViewer = false;
+
+	static final DebugOptionsListener DEBUG_OPTIONS_LISTENER = new DebugOptionsListener() {
+		public void optionsChanged(DebugOptions options) {
+			boolean DEBUG = options.getBooleanOption(CompareUIPlugin.PLUGIN_ID + "/debug", false); //$NON-NLS-1$
+			debugContentMergeViewer = DEBUG && options.getBooleanOption(CompareUIPlugin.PLUGIN_ID + "/content_merge_viewer", false); //$NON-NLS-1$
+		}
+	};
+}
diff --git a/bundles/org.eclipse.team.ui/.options b/bundles/org.eclipse.team.ui/.options
index 1128797ae..529d71773 100644
--- a/bundles/org.eclipse.team.ui/.options
+++ b/bundles/org.eclipse.team.ui/.options
@@ -11,3 +11,6 @@ org.eclipse.team.ui/history=false
 
 # Shows DnD related debugging information
 org.eclipse.team.ui/dnd=false
+
+# Shows information about the compare editor input lifecycle
+org.eclipse.team.ui/compare_editor_input=false
\ No newline at end of file
diff --git a/bundles/org.eclipse.team.ui/src/org/eclipse/team/internal/ui/Policy.java b/bundles/org.eclipse.team.ui/src/org/eclipse/team/internal/ui/Policy.java
index 54ee5e66f..2fd168c6e 100644
--- a/bundles/org.eclipse.team.ui/src/org/eclipse/team/internal/ui/Policy.java
+++ b/bundles/org.eclipse.team.ui/src/org/eclipse/team/internal/ui/Policy.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2014 IBM Corporation and others.
+ * Copyright (c) 2000, 2015 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -7,6 +7,7 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Stefan Xenos <sxenos@gmail.com> (Google) - bug 448968 - Add diagnostic logging
  *******************************************************************************/
 package org.eclipse.team.internal.ui;
 
@@ -25,6 +26,7 @@ public class Policy {
 	public static boolean DEBUG_SYNC_MODELS = false;
 	public static boolean DEBUG_HISTORY = false;
 	public static boolean DEBUG_DND = false;
+	public static boolean DEBUG_COMPARE_EDITOR_INPUT = false;
     
     private static String ACTION_BUNDLE = "org.eclipse.team.internal.ui.actions.actions"; //$NON-NLS-1$
     private static ResourceBundle actionBundle = null;
@@ -46,6 +48,7 @@ public class Policy {
 			DEBUG_SYNC_MODELS = DEBUG && options.getBooleanOption(TeamUIPlugin.ID + "/syncmodels", false); //$NON-NLS-1$
 			DEBUG_HISTORY = DEBUG && options.getBooleanOption(TeamUIPlugin.ID + "/history", false); //$NON-NLS-1$
 			DEBUG_DND = DEBUG && options.getBooleanOption(TeamUIPlugin.ID + "/dnd", false); //$NON-NLS-1$
+			DEBUG_COMPARE_EDITOR_INPUT = DEBUG && options.getBooleanOption(TeamUIPlugin.ID + "/compare_editor_input", false); //$NON-NLS-1$
 		}
 	};
 
diff --git a/bundles/org.eclipse.team.ui/src/org/eclipse/team/ui/synchronize/SaveableCompareEditorInput.java b/bundles/org.eclipse.team.ui/src/org/eclipse/team/ui/synchronize/SaveableCompareEditorInput.java
index e1442fb47..a1c689d00 100644
--- a/bundles/org.eclipse.team.ui/src/org/eclipse/team/ui/synchronize/SaveableCompareEditorInput.java
+++ b/bundles/org.eclipse.team.ui/src/org/eclipse/team/ui/synchronize/SaveableCompareEditorInput.java
@@ -1,12 +1,13 @@
 /*******************************************************************************
- * Copyright (c) 2006, 2011 IBM Corporation and others.
+ * Copyright (c) 2006, 2015 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
  * http://www.eclipse.org/legal/epl-v10.html
  *
  * Contributors:
- * IBM Corporation - initial API and implementation
+ *     IBM Corporation - initial API and implementation
+ *     Stefan Xenos <sxenos@gmail.com> (Google) - bug 448968 - Add diagnostic logging
  *******************************************************************************/
 package org.eclipse.team.ui.synchronize;
 
@@ -152,6 +153,9 @@ public abstract class SaveableCompareEditorInput extends CompareEditorInput impl
 	public SaveableCompareEditorInput(CompareConfiguration configuration, IWorkbenchPage page) {
 		super(configuration);
 		this.page = page;
+		if (Policy.DEBUG_COMPARE_EDITOR_INPUT) {
+			logTrace("constructed"); //$NON-NLS-1$
+		}
 	}
 
 	/* (non-Javadoc)
@@ -159,6 +163,10 @@ public abstract class SaveableCompareEditorInput extends CompareEditorInput impl
 	 */
 	protected void contentsCreated() {
 		super.contentsCreated();
+		if (Policy.DEBUG_COMPARE_EDITOR_INPUT) {
+			logTrace("compareInputChangeListener assigned"); //$NON-NLS-1$
+			logStackTrace();
+		}
 		compareInputChangeListener = new ICompareInputChangeListener() {
 			public void compareInputChanged(ICompareInput source) {
 				if (source == getCompareResult()) {
@@ -203,10 +211,24 @@ public abstract class SaveableCompareEditorInput extends CompareEditorInput impl
 	 * @see org.eclipse.compare.CompareEditorInput#handleDispose()
 	 */
 	protected void handleDispose() {
+		if (Policy.DEBUG_COMPARE_EDITOR_INPUT) {
+			logTrace("handleDispose()"); //$NON-NLS-1$
+			logStackTrace();
+		}
 		super.handleDispose();
 		ICompareInput compareInput = getCompareInput();
-		if (compareInput != null)
+		if (compareInput != null) {
+			if (Policy.DEBUG_COMPARE_EDITOR_INPUT) {
+				if (compareInputChangeListener == null) {
+					logTrace("null change listener detected!"); //$NON-NLS-1$
+					logStackTrace();
+				}
+			}
 			compareInput.removeCompareInputChangeListener(compareInputChangeListener);
+		}
+		if (Policy.DEBUG_COMPARE_EDITOR_INPUT) {
+			logTrace("compareInputChangeListener = null"); //$NON-NLS-1$
+		}
 		compareInputChangeListener = null;
 		if (saveable instanceof SaveableComparison && propertyListener != null) {
 			SaveableComparison scm = (SaveableComparison) saveable;
@@ -222,6 +244,14 @@ public abstract class SaveableCompareEditorInput extends CompareEditorInput impl
 		}
 	}
 	
+	private void logStackTrace() {
+		new Exception("<Fake exception> in " + getClass().getName()).printStackTrace(System.out); //$NON-NLS-1$
+	}
+
+	private void logTrace(String string) {
+		System.out.println("SaveableCompareEditorInput " + System.identityHashCode(this) + ": " + string);  //$NON-NLS-1$//$NON-NLS-2$
+	}
+
 	/**
 	 * Prepare the compare input of this editor input. This method is not intended to be overridden of
 	 * extended by subclasses (but is not final for backwards compatibility reasons).