From 635f4536b519a7662d279afa47fe74cd8015d02e Mon Sep 17 00:00:00 2001
From: Peter Pakkenberg <[email protected]>
Date: Mon, 08 Jun 2026 02:28:33 -0700
Subject: [PATCH] Prevent fd reuse in WebView printing

Fixed: 516947912
Change-Id: I737d9a5babddedc5add93a9fa6ccc78488df5c7b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7902413
Reviewed-by: Nate Fischer <[email protected]>
Auto-Submit: Peter Pakkenberg <[email protected]>
Commit-Queue: Peter Pakkenberg <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1643054}
---

diff --git a/android_webview/browser/aw_print_manager.cc b/android_webview/browser/aw_print_manager.cc
index 873f779..5d46b93d 100644
--- a/android_webview/browser/aw_print_manager.cc
+++ b/android_webview/browser/aw_print_manager.cc
@@ -62,8 +62,8 @@
 }
 
 void AwPrintManager::PdfWritingDone(int page_count) {
-  // Invalidate the file descriptor so it doesn't get reused.
-  fd_ = base::kInvalidFd;
+  // The fd_ should have been reset when printing started.
+  CHECK_EQ(fd_, base::kInvalidFd);
   // Trigger the callback to notify the embedding application that printing is
   // done. A non-positive `page_count` value (<=0) will be presented as an error
   // callback to the application.
@@ -145,6 +145,16 @@
 void AwPrintManager::DidPrintDocument(
     printing::mojom::DidPrintDocumentParamsPtr params,
     DidPrintDocumentCallback callback) {
+  // Exchange the fd_ with kInvalidFd here to prevent it from being used more
+  // than once.
+  int print_fd = std::exchange(fd_, base::kInvalidFd);
+
+  if (print_fd == base::kInvalidFd) {
+    PdfWritingDone(0);
+    std::move(callback).Run(false);
+    return;
+  }
+
   if (params->document_cookie != cookie()) {
     PdfWritingDone(0);
     std::move(callback).Run(false);
@@ -176,7 +186,8 @@
       {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
        base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})
       ->PostTaskAndReplyWithResult(
-          FROM_HERE, base::BindOnce(&SaveDataToFd, fd_, number_pages(), data),
+          FROM_HERE,
+          base::BindOnce(&SaveDataToFd, print_fd, number_pages(), data),
           base::BindOnce(&AwPrintManager::OnDidPrintDocumentWritingDone,
                          weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
diff --git a/android_webview/browser/aw_print_manager.h b/android_webview/browser/aw_print_manager.h
index bdcd853..e0f1ff8 100644
--- a/android_webview/browser/aw_print_manager.h
+++ b/android_webview/browser/aw_print_manager.h
@@ -7,6 +7,7 @@
 
 #include <memory>
 
+#include "base/file_descriptor_posix.h"
 #include "base/memory/weak_ptr.h"
 #include "components/printing/browser/print_manager.h"
 #include "components/printing/common/print.mojom-forward.h"
@@ -58,7 +59,7 @@
   std::unique_ptr<printing::PrintSettings> settings_;
 
   // The file descriptor into which the PDF of the document will be written.
-  int fd_ = -1;
+  int fd_ = base::kInvalidFd;
 
   WEB_CONTENTS_USER_DATA_KEY_DECL();
 
diff --git a/android_webview/browser/aw_print_manager_unittest.cc b/android_webview/browser/aw_print_manager_unittest.cc
new file mode 100644
index 0000000..6b729c8
--- /dev/null
+++ b/android_webview/browser/aw_print_manager_unittest.cc
@@ -0,0 +1,165 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "android_webview/browser/aw_print_manager.h"
+
+#include <utility>
+
+#include "base/files/file.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/functional/bind.h"
+#include "base/functional/callback.h"
+#include "base/memory/read_only_shared_memory_region.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/run_loop.h"
+#include "base/test/bind.h"
+#include "base/test/task_environment.h"
+#include "components/printing/common/print.mojom.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/browser_task_environment.h"
+#include "content/public/test/test_browser_context.h"
+#include "content/public/test/test_content_client_initializer.h"
+#include "content/public/test/web_contents_tester.h"
+#include "printing/print_settings.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace android_webview {
+
+class PrintManagerPeer : public printing::PrintManager {
+ public:
+  static int GetCookie(printing::PrintManager* manager) {
+    return static_cast<PrintManagerPeer*>(manager)->cookie();
+  }
+};
+
+class AwPrintManagerTest : public testing::Test {
+ public:
+  AwPrintManagerTest() = default;
+  ~AwPrintManagerTest() override = default;
+
+  void SetUp() override {
+    test_content_client_initializer_ =
+        std::make_unique<content::TestContentClientInitializer>();
+    browser_context_ = std::make_unique<content::TestBrowserContext>();
+    web_contents_ = content::WebContentsTester::CreateTestWebContents(
+        browser_context_.get(), nullptr);
+    ASSERT_TRUE(web_contents_)
+        << "WebContentsTester::CreateTestWebContents returned null!";
+    AwPrintManager::CreateForWebContents(web_contents_.get());
+  }
+
+  void TearDown() override {
+    web_contents_.reset();
+    browser_context_.reset();
+    test_content_client_initializer_.reset();
+  }
+
+  content::WebContents* web_contents() { return web_contents_.get(); }
+
+ protected:
+  content::BrowserTaskEnvironment task_environment_;
+
+ private:
+  std::unique_ptr<content::TestContentClientInitializer>
+      test_content_client_initializer_;
+  std::unique_ptr<content::TestBrowserContext> browser_context_;
+  std::unique_ptr<content::WebContents> web_contents_;
+};
+
+TEST_F(AwPrintManagerTest, FdIsResetOncePrintingStarts) {
+  auto* print_manager = AwPrintManager::FromWebContents(web_contents());
+  ASSERT_TRUE(print_manager);
+
+  int test_fd = 123;
+  auto settings = std::make_unique<printing::PrintSettings>();
+
+  bool callback_called = false;
+  print_manager->UpdateParam(
+      std::move(settings), test_fd,
+      base::BindLambdaForTesting(
+          [&callback_called](int page_count) { callback_called = true; }));
+
+  auto params = printing::mojom::DidPrintDocumentParams::New();
+  params->document_cookie = 0;  // Wrong cookie to fail early
+  params->content = printing::mojom::DidPrintContentParams::New();
+
+  auto* host = static_cast<printing::mojom::PrintManagerHost*>(print_manager);
+
+  bool did_print_callback_called = false;
+
+  // We pass a wrong cookie, which causes DidPrintDocument to fail early.
+  // DidPrintDocument will call PdfWritingDone(0).
+  // Inside PdfWritingDone(), it CHECKs that fd_ is set to base::kInvalidFd.
+  // If fd_ was not reset when printing started (i.e. at the beginning of
+  // DidPrintDocument), the test will crash.
+  host->DidPrintDocument(
+      std::move(params),
+      base::BindLambdaForTesting([&did_print_callback_called](bool success) {
+        did_print_callback_called = true;
+        EXPECT_FALSE(success);
+      }));
+
+  EXPECT_TRUE(callback_called);
+  EXPECT_TRUE(did_print_callback_called);
+}
+
+TEST_F(AwPrintManagerTest, FdIsResetAfterSuccessfulPrint) {
+  auto* print_manager = AwPrintManager::FromWebContents(web_contents());
+  ASSERT_TRUE(print_manager);
+
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+  base::FilePath temp_file;
+  ASSERT_TRUE(base::CreateTemporaryFileInDir(temp_dir.GetPath(), &temp_file));
+  base::File file(temp_file, base::File::FLAG_OPEN | base::File::FLAG_WRITE);
+  int valid_fd = file.GetPlatformFile();

diff --git a/android_webview/browser/aw_print_manager_unittest.cc b/android_webview/browser/aw_print_manager_unittest.cc
new file mode 100644
index 0000000..6b729c8
--- /dev/null
+++ b/android_webview/browser/aw_print_manager_unittest.cc
@@ -0,0 +1,165 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "android_webview/browser/aw_print_manager.h"
+
+#include <utility>
+
+#include "base/files/file.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/functional/bind.h"
+#include "base/functional/callback.h"
+#include "base/memory/read_only_shared_memory_region.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/run_loop.h"
+#include "base/test/bind.h"
+#include "base/test/task_environment.h"
+#include "components/printing/common/print.mojom.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/browser_task_environment.h"
+#include "content/public/test/test_browser_context.h"
+#include "content/public/test/test_content_client_initializer.h"
+#include "content/public/test/web_contents_tester.h"
+#include "printing/print_settings.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace android_webview {
+
+class PrintManagerPeer : public printing::PrintManager {
+ public:
+  static int GetCookie(printing::PrintManager* manager) {
+    return static_cast<PrintManagerPeer*>(manager)->cookie();
+  }
+};
+
+class AwPrintManagerTest : public testing::Test {
+ public:
+  AwPrintManagerTest() = default;
+  ~AwPrintManagerTest() override = default;
+
+  void SetUp() override {
+    test_content_client_initializer_ =
+        std::make_unique<content::TestContentClientInitializer>();
+    browser_context_ = std::make_unique<content::TestBrowserContext>();
+    web_contents_ = content::WebContentsTester::CreateTestWebContents(
+        browser_context_.get(), nullptr);
+    ASSERT_TRUE(web_contents_)
+        << "WebContentsTester::CreateTestWebContents returned null!";
+    AwPrintManager::CreateForWebContents(web_contents_.get());
+  }
+
+  void TearDown() override {
+    web_contents_.reset();
+    browser_context_.reset();
+    test_content_client_initializer_.reset();
+  }
+
+  content::WebContents* web_contents() { return web_contents_.get(); }
+
+ protected:
+  content::BrowserTaskEnvironment task_environment_;
+
+ private:
+  std::unique_ptr<content::TestContentClientInitializer>
+      test_content_client_initializer_;
+  std::unique_ptr<content::TestBrowserContext> browser_context_;
+  std::unique_ptr<content::WebContents> web_contents_;
+};
+
+TEST_F(AwPrintManagerTest, FdIsResetOncePrintingStarts) {
+  auto* print_manager = AwPrintManager::FromWebContents(web_contents());
+  ASSERT_TRUE(print_manager);
+
+  int test_fd = 123;
+  auto settings = std::make_unique<printing::PrintSettings>();
+
+  bool callback_called = false;
+  print_manager->UpdateParam(
+      std::move(settings), test_fd,
+      base::BindLambdaForTesting(
+          [&callback_called](int page_count) { callback_called = true; }));
+
+  auto params = printing::mojom::DidPrintDocumentParams::New();
+  params->document_cookie = 0;  // Wrong cookie to fail early
+  params->content = printing::mojom::DidPrintContentParams::New();
+
+  auto* host = static_cast<printing::mojom::PrintManagerHost*>(print_manager);
+
+  bool did_print_callback_called = false;
+
+  // We pass a wrong cookie, which causes DidPrintDocument to fail early.
+  // DidPrintDocument will call PdfWritingDone(0).
+  // Inside PdfWritingDone(), it CHECKs that fd_ is set to base::kInvalidFd.
+  // If fd_ was not reset when printing started (i.e. at the beginning of
+  // DidPrintDocument), the test will crash.
+  host->DidPrintDocument(
+      std::move(params),
+      base::BindLambdaForTesting([&did_print_callback_called](bool success) {
+        did_print_callback_called = true;
+        EXPECT_FALSE(success);
+      }));
+
+  EXPECT_TRUE(callback_called);
+  EXPECT_TRUE(did_print_callback_called);
+}
+
+TEST_F(AwPrintManagerTest, FdIsResetAfterSuccessfulPrint) {
+  auto* print_manager = AwPrintManager::FromWebContents(web_contents());
+  ASSERT_TRUE(print_manager);
+
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+  base::FilePath temp_file;
+  ASSERT_TRUE(base::CreateTemporaryFileInDir(temp_dir.GetPath(), &temp_file));
+  base::File file(temp_file, base::File::FLAG_OPEN | base::File::FLAG_WRITE);
+  int valid_fd = file.GetPlatformFile();
+
+  auto settings = std::make_unique<printing::PrintSettings>();
+
+  bool callback_called = false;
+  print_manager->UpdateParam(
+      std::move(settings), valid_fd,
+      base::BindLambdaForTesting([&callback_called](int page_count) {
+        callback_called = true;
+        EXPECT_EQ(1, page_count);
+      }));
+
+  auto params = printing::mojom::DidPrintDocumentParams::New();
+  params->document_cookie = PrintManagerPeer::GetCookie(print_manager);
+
+  auto content = printing::mojom::DidPrintContentParams::New();
+  const uint8_t test_data[] = "test print data";
+  base::MappedReadOnlyRegion mapped_region =
+      base::ReadOnlySharedMemoryRegion::Create(sizeof(test_data));
+  ASSERT_TRUE(mapped_region.IsValid());
+  mapped_region.mapping.GetMemoryAsSpan<uint8_t>().copy_from(test_data);
+
+  content->metafile_data_region = std::move(mapped_region.region);
+  params->content = std::move(content);
+
+  auto* host = static_cast<printing::mojom::PrintManagerHost*>(print_manager);
+
+  host->DidGetPrintedPagesCount(PrintManagerPeer::GetCookie(print_manager), 1);
+
+  base::RunLoop run_loop;
+  bool did_print_callback_called = false;
+
+  host->DidPrintDocument(
+      std::move(params),
+      base::BindLambdaForTesting(
+          [&did_print_callback_called,
+           quit_closure = run_loop.QuitClosure()](bool success) mutable {
+            did_print_callback_called = true;
+            EXPECT_TRUE(success);
+            std::move(quit_closure).Run();
+          }));
+
+  run_loop.Run();
+
+  EXPECT_TRUE(callback_called);
+  EXPECT_TRUE(did_print_callback_called);
+}
+
+}  // namespace android_webview
diff --git a/android_webview/test/BUILD.gn b/android_webview/test/BUILD.gn
index 8e9d4ad0..a5ccaec3fd 100644
--- a/android_webview/test/BUILD.gn
+++ b/android_webview/test/BUILD.gn
@@ -798,6 +798,8 @@
     "//components/policy/core/browser:test_support",
     "//components/prefs",
     "//components/prefs:test_support",
+    "//components/printing/common",
+    "//components/printing/common:mojo_interfaces",
     "//components/safe_browsing/content/browser/web_ui",
     "//components/safe_browsing/core/browser",
     "//components/safe_browsing/core/common",
@@ -812,6 +814,8 @@
     "//net",
     "//net:test_support",
     "//net/third_party/quiche:blind_sign_auth",
+    "//printing",
+    "//printing:settings",
     "//services/cert_verifier/public/mojom",
     "//services/network:test_support",
     "//services/tracing/public/cpp/background_tracing",
@@ -839,6 +843,7 @@
     "../browser/aw_origin_matched_header_unittest.cc",
     "../browser/aw_pac_processor_unittest.cc",
     "../browser/aw_permission_manager_unittest.cc",
+    "../browser/aw_print_manager_unittest.cc",
     "../browser/aw_user_agent_metadata_unittest.cc",
     "../browser/content_restriction/aw_content_restriction_url_loader_throttle_unittest.cc",
     "../browser/cookie_manager_unittest.cc",