From c72906f92191aa760ae462ad5d879ab95ebbc472 Mon Sep 17 00:00:00 2001
From: Alison Gale <[email protected]>
Date: Thu, 13 Nov 2025 13:19:00 -0800
Subject: [PATCH] [SxS] Block opening file picker from inactive tab

This only handles the first bug which is when the tab is inactive when
the file request happens. Will need to follow up to observe tab
activation changes to update when a tab becomes inactive.

Bug: 444653104
Change-Id: Ica1845ddaab9820d2bf1ecc419578042f2fce6f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7087491
Commit-Queue: Alison Gale <[email protected]>
Reviewed-by: Bo Liu <[email protected]>
Reviewed-by: Charlie Reis <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1544520}
---

diff --git a/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc b/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc
index 3b618e1..078c7d2 100644
--- a/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc
+++ b/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc
@@ -716,7 +716,9 @@
                       "  })})();"));
 
   // Top-level page in first window picks files and sends it to iframe.
-  first_party_web_contents->WasShown();
+  browser()->tab_strip_model()->ActivateTabAt(
+      browser()->tab_strip_model()->GetIndexOfWebContents(
+          first_party_web_contents));
   EXPECT_EQ(test_file.BaseName().AsUTF8Unsafe(),
             content::EvalJs(first_party_web_contents,
                             "(async () => {"
@@ -882,7 +884,9 @@
                       "  })})();"));
 
   // Top-level page in first window picks files and sends it to iframe.
-  first_party_web_contents->WasShown();
+  browser()->tab_strip_model()->ActivateTabAt(
+      browser()->tab_strip_model()->GetIndexOfWebContents(
+          first_party_web_contents));
   EXPECT_EQ(test_file.BaseName().AsUTF8Unsafe(),
             content::EvalJs(first_party_web_contents,
                             "(async () => {"
diff --git a/content/browser/file_system_access/file_system_chooser_browsertest.cc b/content/browser/file_system_access/file_system_chooser_browsertest.cc
index 7aa930a..2bddf3e 100644
--- a/content/browser/file_system_access/file_system_chooser_browsertest.cc
+++ b/content/browser/file_system_access/file_system_chooser_browsertest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -25,6 +26,7 @@
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/storage_partition.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/back_forward_cache_util.h"
 #include "content/public/test/browser_test.h"
@@ -2197,6 +2199,34 @@
   EXPECT_EQ(recorder.state, SelectFileDialogRecorder::kNotCreated);
 }
 
+class InactiveWebContentsDelegate : public content::WebContentsDelegate {
+  bool IsContentsActive(WebContents* wc) override { return false; }
+};
+
+IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest, DontShowWhileInactive) {
+  GURL url = embedded_test_server()->GetURL("/title1.html");
+  ASSERT_TRUE(NavigateToURL(shell(), url));
+
+  // Record the state of the dialog.
+  SelectFileDialogRecorder recorder;
+  ui::SelectFileDialog::SetFactory(
+      std::make_unique<ObservableSelectFileDialogFactory>(
+          recorder.GetWeakPtr()));
+
+  // Provide a WebContentsDelegate that marks all web contents as inactive.
+  WebContents* wc = shell()->web_contents();
+  std::unique_ptr<InactiveWebContentsDelegate> mock =
+      std::make_unique<InactiveWebContentsDelegate>();
+  wc->SetDelegate(mock.get());
+
+  // JS should see the dialog as aborted.
+  EXPECT_EQ(
+      "AbortError",
+      content::EvalJs(wc, "window.showOpenFilePicker().catch(e => e.name)"));
+  // The dialog should not have been created.
+  EXPECT_EQ(recorder.state, SelectFileDialogRecorder::kNotCreated);
+}
+
 // TODO(crbug.com/457495639): We need a different way to detect when a
 // WebContents is no longer displayed to the user for android since the
 // intent to select a file always causes a HIDDEN event as the whole app
