From 3d63d12873dcf84fa15de78a3b4c52c1aa113b27 Mon Sep 17 00:00:00 2001
From: Lei Zhang <thestig@chromium.org>
Date: Wed, 22 Jul 2026 18:01:15 -0700
Subject: [PATCH] Print Preview: Store is_pdf bit outside of RequestPrintPreviewParams

The mojom::RequestPrintPreviewParams struct has an `is_modifiable` bit
that is potentially untrustworthy if it is set by a renderer process.
It is also confusing because sometimes the browser process sets this
bit. In the long run, there is no need for `is_modifiable` to exist, as
the browser process can figure out the value on its own. So it does not
need to be passed over IPC via RequestPrintPreviewParams.

Start migrating away from it by storing a separate `is_pdf` bit in
PrintPreviewDialogController. This bit comes from the browser process
and never from a renderer process. Allow callers to read it via
IsPrintingPdf() instead of reading `is_modifiable` via
GetRequestParams(). Convert GetRequestParams() callers to use
IsPrintingPdf() where possible.

Bug: 498367544
Change-Id: I529977a18dd52c7c0ca3fc69fa75bec0364e744c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8129157
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Andy Phan <andyphan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1666757}
---
diff --git a/chrome/browser/printing/print_preview_dialog_controller.cc b/chrome/browser/printing/print_preview_dialog_controller.cc
index e316d44..3bf649c 100644
--- a/chrome/browser/printing/print_preview_dialog_controller.cc
+++ b/chrome/browser/printing/print_preview_dialog_controller.cc
@@ -195,12 +195,13 @@
 
 void PrintPreviewDialogController::PrintPreview(
     WebContents* initiator,
-    const mojom::RequestPrintPreviewParams& params) {
+    const mojom::RequestPrintPreviewParams& params,
+    bool is_pdf) {
   if (initiator->IsCrashed()) {
     return;
   }
 
-  if (!GetOrCreatePreviewDialog(initiator, params)) {
+  if (!GetOrCreatePreviewDialog(initiator, params, is_pdf)) {
     auto* print_view_manager = PrintViewManager::FromWebContents(initiator);
     if (print_view_manager) {
       print_view_manager->PrintPreviewDone();
@@ -217,14 +218,16 @@
 
 WebContents* PrintPreviewDialogController::GetOrCreatePreviewDialogForTesting(
     WebContents* initiator) {
+  constexpr bool kIsPdf = false;
   mojom::RequestPrintPreviewParams params;
-  params.is_modifiable = true;
-  return GetOrCreatePreviewDialog(initiator, params);
+  params.is_modifiable = !kIsPdf;
+  return GetOrCreatePreviewDialog(initiator, params, kIsPdf);
 }
 
 WebContents* PrintPreviewDialogController::GetOrCreatePreviewDialog(
     WebContents* initiator,
-    const mojom::RequestPrintPreviewParams& params) {
+    const mojom::RequestPrintPreviewParams& params,
+    bool is_pdf) {
   DCHECK(initiator);
 
   // Get the print preview dialog for `initiator`.
@@ -242,7 +245,7 @@
     return nullptr;
   }
 
-  return CreatePrintPreviewDialog(tab, initiator, params);
+  return CreatePrintPreviewDialog(tab, initiator, params, is_pdf);
 }
 
 WebContents* PrintPreviewDialogController::GetPrintPreviewForContents(
@@ -276,6 +279,15 @@
   return it != preview_dialog_map_.end() ? &it->second.request_params : nullptr;
 }
 
+std::optional<bool> PrintPreviewDialogController::IsPrintingPdf(
+    content::WebContents* preview_dialog) const {
+  auto it = preview_dialog_map_.find(preview_dialog);
+  if (it != preview_dialog_map_.end()) {
+    return it->second.is_pdf;
+  }
+  return std::nullopt;
+}
+
 void PrintPreviewDialogController::ForEachPreviewDialog(
     base::RepeatingCallback<void(content::WebContents*)> callback) {
   for (const auto& it : preview_dialog_map_)
@@ -303,6 +315,8 @@
   web_contents_collection_.StopObserving(it->second.initiator);
   it->second.initiator = nullptr;
   it->second.request_params = {};
+  // Set to true to match the behavior for resetting `request_params`.
+  it->second.is_pdf = true;
   it->second.scoper.reset();
 }
 
@@ -318,9 +332,11 @@
 PrintPreviewDialogController::InitiatorData::InitiatorData(
     content::WebContents* initiator,
     const mojom::RequestPrintPreviewParams& request_params,
+    bool is_pdf,
     std::unique_ptr<tabs::ScopedTabModalUI> scoper)
     : initiator(initiator),
       request_params(request_params),
+      is_pdf(is_pdf),
       scoper(std::move(scoper)) {}
 
 PrintPreviewDialogController::InitiatorData::~InitiatorData() = default;
@@ -424,7 +440,8 @@
 WebContents* PrintPreviewDialogController::CreatePrintPreviewDialog(
     tabs::TabInterface* tab,
     content::WebContents* initiator,
-    const mojom::RequestPrintPreviewParams& params) {
+    const mojom::RequestPrintPreviewParams& params,
+    bool is_pdf) {
   base::AutoReset<bool> auto_reset(&is_creating_print_preview_dialog_, true);
 
   // The dialog delegates are deleted when the dialog is closed.
@@ -444,7 +461,8 @@
   PrintViewManager::CreateForWebContents(preview_dialog);
 
   // Add an entry to the map.
-  InitiatorData data(initiator, params, tab ? tab->ShowModalUI() : nullptr);
+  InitiatorData data(initiator, params, is_pdf,
+                     tab ? tab->ShowModalUI() : nullptr);
   preview_dialog_map_.emplace(preview_dialog, std::move(data));
 
   // Make the print preview WebContents show up in the task manager.
diff --git a/chrome/browser/printing/print_preview_dialog_controller.h b/chrome/browser/printing/print_preview_dialog_controller.h
index 77fea0a8..23b5b35 100644
--- a/chrome/browser/printing/print_preview_dialog_controller.h
+++ b/chrome/browser/printing/print_preview_dialog_controller.h
@@ -7,6 +7,7 @@
 
 #include <map>
 #include <memory>
+#include <optional>
 
 #include "base/check.h"
 #include "base/functional/callback.h"
@@ -57,7 +58,8 @@
 
   // Initiates print preview for `initiator`.
   void PrintPreview(content::WebContents* initiator,
-                    const mojom::RequestPrintPreviewParams& params);
+                    const mojom::RequestPrintPreviewParams& params,
+                    bool is_pdf);
 
   // Returns the preview dialog for `contents`.
   // Returns `contents` if `contents` is a preview dialog.
@@ -74,6 +76,10 @@
   const mojom::RequestPrintPreviewParams* GetRequestParams(
       content::WebContents* preview_dialog) const;
 
+  // Returns whether the initiator associated with `preview_dialog` is printing
+  // PDF content or not. Returns nullopt if no data exists for `preview_dialog`.
+  std::optional<bool> IsPrintingPdf(content::WebContents* preview_dialog) const;
+
   // Runs `callback` on the dialog of each active print preview operation.
   void ForEachPreviewDialog(
       base::RepeatingCallback<void(content::WebContents*)> callback);
@@ -95,9 +101,11 @@
                                         content::WebContents* preview_dialog) {
     CHECK(initiator);
     CHECK(preview_dialog);
+
+    constexpr bool kIsPdf = false;
     mojom::RequestPrintPreviewParams params;
-    params.is_modifiable = true;
-    InitiatorData data(initiator, params, /*scoper=*/nullptr);
+    params.is_modifiable = !kIsPdf;
+    InitiatorData data(initiator, params, kIsPdf, /*scoper=*/nullptr);
     preview_dialog_map_.emplace(preview_dialog, std::move(data));
   }
   void DisassociateWebContentsesForTesting(
@@ -117,6 +125,7 @@
   struct InitiatorData {
     InitiatorData(content::WebContents* initiator,
                   const mojom::RequestPrintPreviewParams& request_params,
+                  bool is_pdf,
                   std::unique_ptr<tabs::ScopedTabModalUI> scoper);
     InitiatorData(InitiatorData&&) noexcept;
     InitiatorData& operator=(InitiatorData&&) noexcept;
@@ -124,6 +133,7 @@
 
     raw_ptr<content::WebContents> initiator;
     mojom::RequestPrintPreviewParams request_params;
+    bool is_pdf;
 
     // Prevents other tab-modal UIs from showing.
     std::unique_ptr<tabs::ScopedTabModalUI> scoper;
@@ -159,14 +169,16 @@
   // Gets/Creates the print preview dialog for `initiator`.
   content::WebContents* GetOrCreatePreviewDialog(
       content::WebContents* initiator,
-      const mojom::RequestPrintPreviewParams& params);
+      const mojom::RequestPrintPreviewParams& params,
+      bool is_pdf);
 
   // Creates a new print preview dialog if GetOrCreatePreviewDialog() cannot
   // find a print preview dialog for `initiator`.
   content::WebContents* CreatePrintPreviewDialog(
       tabs::TabInterface* tab,
       content::WebContents* initiator,
-      const mojom::RequestPrintPreviewParams& params);
+      const mojom::RequestPrintPreviewParams& params,
+      bool is_pdf);
 
   // Helper function to store the title of the initiator associated with
   // `preview_dialog` in `preview_dialog`'s PrintPreviewUI.
diff --git a/chrome/browser/printing/print_view_manager.cc b/chrome/browser/printing/print_view_manager.cc
index d22df27..db182ee 100644
--- a/chrome/browser/printing/print_view_manager.cc
+++ b/chrome/browser/printing/print_view_manager.cc
@@ -581,8 +581,9 @@
   auto* dialog_controller = PrintPreviewDialogController::GetInstance();
   CHECK(dialog_controller);
   mojom::RequestPrintPreviewParams params;
-  params.is_modifiable = !print_preview_rfh_->GetProcess()->IsPdf();
-  dialog_controller->PrintPreview(web_contents(), params);
+  const bool is_pdf = print_preview_rfh_->GetProcess()->IsPdf();
+  params.is_modifiable = !is_pdf;
+  dialog_controller->PrintPreview(web_contents(), params, is_pdf);
 
   PrintPreviewAllowedForTesting();
 }
@@ -632,7 +633,8 @@
 
   auto* dialog_controller = PrintPreviewDialogController::GetInstance();
   CHECK(dialog_controller);
-  dialog_controller->PrintPreview(web_contents(), *params);
+  dialog_controller->PrintPreview(web_contents(), *params,
+                                  render_frame_host->GetProcess()->IsPdf());
 
   PrintPreviewAllowedForTesting();
 }
