Skip to content

Commit da979ab

Browse files
tobiasmelchertobias-melcher
tobiasmelcher
and
tobias-melcher
authored
Fix UI flickering in code mining by adding wait/notify reconciliation coordination (#2698)
* Fix UI flickering in code mining by adding wait/notify reconciliation coordination Problem: Previous implementation returned emptyList() immediately when reconciliation wasn't complete, causing code minings to disappear/reappear during AST reconciliation → UI flickering Solution: * use CompletableFuture for an ITypeRoot to handle waiting for reconciliation * store and supply CompletableFutures in JavaCodeMiningReconciler map using ITextEditor as key * use future.thenApplySync and future.orTimeOut methods in ICodeMiningProvider.provideCodeMinings() methods to get code minings after reconcilliation or to recognize cancellation/timeout Co-authored-by: Tobias Melcher <tobias.melcher@sap.com>
1 parent 5cddb3b commit da979ab

File tree

4 files changed

+116
-94
lines changed

4 files changed

+116
-94
lines changed

‎org.eclipse.jdt.text.tests/src/org/eclipse/jdt/text/tests/codemining/ParameterNamesCodeMiningTest.java‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,11 @@ public void tearDown() throws Exception {
102102
}
103103
}
104104

105-
private void waitReconciled(JavaSourceViewer viewer) {
105+
private void waitReconciled(JavaEditor editor, JavaSourceViewer viewer) {
106106
assertTrue(new DisplayHelper() {
107107
@Override
108108
protected boolean condition() {
109-
return JavaCodeMiningReconciler.isReconciled(viewer);
109+
return JavaCodeMiningReconciler.getFuture(editor).isDone();
110110
}
111111
}.waitForCondition(viewer.getTextWidget().getDisplay(), 2000), "Editor not reconciled");
112112
}
@@ -122,7 +122,7 @@ public class Foo {
122122
JavaEditor editor= (JavaEditor) EditorUtility.openInEditor(compilationUnit);
123123
fParameterNameCodeMiningProvider.setContext(editor);
124124
JavaSourceViewer viewer= (JavaSourceViewer)editor.getViewer();
125-
waitReconciled(viewer);
125+
waitReconciled(editor, viewer);
126126

127127
assertEquals(3, fParameterNameCodeMiningProvider.provideCodeMinings(viewer, new NullProgressMonitor()).get().size());
128128
}
@@ -138,7 +138,7 @@ public class Foo {
138138
JavaEditor editor= (JavaEditor) EditorUtility.openInEditor(compilationUnit);
139139
fParameterNameCodeMiningProvider.setContext(editor);
140140
JavaSourceViewer viewer= (JavaSourceViewer)editor.getViewer();
141-
waitReconciled(viewer);
141+
waitReconciled(editor, viewer);
142142
assertEquals(2, fParameterNameCodeMiningProvider.provideCodeMinings(viewer, new NullProgressMonitor()).get().size());
143143
}
144144

@@ -162,7 +162,7 @@ public void foo() {
162162
viewer.setCodeMiningProviders(new ICodeMiningProvider[] {
163163
fParameterNameCodeMiningProvider
164164
});
165-
waitReconciled(viewer);
165+
waitReconciled(editor, viewer);
166166
StyledText widget= viewer.getTextWidget();
167167
//
168168
assertEquals(2, fParameterNameCodeMiningProvider.provideCodeMinings(viewer, new NullProgressMonitor()).get().size());
@@ -192,7 +192,7 @@ public void mehod() {
192192
JavaEditor editor= (JavaEditor) EditorUtility.openInEditor(compilationUnit);
193193
fParameterNameCodeMiningProvider.setContext(editor);
194194
JavaSourceViewer viewer= (JavaSourceViewer)editor.getViewer();
195-
waitReconciled(viewer);
195+
waitReconciled(editor, viewer);
196196
// Only code mining on "printf" parameters
197197
assertEquals(2, fParameterNameCodeMiningProvider.provideCodeMinings(viewer, new NullProgressMonitor()).get().size());
198198
}
@@ -221,7 +221,7 @@ public class Foo {
221221
viewer.setCodeMiningProviders(new ICodeMiningProvider[] {
222222
fParameterNameCodeMiningProvider
223223
});
224-
waitReconciled(viewer);
224+
waitReconciled(editor, viewer);
225225

226226
AtomicReference<IStatus> errorInLog= new AtomicReference<>();
227227
ILogListener logListener= (status, plugin) -> {
@@ -254,7 +254,7 @@ public class Foo {
254254
viewer.setCodeMiningProviders(new ICodeMiningProvider[] {
255255
fParameterNameCodeMiningProvider
256256
});
257-
waitReconciled(viewer);
257+
waitReconciled(editor, viewer);
258258
//
259259
StyledText widget= viewer.getTextWidget();
260260
int charWidth= widget.getTextBounds(0, 1).width;
@@ -324,7 +324,7 @@ public void testBug547232() throws Exception {
324324
JavaEditor editor= (JavaEditor) EditorUtility.openInEditor(compilationUnit);
325325
fParameterNameCodeMiningProvider.setContext(editor);
326326
JavaSourceViewer viewer= (JavaSourceViewer)editor.getViewer();
327-
waitReconciled(viewer);
327+
waitReconciled(editor, viewer);
328328

329329
assertEquals(2, fParameterNameCodeMiningProvider.provideCodeMinings(viewer, new NullProgressMonitor()).get().size());
330330
}
@@ -351,7 +351,7 @@ public void testBug549023() throws Exception {
351351
JavaEditor editor= (JavaEditor) EditorUtility.openInEditor(compilationUnit);
352352
fParameterNameCodeMiningProvider.setContext(editor);
353353
JavaSourceViewer viewer= (JavaSourceViewer)editor.getViewer();
354-
waitReconciled(viewer);
354+
waitReconciled(editor, viewer);
355355

356356
assertEquals(2, fParameterNameCodeMiningProvider.provideCodeMinings(viewer, new NullProgressMonitor()).get().size());
357357
}
@@ -374,7 +374,7 @@ public void testBug549126() throws Exception {
374374
JavaEditor editor= (JavaEditor) EditorUtility.openInEditor(compilationUnit);
375375
fParameterNameCodeMiningProvider.setContext(editor);
376376
JavaSourceViewer viewer= (JavaSourceViewer)editor.getViewer();
377-
waitReconciled(viewer);
377+
waitReconciled(editor, viewer);
378378

379379
assertEquals(2, fParameterNameCodeMiningProvider.provideCodeMinings(viewer, new NullProgressMonitor()).get().size());
380380
}
@@ -405,7 +405,7 @@ record Ca (int size){
405405
JavaEditor editor= (JavaEditor) EditorUtility.openInEditor(compilationUnit);
406406
fParameterNameCodeMiningProvider.setContext(editor);
407407
JavaSourceViewer viewer= (JavaSourceViewer)editor.getViewer();
408-
waitReconciled(viewer);
408+
waitReconciled(editor, viewer);
409409

410410
assertEquals(expectedShownNames, fParameterNameCodeMiningProvider.provideCodeMinings(viewer, new NullProgressMonitor()).get().size());
411411
}
@@ -440,7 +440,7 @@ public void test () {
440440
JavaEditor editor= (JavaEditor) EditorUtility.openInEditor(compilationUnit);
441441
fParameterNameCodeMiningProvider.setContext(editor);
442442
JavaSourceViewer viewer= (JavaSourceViewer)editor.getViewer();
443-
waitReconciled(viewer);
443+
waitReconciled(editor, viewer);
444444

445445
// 6 parameter names from Edge
446446
// 4 parameter names from the 2 Map.entry(int, int) calls
@@ -484,7 +484,7 @@ public void foo() {
484484
JavaEditor editor= (JavaEditor) EditorUtility.openInEditor(compilationUnit);
485485
fParameterNameCodeMiningProvider.setContext(editor);
486486
JavaSourceViewer viewer= (JavaSourceViewer)editor.getViewer();
487-
waitReconciled(viewer);
487+
waitReconciled(editor,viewer);
488488
assertEquals(0, fParameterNameCodeMiningProvider.provideCodeMinings(viewer, new NullProgressMonitor()).get().size());
489489
}
490490

‎org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/JavaCodeMiningReconciler.java‎

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,18 @@
1313
*******************************************************************************/
1414
package org.eclipse.jdt.internal.ui.javaeditor;
1515

16-
import java.util.HashSet;
17-
import java.util.Set;
16+
import java.util.Map;
17+
import java.util.concurrent.CompletableFuture;
18+
import java.util.concurrent.ConcurrentHashMap;
1819

1920
import org.eclipse.core.runtime.IProgressMonitor;
2021

2122
import org.eclipse.jface.text.source.ISourceViewer;
2223
import org.eclipse.jface.text.source.ISourceViewerExtension5;
2324

25+
import org.eclipse.ui.texteditor.ITextEditor;
26+
27+
import org.eclipse.jdt.core.ITypeRoot;
2428
import org.eclipse.jdt.core.dom.CompilationUnit;
2529

2630
import org.eclipse.jdt.internal.ui.text.java.IJavaReconcilingListener;
@@ -31,10 +35,12 @@
3135
public class JavaCodeMiningReconciler implements IJavaReconcilingListener {
3236

3337
/**
34-
* Stores the set of viewers for which source is reconciled and requests
35-
* for references can be performed.
38+
* Maps Java editors to futures representing their associated {@link ITypeRoot}.
39+
* <p>
40+
* The future is completed when a reconciled {@link ITypeRoot} becomes available for the editor,
41+
* or cancelled/replaced when a new reconciliation cycle starts.
3642
*/
37-
private static final Set<ISourceViewerExtension5> reconciledViewers= new HashSet<>();
43+
private static final Map<ITextEditor, CompletableFuture<ITypeRoot>> typeRootFutureByEditor= new ConcurrentHashMap<>();
3844

3945
/** The Java editor this Java code mining reconciler is installed on */
4046
private JavaEditor fEditor;
@@ -45,17 +51,45 @@ public class JavaCodeMiningReconciler implements IJavaReconcilingListener {
4551

4652
@Override
4753
public void reconciled(CompilationUnit ast, boolean forced, IProgressMonitor progressMonitor) {
48-
final ISourceViewerExtension5 sourceViewer= fSourceViewer; // take a copy as this can be null-ed in the meantime
49-
if (sourceViewer != null) {
50-
reconciledViewers.add(sourceViewer);
54+
final JavaEditor editor= fEditor; // take a copy as this can be null-ed in the meantime
55+
final ISourceViewerExtension5 sourceViewer= fSourceViewer;
56+
if (editor != null && sourceViewer != null) {
5157
sourceViewer.updateCodeMinings();
58+
CompletableFuture<ITypeRoot> future= typeRootFutureByEditor.get(editor);
59+
if (future != null && !future.isDone()) {
60+
if (ast != null && ast.getTypeRoot() != null) {
61+
future.complete(ast.getTypeRoot());
62+
} else {
63+
future.cancel(false);
64+
}
65+
}
66+
}
67+
}
68+
69+
public static CompletableFuture<ITypeRoot> getFuture(ITextEditor editor) {
70+
return typeRootFutureByEditor.computeIfAbsent(editor, JavaCodeMiningReconciler::typeRootFor);
71+
}
72+
73+
private static CompletableFuture<ITypeRoot> typeRootFor(ITextEditor editor) {
74+
CompletableFuture<ITypeRoot> future= new CompletableFuture<>();
75+
ITypeRoot unit= EditorUtility.getEditorInputJavaElement(editor, true);
76+
if (unit != null) {
77+
future.complete(unit);
5278
}
79+
return future;
5380
}
5481

5582
@Override
5683
public void aboutToBeReconciled() {
57-
// interrupt code minings if modification occurs
58-
reconciledViewers.remove(fSourceViewer);
84+
if (fEditor == null) {
85+
return;
86+
}
87+
typeRootFutureByEditor.compute(fEditor, (editor, existingFuture) -> {
88+
if (existingFuture != null) {
89+
existingFuture.cancel(false);
90+
}
91+
return new CompletableFuture<ITypeRoot>();
92+
});
5993
}
6094

6195
/**
@@ -83,16 +117,14 @@ public void install(JavaEditor editor, ISourceViewer sourceViewer) {
83117
* Uninstall this reconciler from the editor.
84118
*/
85119
public void uninstall() {
86-
reconciledViewers.remove(fSourceViewer);
120+
CompletableFuture<ITypeRoot> future= typeRootFutureByEditor.remove(fEditor);
121+
if (future != null) {
122+
future.cancel(false);
123+
}
87124
if (fEditor instanceof CompilationUnitEditor) {
88125
((CompilationUnitEditor) fEditor).removeReconcileListener(this);
89126
}
90127
fEditor= null;
91128
fSourceViewer= null;
92129
}
93-
94-
public static boolean isReconciled(ISourceViewerExtension5 viewer) {
95-
return reconciledViewers.contains(viewer);
96-
}
97-
98130
}

‎org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/codemining/JavaElementCodeMiningProvider.java‎

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,17 @@
1616
import java.util.ArrayList;
1717
import java.util.Collections;
1818
import java.util.List;
19+
import java.util.concurrent.CancellationException;
1920
import java.util.concurrent.CompletableFuture;
21+
import java.util.concurrent.CompletionException;
22+
import java.util.concurrent.TimeUnit;
2023

2124
import org.eclipse.core.runtime.IProgressMonitor;
2225

2326
import org.eclipse.jface.text.BadLocationException;
2427
import org.eclipse.jface.text.ITextViewer;
2528
import org.eclipse.jface.text.codemining.AbstractCodeMiningProvider;
2629
import org.eclipse.jface.text.codemining.ICodeMining;
27-
import org.eclipse.jface.text.source.ISourceViewerExtension5;
2830

2931
import org.eclipse.ui.texteditor.ITextEditor;
3032

@@ -36,7 +38,6 @@
3638

3739
import org.eclipse.jdt.ui.PreferenceConstants;
3840

39-
import org.eclipse.jdt.internal.ui.javaeditor.EditorUtility;
4041
import org.eclipse.jdt.internal.ui.javaeditor.JavaCodeMiningReconciler;
4142
import org.eclipse.jdt.internal.ui.javaeditor.JavaEditor;
4243
import org.eclipse.jdt.internal.ui.preferences.JavaPreferencesPropertyTester;
@@ -83,40 +84,34 @@ public CompletableFuture<List<? extends ICodeMining>> provideCodeMinings(ITextVi
8384
if (!editorEnabled) {
8485
return CompletableFuture.completedFuture(Collections.emptyList());
8586
}
86-
if (viewer instanceof ISourceViewerExtension5) {
87-
ISourceViewerExtension5 codeMiningViewer = (ISourceViewerExtension5)viewer;
88-
if (!JavaCodeMiningReconciler.isReconciled(codeMiningViewer)) {
89-
// the provider isn't able to return code minings for non-reconciled viewers
90-
return CompletableFuture.completedFuture(Collections.emptyList());
87+
ITextEditor textEditor= super.getAdapter(ITextEditor.class);
88+
CompletableFuture<ITypeRoot> future= JavaCodeMiningReconciler.getFuture(textEditor);
89+
return future.thenApplyAsync(typeRoot -> {
90+
return computeCodeMinings(viewer, textEditor, monitor, typeRoot);
91+
}).orTimeout(15, TimeUnit.SECONDS).handle((result, ex) -> {
92+
if (ex instanceof CompletionException ce &&
93+
ce.getCause() instanceof CancellationException) {
94+
monitor.setCanceled(true);
9195
}
92-
}
93-
return CompletableFuture.supplyAsync(() -> {
94-
monitor.isCanceled();
95-
ITextEditor textEditor= super.getAdapter(ITextEditor.class);
96-
ITypeRoot unit= EditorUtility.getEditorInputJavaElement(textEditor, true);
97-
if (unit == null) {
98-
return Collections.emptyList();
99-
}
100-
try {
101-
IJavaElement[] elements= unit.getChildren();
102-
List<ICodeMining> minings= new ArrayList<>(elements.length);
103-
collectMinings(unit, textEditor, unit.getChildren(), minings, viewer, monitor);
104-
// interrupt if editor was marked to be reconciled in the meantime
105-
if (viewer instanceof ISourceViewerExtension5) {
106-
ISourceViewerExtension5 codeMiningViewer= (ISourceViewerExtension5)viewer;
107-
if (!JavaCodeMiningReconciler.isReconciled(codeMiningViewer)) {
108-
monitor.setCanceled(true);
109-
}
110-
}
111-
monitor.isCanceled();
112-
return minings;
113-
} catch (JavaModelException e) {
114-
// Should never occur
115-
}
116-
return Collections.emptyList();
96+
return result;
11797
});
11898
}
11999

100+
private List<? extends ICodeMining> computeCodeMinings(ITextViewer viewer, ITextEditor textEditor, IProgressMonitor monitor, ITypeRoot unit) {
101+
if (unit == null) {
102+
return Collections.emptyList();
103+
}
104+
try {
105+
IJavaElement[] elements= unit.getChildren();
106+
List<ICodeMining> minings= new ArrayList<>(elements.length);
107+
collectMinings(unit, textEditor, unit.getChildren(), minings, viewer, monitor);
108+
return minings;
109+
} catch (JavaModelException e) {
110+
// Should never occur
111+
}
112+
return Collections.emptyList();
113+
}
114+
120115
/**
121116
* Collect java code minings.
122117
*

0 commit comments

Comments
 (0)