Bug 320672 - [WorkbenchParts] [Compatibility] SWTException when
activating an editor with stale content
This is the second part of the fix for bug 320672, the first one is
covered by bug 511873.
Remember if we are in the middle of the focus state change. During this
operation we should not attempt to call updatePartControl() from
sanityCheckState() (which is called on part activation) because it can
dispose widgets we are currently setting focus to.
In theory, this can happen from other protected methods here too, but
I'm trying to keep the patch as small as possible because this class is
a base class for MANY other text editors.
Change-Id: I86af3ed7463628f7476359f76be262ed2ddaf997
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
Also-by: Vasili Gulevich <vasili.gulevich@xored.com>
diff --git a/org.eclipse.ui.workbench.texteditor.tests/META-INF/MANIFEST.MF b/org.eclipse.ui.workbench.texteditor.tests/META-INF/MANIFEST.MF
index 4a1e176..54d7943 100644
--- a/org.eclipse.ui.workbench.texteditor.tests/META-INF/MANIFEST.MF
+++ b/org.eclipse.ui.workbench.texteditor.tests/META-INF/MANIFEST.MF
@@ -15,6 +15,10 @@
org.eclipse.ui.workbench.texteditor;bundle-version="[3.5.0,4.0.0)",
org.eclipse.ui;bundle-version="[3.5.0,4.0.0)",
org.junit;bundle-version="4.12.0",
- org.eclipse.text.tests;bundle-version="[3.5.0,4.0.0)"
+ org.eclipse.text.tests;bundle-version="[3.5.0,4.0.0)",
+ org.eclipse.ui.editors;bundle-version="3.11.0",
+ org.eclipse.core.filesystem;bundle-version="1.7.0",
+ org.eclipse.ui.ide;bundle-version="3.13.0",
+ org.eclipse.e4.ui.model.workbench;bundle-version="1.3.0"
Bundle-RequiredExecutionEnvironment: JavaSE-1.8
Eclipse-BundleShape: dir
diff --git a/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/StatusEditorTest.java b/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/StatusEditorTest.java
new file mode 100644
index 0000000..0302e62
--- /dev/null
+++ b/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/StatusEditorTest.java
@@ -0,0 +1,128 @@
+/*******************************************************************************
+ * Copyright (c) 2017 Vasili Gulevich 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:
+ * Vasili Gulevich <vasili.gulevich@xored.com> - initial implementation
+ * Andrey Loskutov <loskutov@gmx.de> - polishing and integration
+ *******************************************************************************/
+package org.eclipse.ui.workbench.texteditor.tests;
+
+import java.lang.reflect.Method;
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import org.eclipse.swt.custom.CTabFolder;
+import org.eclipse.swt.widgets.Display;
+
+import org.eclipse.core.filesystem.EFS;
+
+import org.eclipse.core.runtime.ILog;
+import org.eclipse.core.runtime.ILogListener;
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.Platform;
+import org.eclipse.core.runtime.Status;
+
+import org.eclipse.ui.IEditorPart;
+import org.eclipse.ui.IWorkbenchPage;
+import org.eclipse.ui.IWorkbenchWindow;
+import org.eclipse.ui.PlatformUI;
+import org.eclipse.ui.WorkbenchException;
+import org.eclipse.ui.ide.FileStoreEditorInput;
+import org.eclipse.ui.internal.PartSite;
+
+import org.eclipse.ui.texteditor.AbstractTextEditor;
+import org.eclipse.ui.texteditor.IDocumentProvider;
+
+import org.eclipse.ui.editors.text.EditorsUI;
+import org.eclipse.ui.editors.text.ForwardingDocumentProvider;
+import org.eclipse.ui.editors.text.TextEditor;
+
+public class StatusEditorTest {
+ private IWorkbenchWindow window;
+ private Display display;
+ private IWorkbenchPage page;
+
+ @Before
+ public void before() throws WorkbenchException {
+ window = PlatformUI.getWorkbench().openWorkbenchWindow(null);
+ display = window.getShell().getDisplay();
+ page = window.getActivePage();
+ processEvents();
+ }
+
+ @After
+ public void after() {
+ window.close();
+ page = null;
+ processEvents();
+ }
+
+ /*
+ * https://bugs.eclipse.org/bugs/attachment.cgi?bugid=320672
+ *
+ * No exceptions are thrown when a status editor displaying an erroneous status is activated with a mouse click.
+ * @throws Exception
+ */
+ @Test
+ public void doNotThrowOnActivationInStale() throws Exception {
+ IEditorPart editor1 = openNonExistentFile(page, new URI("file:/1.txt"));
+ openNonExistentFile(page, new URI("file:/2.txt"));
+ ILog log = Platform.getLog(Platform.getBundle("org.eclipse.e4.ui.workbench"));
+ List<String> logEvents = new ArrayList<>();
+ ILogListener listener = (status, plugin) -> logEvents.add(status.toString());
+ log.addLogListener(listener);
+ // Clicks are not equivalent to activation from API, so we need this
+ // hack to imitate tab clicks.
+ CTabFolder folder = (CTabFolder) (((PartSite) editor1.getSite()).getModel().getParent().getWidget());
+ try {
+ folder.setSelection(0);
+ processEvents();
+ folder.setSelection(1);
+ processEvents();
+ } finally {
+ log.removeLogListener(listener);
+ }
+ if(!logEvents.isEmpty()) {
+ Assert.assertEquals("Unexpected errors logged", "", logEvents.toString());
+ }
+ }
+
+ private void processEvents() {
+ while (display.readAndDispatch()) {
+ //
+ }
+ }
+
+ private IEditorPart openNonExistentFile(IWorkbenchPage page1, URI uri) throws Exception {
+ FileStoreEditorInput input = new FileStoreEditorInput(EFS.getStore(uri));
+ TextEditor editor = (TextEditor) page1.openEditor(input, EditorsUI.DEFAULT_TEXT_EDITOR_ID, true);
+ Method setMethod= AbstractTextEditor.class.getDeclaredMethod("setDocumentProvider", IDocumentProvider.class);
+ setMethod.setAccessible(true);
+ setMethod.invoke(editor, new ErrorDocumentProvider(editor.getDocumentProvider()));
+ editor.setInput(input);
+ processEvents();
+ return editor;
+ }
+
+ private static class ErrorDocumentProvider extends ForwardingDocumentProvider {
+ public ErrorDocumentProvider(IDocumentProvider parent) {
+ super("", ignored -> { /**/ }, parent);
+ }
+
+ @Override
+ public IStatus getStatus(Object element) {
+ return new Status(IStatus.ERROR, "org.eclipse.ui.workbench.texteditor.tests",
+ 0, "This document provider always fails", null);
+ }
+ }
+}
diff --git a/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/WorkbenchTextEditorTestSuite.java b/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/WorkbenchTextEditorTestSuite.java
index ec8d266..01a8a8e 100644
--- a/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/WorkbenchTextEditorTestSuite.java
+++ b/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/WorkbenchTextEditorTestSuite.java
@@ -33,7 +33,8 @@
ChangeRegionTest.class,
RulerTestSuite.class,
HunkComputerTest.class,
- ScreenshotTest.class
+ ScreenshotTest.class,
+ StatusEditorTest.class
})
public class WorkbenchTextEditorTestSuite {
// see @SuiteClasses
diff --git a/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/StatusTextEditor.java b/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/StatusTextEditor.java
index 5cd1851..28131c4 100644
--- a/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/StatusTextEditor.java
+++ b/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/StatusTextEditor.java
@@ -23,6 +23,7 @@
import org.eclipse.core.runtime.IStatus;
import org.eclipse.ui.IEditorInput;
+import org.eclipse.ui.PlatformUI;
/**
@@ -39,6 +40,11 @@
private Composite fDefaultComposite;
/** The status page */
private Control fStatusControl;
+ /** {@link #setFocus()} is still running */
+ private boolean setFocusIsRunning;
+
+ // No .options for plugin yet
+ private static final boolean DEBUG = false;
/*
* @see org.eclipse.ui.texteditor.AbstractTextEditor.createPartControl(Composite)
@@ -64,6 +70,13 @@
* @param input the input whose status is checked
*/
public void updatePartControl(IEditorInput input) {
+ final String where = "updatePartControl"; //$NON-NLS-1$
+ if (setFocusIsRunning) {
+ trace(where, "ERROR: trying to call update while processing focus", fStatusControl); //$NON-NLS-1$
+ } else {
+ trace(where, "START", fStatusControl); //$NON-NLS-1$
+ }
+
boolean restoreFocus= false;
if (fStatusControl != null) {
@@ -71,6 +84,7 @@
restoreFocus= containsFocus(fStatusControl);
}
fStatusControl.dispose();
+ trace(where, "status control disposed", fStatusControl); //$NON-NLS-1$
fStatusControl= null;
}
@@ -83,6 +97,7 @@
front= fDefaultComposite;
} else {
fStatusControl= createStatusControl(fParent, status);
+ trace(where, "status control created", fStatusControl); //$NON-NLS-1$
front= fStatusControl;
}
}
@@ -97,6 +112,7 @@
if (restoreFocus && fStatusControl != null && !containsFocus(fStatusControl)) {
fParent.setFocus();
}
+ trace(where, "END", fStatusControl); //$NON-NLS-1$
}
private boolean containsFocus(Control control) {
@@ -112,6 +128,14 @@
@Override
public void setFocus() {
+ final String where = "setFocus"; //$NON-NLS-1$
+ if (setFocusIsRunning) {
+ trace(where, "ERROR: trying to call setFocus while processing focus", fStatusControl); //$NON-NLS-1$
+ } else {
+ trace(where, "START", fStatusControl); //$NON-NLS-1$
+ }
+ setFocusIsRunning = true;
+
if (fStatusControl != null && !fStatusControl.isDisposed()) {
/* even if the control does not really take focus, we still have to set it
* to fulfill the contract and to make e.g. Ctrl+PageUp/Down work. */
@@ -119,6 +143,10 @@
} else {
super.setFocus();
}
+
+ setFocusIsRunning = false;
+
+ trace(where, "END", fStatusControl); //$NON-NLS-1$
}
@Override
@@ -227,37 +255,59 @@
@Override
protected void doSetInput(IEditorInput input) throws CoreException {
super.doSetInput(input);
- if (fParent != null && !fParent.isDisposed())
- updatePartControl(getEditorInput());
+ updatePartControl();
}
@Override
public void doRevertToSaved() {
// http://dev.eclipse.org/bugs/show_bug.cgi?id=19014
super.doRevertToSaved();
- if (fParent != null && !fParent.isDisposed())
- updatePartControl(getEditorInput());
+ updatePartControl();
}
@Override
protected void sanityCheckState(IEditorInput input) {
// http://dev.eclipse.org/bugs/show_bug.cgi?id=19014
super.sanityCheckState(input);
- if (fParent != null && !fParent.isDisposed())
- updatePartControl(getEditorInput());
+ if (!setFocusIsRunning) {
+ updatePartControl();
+ } else {
+ trace("sanityCheck", "delaying update", fStatusControl); //$NON-NLS-1$ //$NON-NLS-2$
+ PlatformUI.getWorkbench().getDisplay().asyncExec(() -> {
+ trace("sanityCheck", "incoming update", fStatusControl); //$NON-NLS-1$ //$NON-NLS-2$
+ updatePartControl();
+ });
+ }
}
@Override
protected void handleEditorInputChanged() {
super.handleEditorInputChanged();
- if (fParent != null && !fParent.isDisposed())
- updatePartControl(getEditorInput());
+ updatePartControl();
}
@Override
protected void handleElementContentReplaced() {
super.handleElementContentReplaced();
- if (fParent != null && !fParent.isDisposed())
+ updatePartControl();
+ }
+
+ private void updatePartControl() {
+ if (fParent != null && !fParent.isDisposed()) {
updatePartControl(getEditorInput());
+ }
+ }
+
+ private static void trace(String where, String what, Control o) {
+ if (!DEBUG) {
+ return;
+ }
+ String id;
+ if (o == null) {
+ id = "null"; //$NON-NLS-1$
+ } else {
+ id = System.identityHashCode(o) + (o.isDisposed() ? "<disposed!>" : ""); //$NON-NLS-1$ //$NON-NLS-2$
+ }
+ System.out.println(where + " |" + id + "| " + what); //$NON-NLS-1$//$NON-NLS-2$
}
}