diff --git a/chrome/browser/ui/webui/print_preview/print_preview_handler.cc b/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
index 93e85920..9ed3579c 100644
--- a/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
+++ b/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
@@ -673,17 +673,17 @@
   }
   DCHECK(data->size());
 
-  // After validating |settings|, record metrics.
-  const mojom::RequestPrintPreviewParams* request_params = GetRequestParams();
-  CHECK(request_params);
-  bool is_pdf = !request_params->is_modifiable;
+  // After validating `settings`, record metrics.
+  std::optional<bool> maybe_is_pdf = IsPrintingPdf();
+  CHECK(maybe_is_pdf.has_value());
   if (last_preview_settings_.has_value()) {
-    ReportPrintSettingsStats(settings, last_preview_settings_.value(), is_pdf);
+    ReportPrintSettingsStats(settings, last_preview_settings_.value(),
+                             maybe_is_pdf.value());
   }
   {
     PrintDocumentTypeBuckets doc_type =
-        is_pdf ? PrintDocumentTypeBuckets::kPdfDocument
-               : PrintDocumentTypeBuckets::kHtmlDocument;
+        maybe_is_pdf.value() ? PrintDocumentTypeBuckets::kPdfDocument
+                             : PrintDocumentTypeBuckets::kHtmlDocument;
     ReportPrintDocumentTypeHistograms(doc_type);
   }
   ReportUserActionHistogram(user_action);
