Fix unified diff code mining layout and footer painting

team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java

@@ -749,7 +749,7 @@ public String getRawLabel() {
 		}
 
 		@Override
-		public void dispose() {
+		public synchronized void dispose() {
 			styleRanges = null;
 			lastRectangle = null;
 			cachedFont = null;
@@ -770,7 +770,7 @@ private List<StyleRange> styleRanges(String label) {
 		}
 
 		@Override
-		public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) {
+		public synchronized Point draw(GC gc, StyledText textWidget, Color color, int x, int y) {
 			gc.setBackground(this.deletionBackgroundColor);
 			Color c = textWidget.getForeground();
 			gc.setForeground(c);
@@ -797,7 +797,7 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) {
 			if (ranges.isEmpty()) {
 				// no syntax coloring available; fall back to plain rendering
 				result = super.draw(gc, textWidget, color, x, y);
-				drawBandBorder(gc, this.borderColor, y, textWidget.getBounds().width, lastRectangle.height,
+				drawBandBorder(gc, this.borderColor, y, textWidget.getBounds().width, result.y,
 						this.bandBorder);
 				return result;
 			}
@@ -809,7 +809,9 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) {
 			gc.setFont(font);
 			drawStyleRanges(gc, textWidget, ranges, label, rawLabel, styledFonts, x, y, null,
 					whitespace, trailingCarriageReturn, trailingLineFeed);
-			drawBandBorder(gc, this.borderColor, y, textWidget.getBounds().width, lastRectangle.height,
+			// Syntax-coloring callbacks can dispose the mining and clear lastRectangle.
+			// Use the size measured by this draw call rather than the invalidated cache.
+			drawBandBorder(gc, this.borderColor, y, textWidget.getBounds().width, result.y,
 					this.bandBorder);
 			return result;
 		}

team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java

@@ -1672,8 +1672,9 @@ private static void applyDiffRightStr(ITextViewer tv, IAnnotationModel model, Un
 				positions.add(pos);
 				replaceStrings.add(diff.rightStr);
 			}
-			model.removeAnnotation(lanno);
 		}
+		// The painter must see deleted headers when it clears their reserved height.
+		removeAnnotations(model, annos);
 		if (tv instanceof ISourceViewerExtension5 ext) {
 			ext.updateCodeMinings();
 		}

team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffCodeMiningProviderTest.java

@@ -18,6 +18,7 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
@@ -49,6 +50,7 @@
 import org.eclipse.jface.text.BadLocationException;
 import org.eclipse.jface.text.Document;
 import org.eclipse.jface.text.IDocument;
+import org.eclipse.jface.text.IRegion;
 import org.eclipse.jface.text.ITextViewer;
 import org.eclipse.jface.text.Position;
 import org.eclipse.jface.text.TextAttribute;
@@ -74,6 +76,7 @@
 import org.eclipse.jface.text.source.projection.ProjectionAnnotationModel;
 import org.eclipse.jface.text.source.projection.ProjectionViewer;
 import org.eclipse.swt.SWT;
+import org.eclipse.swt.custom.StyleRange;
 import org.eclipse.swt.custom.StyledText;
 import org.eclipse.swt.graphics.Color;
 import org.eclipse.swt.graphics.Font;
@@ -633,6 +636,55 @@ private void draw(UnifiedDiffLineHeaderCodeMining mining, List<String> scannedDo
 		}
 	}
 
+	@Test
+	public void testFooterMiningDisposalDuringPlainDrawingDoesNotLoseHeight() throws Exception {
+		assertFooterDisposalDuringDrawing(false);
+	}
+
+	@Test
+	public void testFooterMiningDisposalDuringStyledDrawingDoesNotLoseHeight() throws Exception {
+		assertFooterDisposalDuringDrawing(true);
+	}
+
+	private void assertFooterDisposalDuringDrawing(boolean styled) throws Exception {
+		switchToDocument(new Document("line 0\nx"));
+		assertTrue(open("line 0\nchanged\n", MODE).isOK());
+		UnifiedDiffFooterCodeMining footer = footerMining();
+		UnifiedDiffFooterCodeMining[] painting = new UnifiedDiffFooterCodeMining[1];
+		ProjectionViewer paintingViewer = new ProjectionViewer(shell, null, null, false, SWT.V_SCROLL) {
+			@Override
+			public List<StyleRange> computeStyleRanges(IDocument syntaxDocument, IRegion region) {
+				// Invalidate the cache after draw measured its size, before it draws the border.
+				painting[0].dispose();
+				return styled ? new ArrayList<>(List.of(new StyleRange(region.getOffset(), region.getLength(),
+						display.getSystemColor(SWT.COLOR_DARK_BLUE), null))) : new ArrayList<>();
+			}
+		};
+		paintingViewer.setDocument(document);
+		paintingViewer.getTextWidget().setSize(400, 100);
+		Color deletionColor = new Color(display, 11, 22, 33);
+		Color detailedDiffColor = new Color(display, 44, 55, 66);
+		Color borderColor = new Color(display, 77, 88, 99);
+		painting[0] = new UnifiedDiffFooterCodeMining(document, provider, footer.getUnifiedDiff(), 4,
+				deletionColor, detailedDiffColor, borderColor, paintingViewer);
+		Image image = new Image(display, 400, 100);
+		GC gc = new GC(image);
+		try {
+			Point size = painting[0].draw(gc, paintingViewer.getTextWidget(), null, 0, 0);
+			assertTrue(size.y > 0, "drawing must retain its measured height after cache invalidation");
+			assertNull(painting[0].getLastRectangle(), "disposal must have cleared the cached rectangle");
+			assertTrue(firstColumnWith(image, borderColor) >= 0, "drawing must still paint the band border");
+		} finally {
+			gc.dispose();
+			image.dispose();
+			painting[0].dispose();
+			paintingViewer.getTextWidget().dispose();
+			deletionColor.dispose();
+			detailedDiffColor.dispose();
+			borderColor.dispose();
+		}
+	}
+
 	/**
 	 * A word-level footer highlight following bold text must start after the bold
 	 * text, not where the base font would put it. Inspects the pixels

team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffManagerTest.java

@@ -23,6 +23,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -1127,16 +1128,31 @@ public void testRevertingEarlierDiffRefreshesRemainingCodeMiningPosition() throw
 
 		List<UnifiedDiffManager.UnifiedDiff> diffs = UnifiedDiffManager.get(viewer());
 		assertEquals(2, diffs.size(), "the input must produce two revert diffs");
+		StyledText tw = viewer().getTextWidget();
+		tw.setSize(800, 600);
+		assertNotNull(waitForLineHeaderAnnotation());
+		UnifiedDiffManager.UnifiedDiff laterDiff = diffs.get(1);
+		lineHeaderMiningFor(laterDiff);
+		firePaint(tw);
+		int firstLine = tw.getLineAtOffset(diffs.get(0).leftStart);
+		assertTrue(tw.getLineVerticalIndent(firstLine) > 0, "the first mining must reserve height before reverting");
 		viewer().setSelectedRange(diffs.get(0).leftStart, 0);
 
 		executeEditorCommand("org.eclipse.compare.acceptCurrentDiff"); //$NON-NLS-1$
 		waitForDocumentContent(afterRevert);
 
 		diffs = UnifiedDiffManager.get(viewer());
 		assertEquals(1, diffs.size(), "reverting the first diff must leave the later diff active");
+		assertSame(laterDiff, diffs.get(0), "the surviving band must belong to the later diff");
+		assertEquals(0, tw.getLineVerticalIndent(firstLine), "removing the first header must clear its reserved height");
 		UnifiedDiffLineHeaderCodeMining mining = lineHeaderMiningFor(diffs.get(0));
 		assertEquals(afterRevert.indexOf("new later replacement"), mining.getPosition().offset,
 				"the remaining code mining must be positioned after the reverted text");
+		assertTrue(mining.getLabel().contains("old later"), "the surviving mining must retain the later deletion text");
+		firePaint(tw);
+		assertNotNull(mining.getLastRectangle(), "the surviving mining must be painted");
+		assertEquals(tw.getLocationAtOffset(tw.getOffsetAtLine(tw.getLineAtOffset(mining.getPosition().offset))).x,
+				mining.getLastRectangle().x, "the surviving mining must paint at the line origin");
 	}
 
 	@Test