From 521cd497d12bad08d9ff04a45e57ce22249cda56 Mon Sep 17 00:00:00 2001
From: Fred Shih <ffred@chromium.org>
Date: Thu, 10 Sep 2026 18:41:46 -0700
Subject: [PATCH] Guard collaboration delegate lifetime when showing modal dialogs

Showing a browser-modal dialog via chrome::ShowBrowserModal() can run
a nested message loop on certain platforms, during which the flow may
exit and destroy the CollaborationControllerDelegateDesktop instance.
Assigning member variables for callbacks and returned widget pointers
after the call returned could attempt member access on an instance that
had already been destroyed.

Store result callbacks before showing dialogs, and use a WeakPtr check
to ensure the delegate is still valid before storing the returned widget
pointer. Add regression tests covering delegate destruction while dialogs
are being shown.

TAG=agy

Bug: 555932520
Change-Id: I82cc41b2a604fee88bc7905c5253cc41ceacb9c8
Fixed: 555932520
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8384620
Commit-Queue: Fred Shih <ffred@chromium.org>
Reviewed-by: Yuheng Huang <yuhengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1695884}
---
diff --git a/chrome/browser/ui/views/data_sharing/collaboration_controller_delegate_desktop.cc b/chrome/browser/ui/views/data_sharing/collaboration_controller_delegate_desktop.cc
index c201d27c7..186144517 100644
--- a/chrome/browser/ui/views/data_sharing/collaboration_controller_delegate_desktop.cc
+++ b/chrome/browser/ui/views/data_sharing/collaboration_controller_delegate_desktop.cc
@@ -195,8 +195,8 @@
 
   DataSharingBubbleController::From(browser_)->Close();
 
-  ShowErrorDialog(error);
   error_ui_callback_ = std::move(result);
+  ShowErrorDialog(error);
 }
 
 void CollaborationControllerDelegateDesktop::Cancel(ResultCallback result) {
@@ -212,8 +212,8 @@
     ResultCallback result) {
   access_point_ = GetAccessPointForFlowType(flow_type);
 
-  MaybeShowSignInOrSyncPromptDialog();
   authentication_ui_callback_ = std::move(result);
+  MaybeShowSignInOrSyncPromptDialog();
 }
 
 void CollaborationControllerDelegateDesktop::NotifySignInAndSyncStatusChange() {
@@ -433,8 +433,14 @@
   }
 
   std::unique_ptr<ui::DialogModel> dialog_model = builder.Build();
-  error_dialog_widget_ =
+  // Showing the dialog may run a nested loop on some platforms, during which
+  // the flow may exit and destroy `this`.
+  auto weak_this = weak_ptr_factory_.GetWeakPtr();
+  views::Widget* widget =
       chrome::ShowBrowserModal(browser_, std::move(dialog_model));
+  if (weak_this) {
+    error_dialog_widget_ = widget;
+  }
 }
 
 #if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
@@ -598,8 +604,14 @@
   }
 
   std::unique_ptr<ui::DialogModel> dialog_model = dialog_builder.Build();
-  prompt_dialog_widget_ =
+  // Showing the dialog may run a nested loop on some platforms, during which
+  // the flow may exit and destroy `this`.
+  auto weak_this = weak_ptr_factory_.GetWeakPtr();
+  views::Widget* widget =
       chrome::ShowBrowserModal(browser_, std::move(dialog_model));