diff --git a/content/browser/web_contents_based_canceller.cc b/content/browser/web_contents_based_canceller.cc
index 6c2a39c..b72e3f36 100644
--- a/content/browser/web_contents_based_canceller.cc
+++ b/content/browser/web_contents_based_canceller.cc
@@ -4,9 +4,13 @@
 
 #include "content/browser/web_contents_based_canceller.h"
 
+#include "base/feature_list.h"
 #include "base/memory/ptr_util.h"
+#include "content/common/features.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_delegate.h"
+
 namespace content {
 
 // static
@@ -37,7 +41,7 @@
     return false;
   }
   return CanShowForVisibility(web_contents()->GetVisibility()) &&
-         CanShowForRFHActiveState();
+         CanShowForRFHActiveState() && CanShowForTabState();
 }
 
 bool WebContentsBasedCanceller::CanShowForVisibility(Visibility visibility) {
@@ -50,6 +54,21 @@
   return render_frame_host && render_frame_host->IsActive();
 }
 
+bool WebContentsBasedCanceller::CanShowForTabState() {
+  if (!base::FeatureList::IsEnabled(
+          features::kSideBySideFilePickerCancelling)) {
+    return true;
+  }
+  // Within Split View, it is possible for the tab containing a WebContents to
+  // be visible but not active. This scenario is considered a cancel condition
+  // for kVisibility rather than kActiveState because kActiveState is determined
+  // by the RenderFrameHost state, while kVisibility is determined by the
+  // WebContents state.
+  WebContentsDelegate* web_contents_delegate = web_contents()->GetDelegate();
+  return condition_ != CancelCondition::kVisibility || !web_contents_delegate ||
+         web_contents_delegate->IsContentsActive(web_contents());
+}
+
 void WebContentsBasedCanceller::SetCancelCallback(
     CancelCallback cancel_callback) {
   CHECK(cancel_callback_.is_null());
diff --git a/content/browser/web_contents_based_canceller.h b/content/browser/web_contents_based_canceller.h
index 9456ae9..c12b42b 100644
--- a/content/browser/web_contents_based_canceller.h
+++ b/content/browser/web_contents_based_canceller.h
@@ -35,11 +35,12 @@
     // something that is incompatible with back/forward-cache, e.g. screen
     // capture should not occur while in back/forward-cache.
     kActiveState = 0,
-    // Cancel if `WebContents::GetVisibility` is `HIDDEN`. Use this for
-    // something that is incompatible with switching tabs, e.g. a modal system
-    // dialog like a file-chooser that displays on top of everything should be
-    // dismissed if the tab is no longer the visible tab. This also implies
-    // `kActiveState` because an inactive RFH won't be visible.
+    // Cancel if `WebContents::GetVisibility` is `HIDDEN` or the WebContents is
+    // not the active tab in the browser. Use this for something that is
+    // incompatible with switching tabs, e.g. a modal system dialog like a
+    // file-chooser that displays on top of everything should be dismissed if
+    // the tab is no longer the visible tab. This also implies `kActiveState`
+    // because an inactive RFH won't be visible.
     kVisibility = 1,
   };
   // If the cancel condition is already true, this returns `nullptr`. Otherwise
@@ -63,6 +64,7 @@
   bool CanShow();
   bool CanShowForVisibility(Visibility visibility);
   bool CanShowForRFHActiveState();
+  bool CanShowForTabState();
 
   // WebContentsObserver
   void OnVisibilityChanged(Visibility visibility) override;
diff --git a/content/common/features.cc b/content/common/features.cc
index d9fdcd1..f6b38a04 100644
--- a/content/common/features.cc
+++ b/content/common/features.cc
@@ -606,6 +606,10 @@
 BASE_FEATURE(kServiceWorkerClientUrlIsCreationUrl,
              base::FEATURE_ENABLED_BY_DEFAULT);
 