@@ -861,20 +861,33 @@
     const std::string& callback_id,
     base::DictValue policies,
     const std::string& default_printer) {
-  const mojom::RequestPrintPreviewParams* request_params = GetRequestParams();
+  bool is_pdf;
   mojom::RequestPrintPreviewParams dummy_params;
-  if (!request_params) {
+
+  // `request_params` and `maybe_is_pdf` should both be non-null or both be
+  // null.
+  auto* dialog_controller = PrintPreviewDialogController::GetInstance();
+  CHECK(dialog_controller);
+  const mojom::RequestPrintPreviewParams* request_params =
+      dialog_controller->GetRequestParams(preview_web_contents());
+  std::optional<bool> maybe_is_pdf = IsPrintingPdf();
+  if (request_params) {
+    CHECK(maybe_is_pdf.has_value());
+    is_pdf = maybe_is_pdf.value();
+  } else {
     // This only happens with a direct navigation to chrome://print, which can
     // happen in some tests. Just use `dummy_params` to set up the test with
     // some sane values, so it does not crash.
-    dummy_params.is_modifiable = true;
+    CHECK(!maybe_is_pdf.has_value());
+    constexpr bool kIsPdf = false;
+    is_pdf = kIsPdf;
+    dummy_params.is_modifiable = !kIsPdf;
     request_params = &dummy_params;
   }
 
   base::DictValue initial_settings;
   initial_settings.Set(kDocumentTitle, print_preview_ui()->initiator_title());
-  initial_settings.Set(kSettingPreviewModifiable,
-                       request_params->is_modifiable);
+  initial_settings.Set(kSettingPreviewModifiable, !is_pdf);
 #if BUILDFLAG(IS_CHROMEOS)
   initial_settings.Set(kSettingPreviewIsFromArc, request_params->is_from_arc);
 #endif
@@ -953,11 +966,10 @@
   return dialog_controller->GetInitiator(preview_web_contents());
 }
 
-const mojom::RequestPrintPreviewParams*
-PrintPreviewHandler::GetRequestParams() {
+std::optional<bool> PrintPreviewHandler::IsPrintingPdf() {
   auto* dialog_controller = PrintPreviewDialogController::GetInstance();
   CHECK(dialog_controller);
-  return dialog_controller->GetRequestParams(preview_web_contents());
+  return dialog_controller->IsPrintingPdf(preview_web_contents());
 }
 
 void PrintPreviewHandler::OnPrintPreviewReady(
diff --git a/chrome/browser/ui/webui/print_preview/print_preview_handler.h b/chrome/browser/ui/webui/print_preview/print_preview_handler.h
index 7432256..b65fcda 100644
--- a/chrome/browser/ui/webui/print_preview/print_preview_handler.h
+++ b/chrome/browser/ui/webui/print_preview/print_preview_handler.h
@@ -147,7 +147,9 @@
 
   PrintPreviewUI* print_preview_ui();
 
-  const mojom::RequestPrintPreviewParams* GetRequestParams();
+  // Returns whether `this` is printing a PDF or not. Returns std::nullopt if
+  // the associated Print Preview dialog does not exist.
+  std::optional<bool> IsPrintingPdf();
 
   PrefService* GetPrefs();
 
diff --git a/chrome/browser/ui/webui/print_preview/print_preview_ui.cc b/chrome/browser/ui/webui/print_preview/print_preview_ui.cc
index d5b35950..c4160fae 100644
--- a/chrome/browser/ui/webui/print_preview/print_preview_ui.cc
+++ b/chrome/browser/ui/webui/print_preview/print_preview_ui.cc
@@ -584,10 +584,10 @@
 
   auto* dialog_controller = PrintPreviewDialogController::GetInstance();
   CHECK(dialog_controller);
-  const mojom::RequestPrintPreviewParams* request_params =
-      dialog_controller->GetRequestParams(web_ui()->GetWebContents());
-  CHECK(request_params);
-  return request_params->is_modifiable;
+  std::optional<bool> maybe_is_pdf =
+      dialog_controller->IsPrintingPdf(web_ui()->GetWebContents());
+  CHECK(maybe_is_pdf.has_value());
+  return !maybe_is_pdf.value();
 }
 
 void PrintPreviewUI::OnCompositePdfPageDone(