From aca44a7bd9ead429280c97c55939409dcad7fbae Mon Sep 17 00:00:00 2001 From: Sheng Chen Date: Tue, 7 Jul 2026 15:09:32 +0800 Subject: [PATCH 1/3] fix: consolidate control validation into layoutNow requestLayout and flushPendingLayoutRequests each filtered out null/ disposed controls via a shared validControls helper before delegating to layoutNow, duplicating the same defensive check at every call site. layoutNow now performs its own null/disposed filtering right where it dereferences the controls (getShell(), shell.layout(...)), so the invariant is enforced in exactly one place instead of trusted by two separate callers. requestLayout becomes a pure dispatcher: it no longer pre-filters, it just forwards the raw controls to pendingLayoutRequests or layoutNow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../copilot/eclipse/ui/chat/ChatView.java | 76 ++++++++++++++++++- .../eclipse/ui/chat/ReferencedFile.java | 9 ++- 2 files changed, 80 insertions(+), 5 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java index 3cf6ff19..870f731a 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java @@ -4,7 +4,9 @@ package com.microsoft.copilot.eclipse.ui.chat; import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -32,6 +34,7 @@ import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Shell; import org.eclipse.ui.IPartListener2; import org.eclipse.ui.IWorkbenchPartReference; import org.eclipse.ui.PlatformUI; @@ -164,6 +167,9 @@ public class ChatView extends ViewPart implements ChatProgressListener, MessageL private IContextActivation chatViewContextActivation; private IPartListener2 partListener; + /** Controls whose {@link #requestLayout(Control...)} call was suppressed while this view was hidden. */ + private final Set pendingLayoutRequests = new LinkedHashSet<>(); + @Override public void createPartControl(Composite parent) { this.parent = parent; @@ -517,7 +523,12 @@ public void partOpened(IWorkbenchPartReference partRef) { @Override public void partVisible(IWorkbenchPartReference partRef) { - // No action needed + // Replay layout requests skipped while this view was hidden behind another part in the same + // stack, targeted at just the specific control(s) that changed -- avoids a blind full-tree + // relayout of the (possibly long) chat conversation. + if (partRef.getPart(false) == ChatView.this) { + flushPendingLayoutRequests(); + } } }; @@ -1702,16 +1713,73 @@ public void dispose() { getSite().getPage().removePartListener(this.partListener); this.partListener = null; } + pendingLayoutRequests.clear(); deactivateChatViewContext(); super.dispose(); } /** - * Layout the view. + * Request a layout of the given control(s), which must be part of this view's widget tree. + * + *

Passing every control whose content actually changed (rather than a blind {@code + * parent.layout(true, true)}) lets SWT flush exactly those controls' cached sizes -- and only the + * ancestor chains leading to them -- so it never touches unrelated subtrees like the chat + * conversation history. Passing only a subset of what changed (e.g. a text label but not a + * sibling icon label that also changed) will leave the omitted control's cached size stale. + * + *

Skipped while the view isn't visible (e.g. stacked behind another part) to avoid layout cost + * on every editor switch; {@code changed} is instead remembered and replayed by {@link + * #flushPendingLayoutRequests()} once the view becomes visible again. + * + * @param changed every control whose content/layout data just changed + */ + public void requestLayout(Control... changed) { + if (getSite() == null || getSite().getPage() == null || !getSite().getPage().isPartVisible(this)) { + Collections.addAll(pendingLayoutRequests, changed); + return; + } + layoutNow(changed); + } + + /** + * Replays layout requests that were suppressed by {@link #requestLayout(Control...)} while this + * view was hidden, targeted at just the specific control(s) that changed. */ - public void layout(boolean changed, boolean all) { - parent.layout(changed, all); + private void flushPendingLayoutRequests() { + if (pendingLayoutRequests.isEmpty()) { + return; + } + // Snapshot and clear before replaying: a control's requestLayout() could in principle + // synchronously trigger a new call back into requestLayout(Control...), which would otherwise + // mutate pendingLayoutRequests while it's being iterated. + Set toFlush = new LinkedHashSet<>(pendingLayoutRequests); + pendingLayoutRequests.clear(); + layoutNow(toFlush.toArray(new Control[0])); + } + + /** + * Flushes cached sizes for exactly the given controls (and the ancestor chains leading to them) + * and lays them out, in a single batched pass. Silently ignores {@code null} or disposed + * controls. + */ + private void layoutNow(Control... controls) { + List valid = new ArrayList<>(); + if (controls != null) { + for (Control control : controls) { + if (control != null && !control.isDisposed()) { + valid.add(control); + } + } + } + if (valid.isEmpty()) { + return; + } + Shell shell = valid.get(0).getShell(); + if (shell == null || shell.isDisposed()) { + return; + } + shell.layout(valid.toArray(new Control[0]), SWT.DEFER); } /** diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFile.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFile.java index db63a981..e1bf9b25 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFile.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFile.java @@ -170,7 +170,14 @@ protected void setFile(@Nullable IResource file) { setLayoutData(layoutData); ChatView chatView = UiUtils.getView(Constants.CHAT_VIEW_ID, ChatView.class); if (chatView != null) { - chatView.layout(true, true); + // Pass every child whose content setFile()/setupXDisplay() can touch (icon image, file name + // text/CSS class, close button image) -- not just this composite or a single child. SWT's + // targeted requestLayout() only flushes cached sizes for the exact controls it's given (plus + // their ancestor chains up to cmpFileRef and beyond); passing a subset silently leaves the + // others' cached sizes stale (previously: a clipped icon, and before that a chip that didn't + // shrink). Since all three are descendants of this composite, their ancestor walk-up already + // covers this chip's own RowData/visibility change too -- no need to pass `this` separately. + chatView.requestLayout(lblfileIcon, lblFileName, lblClose); } } From 04e76827eb32b553259b4c97ac24339a42d0c373 Mon Sep 17 00:00:00 2001 From: Sheng Chen Date: Tue, 7 Jul 2026 15:16:48 +0800 Subject: [PATCH 2/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../microsoft/copilot/eclipse/ui/chat/ChatView.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java index 870f731a..0293e160 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java @@ -1734,11 +1734,15 @@ public void dispose() { * * @param changed every control whose content/layout data just changed */ - public void requestLayout(Control... changed) { - if (getSite() == null || getSite().getPage() == null || !getSite().getPage().isPartVisible(this)) { +public void requestLayout(Control... changed) { + if (getSite() == null || getSite().getPage() == null || !getSite().getPage().isPartVisible(this)) { + if (changed != null) { Collections.addAll(pendingLayoutRequests, changed); - return; } + return; + } + layoutNow(changed); +} layoutNow(changed); } From 8a8b1a7f91497d503f62395a9f25aa44ab44b2e7 Mon Sep 17 00:00:00 2001 From: Sheng Chen Date: Tue, 7 Jul 2026 15:26:22 +0800 Subject: [PATCH 3/3] fix: repair syntax broken by autofix commit in requestLayout The prior "Potential fix for pull request finding" commit (04e76827) badly spliced a null-check into requestLayout, leaving a dangling duplicate `layoutNow(changed); }` outside the method body and dedenting the whole method. This left invalid Java that failed the Tycho/ECJ compile in CI. Restores correct syntax/indentation while keeping the null-guard around Collections.addAll(pendingLayoutRequests, changed), which is the same defensive check layoutNow already applies to its own `controls` parameter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../copilot/eclipse/ui/chat/ChatView.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java index 0293e160..c5aa62c6 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java @@ -1734,15 +1734,13 @@ public void dispose() { * * @param changed every control whose content/layout data just changed */ -public void requestLayout(Control... changed) { - if (getSite() == null || getSite().getPage() == null || !getSite().getPage().isPartVisible(this)) { - if (changed != null) { - Collections.addAll(pendingLayoutRequests, changed); + public void requestLayout(Control... changed) { + if (getSite() == null || getSite().getPage() == null || !getSite().getPage().isPartVisible(this)) { + if (changed != null) { + Collections.addAll(pendingLayoutRequests, changed); + } + return; } - return; - } - layoutNow(changed); -} layoutNow(changed); }