[unseasoned-pdf] Extend SubmitForm() support to the Unseasoned viewer

Allow the handling of relative form URLs. Add unit tests to verify
relative URL handling.

Migrate SubmitForm() to PdfViewPluginBase.

Add browser tests to ensure that the form submission behavior is the
same with and without the Unseasoned PDF feature enabled.

Bug: 1255225
Change-Id: Id1fc9c696b0ea54fd10486103a58f6c55d0103c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3214649
Auto-Submit: Daniel Hosseinian <dhoss@chromium.org>
Commit-Queue: K. Moon <kmoon@chromium.org>
Reviewed-by: K. Moon <kmoon@chromium.org>
Cr-Commit-Position: refs/heads/main@{#930925}

Author: Daniel Hosseinian

chrome/browser/pdf/pdf_extension_test.cc

@@ -154,6 +154,7 @@ using ::guest_view::TestGuestViewManagerFactory;
 using ::pdf_extension_test_util::ConvertPageCoordToScreenCoord;
 using ::testing::IsEmpty;
 using ::testing::MatchesRegex;
+using ::testing::StartsWith;
 using ::ui::AXTreeFormatter;
 
 const int kNumberLoadTestParts = 10;
@@ -3701,6 +3702,54 @@ IN_PROC_BROWSER_TEST_P(PDFExtensionPrerenderTest,
   ASSERT_EQ(web_contents->GetLastCommittedURL(), pdf_url);
 }
 
+class PDFExtensionSubmitFormTest : public PDFExtensionTest {
+ public:
+  void SetUpOnMainThread() override {
+    embedded_test_server()->RegisterRequestMonitor(base::BindLambdaForTesting(
+        [this](const net::test_server::HttpRequest& request) {
+          if (request.relative_url != "/pdf/test_endpoint")
+            return;
+
+          EXPECT_EQ(request.method, net::test_server::METHOD_POST);
+          EXPECT_THAT(request.content, StartsWith("\%FDF"));
+          ASSERT_TRUE(quit_closure_);
+          std::move(quit_closure_).Run();
+        }));
+
+    PDFExtensionTest::SetUpOnMainThread();
+  }
+
+ protected:
+  // Retrieves a `base::RunLoop` and saves its `QuitClosure()`. The test
+  // monitors HTTP requests on the IO thread, so `quit_closure_` needs to be set
+  // up on the UI thread before the requests can arrive.
+  std::unique_ptr<base::RunLoop> CreateFormSubmissionRunLoop() {
+    auto run_loop = std::make_unique<base::RunLoop>();
+    EXPECT_FALSE(quit_closure_);
+    quit_closure_ = run_loop->QuitClosure();
+    return run_loop;
+  }
+
+ private:
+  base::OnceClosure quit_closure_;
+};
+
+IN_PROC_BROWSER_TEST_P(PDFExtensionSubmitFormTest, SubmitForm) {
+  WebContents* guest_contents = LoadPdfGetGuestContents(
+      embedded_test_server()->GetURL("/pdf/submit_form.pdf"));
+  ASSERT_TRUE(guest_contents);
+
+  std::unique_ptr<base::RunLoop> run_loop = CreateFormSubmissionRunLoop();
+
+  // Click on the "Submit Form" button.
+  content::SimulateMouseClickAt(
+      guest_contents, blink::WebInputEvent::kNoModifiers,
+      blink::WebMouseEvent::Button::kLeft,
+      ConvertPageCoordToScreenCoord(guest_contents, {200, 200}));
+
+  run_loop->Run();
+}
+
 // TODO(crbug.com/702993): Stop testing both modes after unseasoned launches.
 INSTANTIATE_FEATURE_OVERRIDE_TEST_SUITE(PDFExtensionTest);
 INSTANTIATE_FEATURE_OVERRIDE_TEST_SUITE(PDFExtensionTestWithPartialLoading);
@@ -3721,3 +3770,4 @@ INSTANTIATE_FEATURE_OVERRIDE_TEST_SUITE(PDFExtensionHitTestTest);
 INSTANTIATE_FEATURE_OVERRIDE_TEST_SUITE(
     PDFExtensionAccessibilityNavigationTest);
 INSTANTIATE_FEATURE_OVERRIDE_TEST_SUITE(PDFExtensionPrerenderTest);
+INSTANTIATE_FEATURE_OVERRIDE_TEST_SUITE(PDFExtensionSubmitFormTest);

chrome/test/data/pdf/submit_form.in

@@ -0,0 +1,48 @@
+{{header}}
+{{object 1 0}} <<
+  /Type /Catalog
+  /Pages 2 0 R
+  /AcroForm 4 0 R
+>>
+endobj
+{{object 2 0}} <<
+  /Type /Pages
+  /Count 1
+  /Kids [3 0 R]
+>>
+endobj
+{{object 3 0}} <<
+  /Type /Page
+  /Parent 2 0 R
+  /Annots [5 0 R]
+  /MediaBox [0 0 400 400]
+>>
+endobj
+{{object 4 0}} <<
+  /Fields [5 0 R]
+>>
+endobj
+{{object 5 0}} <<
+  /Type /Annot
+  /Subtype /Widget
+  /T (SubmitFormButton)
+  /FT /Btn
+  /Ff 65536
+  /Rect [150 150 250 250]
+  /AA << /D 6 0 R >>
+  /MK <<
+     /BG [1.0 0.0 0.0]
+     /CA (Submit Form)
+   >>
+>>
+endobj
+{{object 6 0}} <<
+  /Type /Action
+  /S /JavaScript
+  /JS (this.submitForm\('test_endpoint'\))
+>>
+endobj
+{{xref}}
+{{trailer}}
+{{startxref}}
+%%EOF

chrome/test/data/pdf/submit_form.pdf

@@ -17,7 +17,6 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/location.h"
-#include "base/logging.h"
 #include "base/memory/weak_ptr.h"
 #include "base/notreached.h"
 #include "base/numerics/safe_conversions.h"
@@ -715,24 +714,6 @@ std::string OutOfProcessInstance::Prompt(const std::string& question,
   return result.is_string() ? result.AsString() : std::string();
 }
 
-void OutOfProcessInstance::SubmitForm(const std::string& url,
-                                      const void* data,
-                                      int length) {
-  UrlRequest request;
-  request.url = url;
-  request.method = "POST";
-  request.body.assign(static_cast<const char*>(data), length);
-
-  form_loader_ = CreateUrlLoaderInternal();
-  form_loader_->Open(request, base::BindOnce(&OutOfProcessInstance::FormDidOpen,
-                                             weak_factory_.GetWeakPtr()));
-}
-
-void OutOfProcessInstance::FormDidOpen(int32_t result) {
-  // TODO(crbug.com/719344): Process response.
-  LOG_IF(ERROR, result != PP_OK) << "FormDidOpen failed: " << result;
-}
-
 std::vector<PDFEngine::Client::SearchStringResult>
 OutOfProcessInstance::SearchString(const char16_t* string,
                                    const char16_t* term,

pdf/out_of_process_instance.cc

@@ -95,9 +95,6 @@ class OutOfProcessInstance : public PdfViewPluginBase,
   bool Confirm(const std::string& message) override;
   std::string Prompt(const std::string& question,
                      const std::string& default_answer) override;
-  void SubmitForm(const std::string& url,
-                  const void* data,
-                  int length) override;
   std::vector<SearchStringResult> SearchString(const char16_t* string,
                                                const char16_t* term,
                                                bool case_sensitive) override;
@@ -149,8 +146,6 @@ class OutOfProcessInstance : public PdfViewPluginBase,
  private:
   bool CanSaveEdits() const;
 
-  void FormDidOpen(int32_t result);
-
   // The Pepper image data that is in sync with mutable_image_data().
   pp::ImageData pepper_image_data_;
 

pdf/out_of_process_instance.h

@@ -26,6 +26,7 @@
 #include "base/i18n/rtl.h"
 #include "base/i18n/time_formatting.h"
 #include "base/location.h"
+#include "base/logging.h"
 #include "base/memory/weak_ptr.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/notreached.h"
@@ -69,6 +70,7 @@
 #include "ui/gfx/geometry/skia_conversions.h"
 #include "ui/gfx/geometry/vector2d.h"
 #include "ui/gfx/geometry/vector2d_f.h"
+#include "url/gurl.h"
 
 namespace chrome_pdf {
 
@@ -375,6 +377,24 @@ void PdfViewPluginBase::Print() {
   InvokePrintDialog();
 }
 
+void PdfViewPluginBase::SubmitForm(const std::string& url,
+                                   const void* data,
+                                   int length) {
+  // `url` might be a relative URL. Resolve it against the document's URL.
+  GURL resolved_url = GURL(GetURL()).Resolve(url);
+  if (!resolved_url.is_valid())
+    return;
+
+  UrlRequest request;
+  request.url = resolved_url.spec();
+  request.method = "POST";
+  request.body.assign(static_cast<const char*>(data), length);
+
+  form_loader_ = CreateUrlLoaderInternal();
+  form_loader_->Open(
+      request, base::BindOnce(&PdfViewPluginBase::DidFormOpen, GetWeakPtr()));
+}
+
 std::unique_ptr<UrlLoader> PdfViewPluginBase::CreateUrlLoader() {
   if (full_frame_) {
     DidStartLoading();
@@ -1680,6 +1700,12 @@ void PdfViewPluginBase::DidOpenPreview(std::unique_ptr<UrlLoader> loader,
   preview_engine_->HandleDocumentLoad(std::move(loader), GetURL());
 }
 
+void PdfViewPluginBase::DidFormOpen(int32_t result) {
+  // TODO(crbug.com/719344): Process response.
+  LOG_IF(ERROR, result != kSuccess) << "DidFormOpen failed: " << result;
+  form_loader_.reset();
+}
+
 void PdfViewPluginBase::OnPrintPreviewLoaded() {
   // Scroll location is retained across document loads in print preview mode, so
   // there's no need to override the scroll position by scrolling again.

pdf/pdf_view_plugin_base.cc

@@ -116,6 +116,9 @@ class PdfViewPluginBase : public PDFEngine::Client,
              const std::string& subject,
              const std::string& body) override;
   void Print() override;
+  void SubmitForm(const std::string& url,
+                  const void* data,
+                  int length) override;
   std::unique_ptr<UrlLoader> CreateUrlLoader() override;
   void DocumentLoadComplete() override;
   void DocumentLoadFailed() override;
@@ -484,6 +487,9 @@ class PdfViewPluginBase : public PDFEngine::Client,
   // Handles `LoadUrl()` result for print preview.
   void DidOpenPreview(std::unique_ptr<UrlLoader> loader, int32_t result);
 
+  // Handles `Open()` result for `form_loader_`.
+  void DidFormOpen(int32_t result);
+
   // Performs tasks necessary when the document is loaded in print preview mode.
   void OnPrintPreviewLoaded();
 
@@ -619,6 +625,9 @@ class PdfViewPluginBase : public PDFEngine::Client,
   // Whether the document is in edit mode.
   bool edit_mode_ = false;
 
+  // Used for submitting forms.
+  std::unique_ptr<UrlLoader> form_loader_;
+
   // Assigned a value only between `PrintBegin()` and `PrintEnd()` calls.
   absl::optional<blink::WebPrintParams> print_params_;
 

pdf/pdf_view_plugin_base.h

@@ -10,7 +10,9 @@
 #include <vector>
 
 #include "base/containers/contains.h"
+#include "base/cxx17_backports.h"
 #include "base/memory/weak_ptr.h"
+#include "base/strings/string_piece.h"
 #include "base/test/icu_test_util.h"
 #include "base/test/values_test_util.h"
 #include "base/time/time.h"
@@ -40,6 +42,7 @@ using ::testing::ElementsAre;
 using ::testing::IsEmpty;
 using ::testing::Return;
 using ::testing::SaveArg;
+using ::testing::StrEq;
 
 // Keep it in-sync with the `kFinalFallbackName` returned by
 // net::GetSuggestedFilename().
@@ -144,6 +147,8 @@ class FakePdfViewPluginBase : public PdfViewPluginBase {
   using PdfViewPluginBase::UpdateGeometryOnPluginRectChanged;
   using PdfViewPluginBase::UpdateScroll;
 
+  MOCK_METHOD(std::string, GetURL, (), (override));
+
   MOCK_METHOD(bool, Confirm, (const std::string&), (override));
 
   MOCK_METHOD(std::string,
@@ -1226,4 +1231,74 @@ TEST_F(PdfViewPluginBaseWithEngineTest, GetAccessibilityDocInfo) {
   EXPECT_TRUE(doc_info.text_copyable);
 }
 
+class PdfViewPluginBaseSubmitFormTest : public PdfViewPluginBaseTest {
+ public:
+  void SubmitForm(const std::string& url,
+                  base::StringPiece form_data = "data") {
+    EXPECT_CALL(fake_plugin_, CreateUrlLoaderInternal).WillOnce([this]() {
+      auto mock_loader = std::make_unique<testing::NiceMock<MockUrlLoader>>();
+      EXPECT_CALL(*mock_loader, Open).WillOnce(testing::SaveArg<0>(&request_));
+      return mock_loader;
+    });
+
+    fake_plugin_.SubmitForm(url, form_data.data(), form_data.size());
+  }
+
+  void SubmitFailingForm(const std::string& url) {
+    EXPECT_CALL(fake_plugin_, CreateUrlLoaderInternal).Times(0);
+    constexpr char kFormData[] = "form data";
+    fake_plugin_.SubmitForm(url, kFormData, base::size(kFormData));
+  }
+
+ protected:
+  UrlRequest request_;
+};
+
+TEST_F(PdfViewPluginBaseSubmitFormTest, RequestMethodAndBody) {
+  EXPECT_CALL(fake_plugin_, GetURL)
+      .WillOnce(Return("https://www.example.com/path/to/the.pdf"));
+  constexpr char kFormData[] = "form data";
+  SubmitForm(/*url=*/"", kFormData);
+  EXPECT_EQ(request_.method, "POST");
+  EXPECT_THAT(request_.body, StrEq(kFormData));
+}
+
+TEST_F(PdfViewPluginBaseSubmitFormTest, RelativeUrl) {
+  EXPECT_CALL(fake_plugin_, GetURL)
+      .WillOnce(Return("https://www.example.com/path/to/the.pdf"));
+  SubmitForm("relative_endpoint");
+  EXPECT_EQ(request_.url, "https://www.example.com/path/to/relative_endpoint");
+}
+
+TEST_F(PdfViewPluginBaseSubmitFormTest, NoRelativeUrl) {
+  EXPECT_CALL(fake_plugin_, GetURL)
+      .WillOnce(Return("https://www.example.com/path/to/the.pdf"));
+  SubmitForm("");
+  EXPECT_EQ(request_.url, "https://www.example.com/path/to/the.pdf");
+}
+
+TEST_F(PdfViewPluginBaseSubmitFormTest, AbsoluteUrl) {
+  EXPECT_CALL(fake_plugin_, GetURL)
+      .WillOnce(Return("https://a.example.com/path/to/the.pdf"));
+  SubmitForm("https://b.example.com/relative_endpoint");
+  EXPECT_EQ(request_.url, "https://b.example.com/relative_endpoint");
+}
+
+TEST_F(PdfViewPluginBaseSubmitFormTest, EmptyDocumentUrl) {
+  EXPECT_CALL(fake_plugin_, GetURL).WillOnce(Return(std::string()));
+  SubmitFailingForm("relative_endpoint");
+}
+
+TEST_F(PdfViewPluginBaseSubmitFormTest, RelativeUrlInvalidDocumentUrl) {
+  EXPECT_CALL(fake_plugin_, GetURL)
+      .WillOnce(Return(R"(https://www.%B%Ad.com/path/to/the.pdf)"));
+  SubmitFailingForm("relative_endpoint");
+}
+
+TEST_F(PdfViewPluginBaseSubmitFormTest, AbsoluteUrlInvalidDocumentUrl) {
+  EXPECT_CALL(fake_plugin_, GetURL)
+      .WillOnce(Return(R"(https://www.%B%Ad.com/path/to/the.pdf)"));
+  SubmitFailingForm("https://wwww.example.com");
+}
+
 }  // namespace chrome_pdf

pdf/pdf_view_plugin_base_unittest.cc

@@ -670,10 +670,6 @@ std::string PdfViewWebPlugin::Prompt(const std::string& question,
       .Utf8();
 }
 
-void PdfViewWebPlugin::SubmitForm(const std::string& url,
-                                  const void* data,
-                                  int length) {}
-
 std::vector<PDFEngine::Client::SearchStringResult>
 PdfViewWebPlugin::SearchString(const char16_t* string,
                                const char16_t* term,

pdf/pdf_view_web_plugin.cc

@@ -228,9 +228,6 @@ class PdfViewWebPlugin final : public PdfViewPluginBase,
   bool Confirm(const std::string& message) override;
   std::string Prompt(const std::string& question,
                      const std::string& default_answer) override;
-  void SubmitForm(const std::string& url,
-                  const void* data,
-                  int length) override;
   std::vector<SearchStringResult> SearchString(const char16_t* string,
                                                const char16_t* term,
                                                bool case_sensitive) override;