+// Handles blocking the file picker when a visible but inactive tab in a split
+// triggers it. This serves as a kill switch for crbug.com/444653104.
+BASE_FEATURE(kSideBySideFilePickerCancelling, base::FEATURE_ENABLED_BY_DEFAULT);
+
 // Enables skipping the early call to CommitPending when navigating away from a
 // crashed frame.
 BASE_FEATURE(kSkipEarlyCommitPendingForCrashedFrame,
diff --git a/content/common/features.h b/content/common/features.h
index 5b6d84c..762c98a 100644
--- a/content/common/features.h
+++ b/content/common/features.h
@@ -184,6 +184,7 @@
 CONTENT_EXPORT BASE_DECLARE_FEATURE(
     kServiceWorkerStaticRouterStartServiceWorker);

diff --git a/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc b/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc
index 3b618e1..078c7d2 100644
--- a/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc
+++ b/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc
@@ -716,7 +716,9 @@
                       "  })})();"));
 
   // Top-level page in first window picks files and sends it to iframe.
-  first_party_web_contents->WasShown();
+  browser()->tab_strip_model()->ActivateTabAt(
+      browser()->tab_strip_model()->GetIndexOfWebContents(
+          first_party_web_contents));
   EXPECT_EQ(test_file.BaseName().AsUTF8Unsafe(),
             content::EvalJs(first_party_web_contents,
                             "(async () => {"
@@ -882,7 +884,9 @@
                       "  })})();"));
 
   // Top-level page in first window picks files and sends it to iframe.
-  first_party_web_contents->WasShown();
+  browser()->tab_strip_model()->ActivateTabAt(
+      browser()->tab_strip_model()->GetIndexOfWebContents(
+          first_party_web_contents));
   EXPECT_EQ(test_file.BaseName().AsUTF8Unsafe(),
             content::EvalJs(first_party_web_contents,
                             "(async () => {"
diff --git a/content/browser/file_system_access/file_system_chooser_browsertest.cc b/content/browser/file_system_access/file_system_chooser_browsertest.cc
index 7aa930a..2bddf3e 100644
--- a/content/browser/file_system_access/file_system_chooser_browsertest.cc
+++ b/content/browser/file_system_access/file_system_chooser_browsertest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -25,6 +26,7 @@
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/storage_partition.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/back_forward_cache_util.h"
 #include "content/public/test/browser_test.h"
@@ -2197,6 +2199,34 @@
   EXPECT_EQ(recorder.state, SelectFileDialogRecorder::kNotCreated);
 }
 
+class InactiveWebContentsDelegate : public content::WebContentsDelegate {
+  bool IsContentsActive(WebContents* wc) override { return false; }
+};
+
+IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest, DontShowWhileInactive) {
+  GURL url = embedded_test_server()->GetURL("/title1.html");
+  ASSERT_TRUE(NavigateToURL(shell(), url));
+
+  // Record the state of the dialog.
+  SelectFileDialogRecorder recorder;
+  ui::SelectFileDialog::SetFactory(
+      std::make_unique<ObservableSelectFileDialogFactory>(
+          recorder.GetWeakPtr()));
+
+  // Provide a WebContentsDelegate that marks all web contents as inactive.
+  WebContents* wc = shell()->web_contents();
+  std::unique_ptr<InactiveWebContentsDelegate> mock =
+      std::make_unique<InactiveWebContentsDelegate>();
+  wc->SetDelegate(mock.get());
+
+  // JS should see the dialog as aborted.
+  EXPECT_EQ(
+      "AbortError",
+      content::EvalJs(wc, "window.showOpenFilePicker().catch(e => e.name)"));
+  // The dialog should not have been created.
+  EXPECT_EQ(recorder.state, SelectFileDialogRecorder::kNotCreated);
+}
+
 // TODO(crbug.com/457495639): We need a different way to detect when a
 // WebContents is no longer displayed to the user for android since the
 // intent to select a file always causes a HIDDEN event as the whole app