+  if (weak_this) {
+    prompt_dialog_widget_ = widget;
+  }
 }
 
 void CollaborationControllerDelegateDesktop::OnPromptDialogOk() {
diff --git a/chrome/browser/ui/views/data_sharing/collaboration_controller_delegate_desktop_interactive_uitest.cc b/chrome/browser/ui/views/data_sharing/collaboration_controller_delegate_desktop_interactive_uitest.cc
index 3a381b5..ab2134e 100644
--- a/chrome/browser/ui/views/data_sharing/collaboration_controller_delegate_desktop_interactive_uitest.cc
+++ b/chrome/browser/ui/views/data_sharing/collaboration_controller_delegate_desktop_interactive_uitest.cc
@@ -4,6 +4,9 @@
 
 #include "chrome/browser/ui/views/data_sharing/collaboration_controller_delegate_desktop.h"
 
+#include <memory>
+
+#include "base/test/bind.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/mock_callback.h"
 #include "chrome/browser/signin/account_preview_data_service_factory.h"
@@ -46,6 +49,7 @@
 #include "ui/base/page_transition_types.h"
 #include "ui/gfx/image/image_unittest_util.h"
 #include "ui/views/test/dialog_test.h"
+#include "ui/views/widget/any_widget_observer.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/dialog_client_view.h"
 
@@ -387,6 +391,64 @@
 }
 
 IN_PROC_BROWSER_TEST_F(CollaborationControllerDelegateDesktopInteractiveUITest,
+                       DelegateDestroyedWhileShowingErrorDialog) {
+  // The delegate can be destroyed while its dialog is midway through being
+  // shown, e.g. when the flow is exited during the modal sheet presentation
+  // on Mac. Showing the error dialog must tolerate the delegate going away
+  // before chrome::ShowBrowserModal() returns.
+  auto delegate =
+      std::make_unique<TestCollaborationControllerDelegateDesktop>(browser());
+
+  views::Widget* dialog_widget = nullptr;
+  views::AnyWidgetObserver observer(views::test::AnyWidgetTestPasskey{});
+  observer.set_shown_callback(
+      base::BindLambdaForTesting([&](views::Widget* widget) {
+        dialog_widget = widget;
+        delegate.reset();
+      }));
+
+  base::MockCallback<
+      collaboration::CollaborationControllerDelegate::ResultCallback>
+      callback;
+  delegate->ShowError(collaboration::CollaborationControllerDelegate::ErrorInfo(
+                          collaboration::CollaborationControllerDelegate::
+                              ErrorInfo::Type::kUnknown),
+                      callback.Get());
+
+  EXPECT_FALSE(delegate);
+  ASSERT_NE(nullptr, dialog_widget);
+  dialog_widget->CloseNow();
+}
+
+IN_PROC_BROWSER_TEST_F(CollaborationControllerDelegateDesktopInteractiveUITest,
+                       DelegateDestroyedWhileShowingPromptDialog) {
+  // Same as DelegateDestroyedWhileShowingErrorDialog, but for the sign-in or
+  // sync prompt dialog.
+  collaboration::ServiceStatus status;
+  auto delegate =
+      std::make_unique<TestCollaborationControllerDelegateDesktop>(browser());
+  EXPECT_CALL(*delegate, GetServiceStatus()).WillOnce(testing::Return(status));
+
+  views::Widget* dialog_widget = nullptr;
+  views::AnyWidgetObserver observer(views::test::AnyWidgetTestPasskey{});
+  observer.set_shown_callback(
+      base::BindLambdaForTesting([&](views::Widget* widget) {
+        dialog_widget = widget;
+        delegate.reset();
+      }));
+
+  base::MockCallback<
+      collaboration::CollaborationControllerDelegate::ResultCallback>
+      callback;
+  delegate->ShowAuthenticationUi(collaboration::FlowType::kJoin,
+                                 callback.Get());
+
+  EXPECT_FALSE(delegate);
+  ASSERT_NE(nullptr, dialog_widget);
+  dialog_widget->CloseNow();
+}
+
+IN_PROC_BROWSER_TEST_F(CollaborationControllerDelegateDesktopInteractiveUITest,
                        ShowErrorDialogAndOpenUpdateChromePage) {
   // Show error dialog and open update chrome page.
   TestCollaborationControllerDelegateDesktop delegate(browser());

diff --git a/chrome/browser/ui/views/data_sharing/collaboration_controller_delegate_desktop_interactive_uitest.cc b/chrome/browser/ui/views/data_sharing/collaboration_controller_delegate_desktop_interactive_uitest.cc
index 3a381b5..ab2134e 100644
--- a/chrome/browser/ui/views/data_sharing/collaboration_controller_delegate_desktop_interactive_uitest.cc
+++ b/chrome/browser/ui/views/data_sharing/collaboration_controller_delegate_desktop_interactive_uitest.cc
@@ -4,6 +4,9 @@
 
 #include "chrome/browser/ui/views/data_sharing/collaboration_controller_delegate_desktop.h"
 
+#include <memory>
+
+#include "base/test/bind.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/mock_callback.h"
 #include "chrome/browser/signin/account_preview_data_service_factory.h"
@@ -46,6 +49,7 @@
 #include "ui/base/page_transition_types.h"
 #include "ui/gfx/image/image_unittest_util.h"
 #include "ui/views/test/dialog_test.h"
+#include "ui/views/widget/any_widget_observer.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/dialog_client_view.h"
 
@@ -387,6 +391,64 @@
 }
 
 IN_PROC_BROWSER_TEST_F(CollaborationControllerDelegateDesktopInteractiveUITest,
+                       DelegateDestroyedWhileShowingErrorDialog) {
+  // The delegate can be destroyed while its dialog is midway through being
+  // shown, e.g. when the flow is exited during the modal sheet presentation
+  // on Mac. Showing the error dialog must tolerate the delegate going away
+  // before chrome::ShowBrowserModal() returns.
+  auto delegate =
+      std::make_unique<TestCollaborationControllerDelegateDesktop>(browser());
+
+  views::Widget* dialog_widget = nullptr;
+  views::AnyWidgetObserver observer(views::test::AnyWidgetTestPasskey{});
+  observer.set_shown_callback(
+      base::BindLambdaForTesting([&](views::Widget* widget) {
+        dialog_widget = widget;
+        delegate.reset();
+      }));
+
+  base::MockCallback<
+      collaboration::CollaborationControllerDelegate::ResultCallback>
+      callback;
+  delegate->ShowError(collaboration::CollaborationControllerDelegate::ErrorInfo(
+                          collaboration::CollaborationControllerDelegate::
+                              ErrorInfo::Type::kUnknown),
+                      callback.Get());
+
+  EXPECT_FALSE(delegate);
+  ASSERT_NE(nullptr, dialog_widget);
+  dialog_widget->CloseNow();
+}
+
+IN_PROC_BROWSER_TEST_F(CollaborationControllerDelegateDesktopInteractiveUITest,
+                       DelegateDestroyedWhileShowingPromptDialog) {
+  // Same as DelegateDestroyedWhileShowingErrorDialog, but for the sign-in or
+  // sync prompt dialog.
+  collaboration::ServiceStatus status;
+  auto delegate =
+      std::make_unique<TestCollaborationControllerDelegateDesktop>(browser());
+  EXPECT_CALL(*delegate, GetServiceStatus()).WillOnce(testing::Return(status));
+
+  views::Widget* dialog_widget = nullptr;
+  views::AnyWidgetObserver observer(views::test::AnyWidgetTestPasskey{});
+  observer.set_shown_callback(
+      base::BindLambdaForTesting([&](views::Widget* widget) {
+        dialog_widget = widget;
+        delegate.reset();
+      }));
+
+  base::MockCallback<
+      collaboration::CollaborationControllerDelegate::ResultCallback>
+      callback;
+  delegate->ShowAuthenticationUi(collaboration::FlowType::kJoin,
+                                 callback.Get());
+
+  EXPECT_FALSE(delegate);
+  ASSERT_NE(nullptr, dialog_widget);
+  dialog_widget->CloseNow();
+}
+
+IN_PROC_BROWSER_TEST_F(CollaborationControllerDelegateDesktopInteractiveUITest,
                        ShowErrorDialogAndOpenUpdateChromePage) {
   // Show error dialog and open update chrome page.
   TestCollaborationControllerDelegateDesktop delegate(browser());