fix: consolidate control validation into layoutNow (#340)

* 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>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix: repair syntax broken by autofix commit in requestLayout

The prior "Potential fix for pull request finding" commit (04e7682)
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>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Author: 3 people

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<Control> 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,75 @@ 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.
+   *
+   * <p>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.
+   *
+   * <p>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)) {
+      if (changed != null) {
+        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<Control> 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<Control> 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);
   }
 
   /**

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);
     }
   }