[unseasoned-pdf] Add support for "has-edits" attribute

Implements support for the "has-edits" attribute set when exiting from
annotation mode.

Also improves overall consistency between OutOfProcessInstance::Init()
and ParseWebPluginParams(), and adds default implementations of all copy
and move operations to ParsedParams.

Simplifies ParsedParamsTest to test individual attributes independently.

Fixed: 1257666
Bug: 1232152
Change-Id: I365cefb0069c4e2ebeb89abd278b1a471da260cd
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3228752
Commit-Queue: K. Moon <kmoon@chromium.org>
Reviewed-by: Daniel Hosseinian <dhoss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#932687}

chrome/browser/resources/pdf/controller.js

@@ -496,9 +496,9 @@ export class PluginController {
     } else {
       url = URL.createObjectURL(new Blob([data]));
       this.plugin_.setAttribute('src', url);
+      this.plugin_.setAttribute('has-edits', '');
     }
 
-    this.plugin_.setAttribute('has-edits', '');
     this.plugin_.style.display = 'block';
     try {
       await this.getLoadedCallback_();

chrome/browser/resources/pdf/pdf_internal_plugin_wrapper.js

@@ -15,6 +15,7 @@ channel.port1.onmessage = e => {
       URL.revokeObjectURL(plugin.src);
     }
     plugin.src = URL.createObjectURL(new Blob([e.data.dataToLoad]));
+    plugin.setAttribute('has-edits', '');
   } else {
     plugin.postMessage(e.data);
   }

pdf/out_of_process_instance.cc

@@ -450,6 +450,7 @@ bool OutOfProcessInstance::Init(uint32_t argc,
 
   text_input_ = std::make_unique<pp::TextInput_Dev>(this);
 
+  // Parse attributes. Keep in sync with `ParseWebPluginParams()`.
   const char* src_url = nullptr;
   const char* original_url = nullptr;
   const char* top_level_url = nullptr;

pdf/parsed_params.cc

@@ -9,6 +9,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "pdf/pdfium/pdfium_form_filler.h"
 #include "third_party/blink/public/platform/web_string.h"
+#include "third_party/blink/public/platform/web_vector.h"
 #include "third_party/blink/public/web/web_plugin_params.h"
 
 namespace chrome_pdf {
@@ -16,38 +17,45 @@ namespace chrome_pdf {
 ParsedParams::ParsedParams() = default;
 
 ParsedParams::ParsedParams(const ParsedParams& other) = default;
+ParsedParams& ParsedParams::operator=(const ParsedParams& other) = default;
+
+ParsedParams::ParsedParams(ParsedParams&& other) = default;
+ParsedParams& ParsedParams::operator=(ParsedParams&& other) = default;
 
 ParsedParams::~ParsedParams() = default;
 
 absl::optional<ParsedParams> ParseWebPluginParams(
     const blink::WebPluginParams& params) {
+  // Keep in sync with `OutOfProcessInstance::Init()`.
+  // TODO(crbug.com/1232152): Don't have two implementations.
   ParsedParams result;
   for (size_t i = 0; i < params.attribute_names.size(); ++i) {
-    if (params.attribute_names[i] == "original-url") {
+    if (params.attribute_names[i] == "src") {
-      result.original_url = params.attribute_values[i].Utf8();
-    } else if (params.attribute_names[i] == "src") {
       result.src_url = params.attribute_values[i].Utf8();
+    } else if (params.attribute_names[i] == "original-url") {
+      result.original_url = params.attribute_values[i].Utf8();
+    } else if (params.attribute_names[i] == "top-level-url") {
+      result.top_level_url = params.attribute_values[i].Utf8();
     } else if (params.attribute_names[i] == "full-frame") {
       result.full_frame = true;
     } else if (params.attribute_names[i] == "background-color") {
-      SkColor background_color;
       if (!base::StringToUint(params.attribute_values[i].Utf8(),
-                              &background_color)) {
+                              &result.background_color)) {
         return absl::nullopt;
       }
-      result.background_color = background_color;
     } else if (params.attribute_names[i] == "javascript") {
       if (params.attribute_values[i] != "allow")
         result.script_option = PDFiumFormFiller::ScriptOption::kNoJavaScript;
+    } else if (params.attribute_names[i] == "has-edits") {
+      result.has_edits = true;
     }
   }
 
   if (result.src_url.empty())
     return absl::nullopt;
 
-  if (result.original_url.empty()) {
+  if (result.original_url.empty())
     result.original_url = result.src_url;
-  }
 
   return result;
 }

pdf/parsed_params.h

@@ -13,30 +13,39 @@
 
 namespace blink {
 struct WebPluginParams;
-}
+}  // namespace blink
 
 namespace chrome_pdf {
 
 struct ParsedParams {
   ParsedParams();
   ParsedParams(const ParsedParams& other);
+  ParsedParams& operator=(const ParsedParams& other);
+  ParsedParams(ParsedParams&& other);
+  ParsedParams& operator=(ParsedParams&& other);
   ~ParsedParams();
 
-  // The document original URL. Must not be empty.
-  std::string original_url;
-
   // The plugin source URL. Must not be empty.
   std::string src_url;
 
-  // The background color for the PDF viewer.
+  // The document original URL. Must not be empty.
-  absl::optional<SkColor> background_color;
+  std::string original_url;
+
+  // The top-level URL.
+  std::string top_level_url;
 
   // Whether the plugin should occupy the entire frame.
   bool full_frame = false;
 
+  // The background color for the PDF viewer.
+  SkColor background_color = SK_ColorTRANSPARENT;
+
   // Whether to execute JavaScript and maybe XFA.
   PDFiumFormFiller::ScriptOption script_option =
       PDFiumFormFiller::DefaultScriptOption();
+
+  // Whether the PDF was edited previously in annotation mode.
+  bool has_edits = false;
 };
 
 // Creates an `ParsedParams` by parsing a `blink::WebPluginParams`. If

pdf/parsed_params_unittest.cc

@@ -6,8 +6,10 @@
 
 #include <string>
 
-#include "base/strings/string_number_conversions.h"
+#include "pdf/pdfium/pdfium_form_filler.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 #include "third_party/blink/public/platform/web_string.h"
 #include "third_party/blink/public/platform/web_vector.h"
 #include "third_party/blink/public/web/web_plugin_params.h"
@@ -17,103 +19,166 @@ namespace chrome_pdf {
 
 namespace {
 
-constexpr char kDummyOriginalUrl[] = "https://test.com/dummy.pdf";
+using ::testing::AnyOf;
-constexpr char kDummySrcUrl[] = "chrome-extension://dummy-source-url";
 
-constexpr SkColor kNewBackgroundColor = SkColorSetARGB(0xFF, 0x52, 0x56, 0x59);
+constexpr char kFakeSrcUrl[] = "chrome-extension://fake-source-url";
 
-// `kNewBackgroundColor` as a decimal number in string format.
+// Creates a `blink::WebPluginParams` with only required attributes.
-constexpr char kNewBackgroundColorStr[] = "4283586137";
+blink::WebPluginParams CreateMinimalWebPluginParams() {
-
-// Creates a `blink::WebPluginParams` without any URL attributes, namely "src"
-// and "original-url". The return value only contains valid "background-color"
-// and "full-frame" attributes.
-blink::WebPluginParams CreateWebPluginParamsWithoutUrl() {
   blink::WebPluginParams params;
-  params.attribute_names.push_back(blink::WebString("background-color"));
-  params.attribute_values.push_back(blink::WebString(kNewBackgroundColorStr));
-  params.attribute_names.push_back(blink::WebString("full-frame"));
-  params.attribute_values.push_back(blink::WebString(""));
-  return params;
-}
-
-// Creates a `blink::WebPluginParams` with only the URL attributes: "src" and
-// "original-url".
-blink::WebPluginParams CreateWebPluginParamsWithUrls() {
-  blink::WebPluginParams params;
-  params.attribute_names.push_back(blink::WebString("original-url"));
-  params.attribute_values.push_back(blink::WebString(kDummyOriginalUrl));
   params.attribute_names.push_back(blink::WebString("src"));
-  params.attribute_values.push_back(blink::WebString(kDummySrcUrl));
+  params.attribute_values.push_back(blink::WebString(kFakeSrcUrl));
   return params;
 }
 
 }  // namespace
 
-TEST(ParsedParamsTest, ParseValidWebPluginParams) {
+TEST(ParsedParamsTest, ParseWebPluginParamsMinimal) {
-  blink::WebPluginParams params = CreateWebPluginParamsWithoutUrl();
+  blink::WebPluginParams params = CreateMinimalWebPluginParams();
-  params.attribute_names.push_back(blink::WebString("original-url"));
-  params.attribute_values.push_back(blink::WebString(kDummyOriginalUrl));
-  params.attribute_names.push_back(blink::WebString("src"));
-  params.attribute_values.push_back(blink::WebString(kDummySrcUrl));
 
   absl::optional<ParsedParams> result = ParseWebPluginParams(params);
   ASSERT_TRUE(result.has_value());
-  EXPECT_EQ(kDummyOriginalUrl, result->original_url);
+
-  EXPECT_EQ(kDummySrcUrl, result->src_url);
+  EXPECT_EQ(kFakeSrcUrl, result->src_url);
-  ASSERT_TRUE(result->background_color.has_value());
+  EXPECT_EQ(kFakeSrcUrl, result->original_url);
-  EXPECT_EQ(kNewBackgroundColor, result->background_color.value());
+  EXPECT_EQ("", result->top_level_url);
-  EXPECT_TRUE(result->full_frame);
+  EXPECT_FALSE(result->full_frame);
+  EXPECT_EQ(SK_ColorTRANSPARENT, result->background_color);
+  EXPECT_EQ(PDFiumFormFiller::DefaultScriptOption(), result->script_option);
+  EXPECT_FALSE(result->has_edits);
 }
 
 TEST(ParsedParamsTest, ParseWebPluginParamsWithoutSourceUrl) {
-  blink::WebPluginParams params = CreateWebPluginParamsWithoutUrl();
+  blink::WebPluginParams params;
-  params.attribute_names.push_back(blink::WebString("original-url"));
-  params.attribute_values.push_back(blink::WebString(kDummyOriginalUrl));
 
-  // Expect the `ParsedParams` to be invalid due to missing the source URL.
   absl::optional<ParsedParams> result = ParseWebPluginParams(params);
   EXPECT_FALSE(result.has_value());
 }
 
-TEST(ParseParsedParamsTest, ParseWebPluginParamsWithoutOriginalUrl) {
+TEST(ParsedParamsTest, ParseWebPluginParamsWithOriginalUrl) {
-  blink::WebPluginParams params = CreateWebPluginParamsWithoutUrl();
+  blink::WebPluginParams params = CreateMinimalWebPluginParams();
-  params.attribute_names.push_back(blink::WebString("src"));
+  params.attribute_names.push_back(blink::WebString("original-url"));
-  params.attribute_values.push_back(blink::WebString(kDummySrcUrl));
+  params.attribute_values.push_back(
+      blink::WebString("https://example.com/original.pdf"));
 
-  // Expect the `ParsedParams` to be valid and `original_url` to be the same as
-  // `src_url`.
   absl::optional<ParsedParams> result = ParseWebPluginParams(params);
   ASSERT_TRUE(result.has_value());
-  EXPECT_EQ(kDummySrcUrl, result->original_url);
+
-  EXPECT_EQ(kDummySrcUrl, result->src_url);
+  EXPECT_EQ(kFakeSrcUrl, result->src_url);
-  ASSERT_TRUE(result->background_color.has_value());
+  EXPECT_EQ("https://example.com/original.pdf", result->original_url);
-  EXPECT_EQ(kNewBackgroundColor, result->background_color.value());
+}
+
+TEST(ParsedParamsTest, ParseWebPluginParamsWithTopLevelUrl) {
+  blink::WebPluginParams params = CreateMinimalWebPluginParams();
+  params.attribute_names.push_back(blink::WebString("top-level-url"));
+  params.attribute_values.push_back(
+      blink::WebString("https://example.net/top.html"));
+
+  absl::optional<ParsedParams> result = ParseWebPluginParams(params);
+  ASSERT_TRUE(result.has_value());
+
+  EXPECT_EQ("https://example.net/top.html", result->top_level_url);
+}
+
+TEST(ParsedParamsTest, ParseWebPluginParamsWithFullFrame) {
+  blink::WebPluginParams params = CreateMinimalWebPluginParams();
+  params.attribute_names.push_back(blink::WebString("full-frame"));
+  params.attribute_values.push_back(blink::WebString(""));
+
+  absl::optional<ParsedParams> result = ParseWebPluginParams(params);
+  ASSERT_TRUE(result.has_value());
+
   EXPECT_TRUE(result->full_frame);
 }
 
-TEST(ParsedParamsTest, ParseWebPluginParamsWithoutBackgroundColor) {
+TEST(ParsedParamsTest, ParseWebPluginParamsWithFullFrameNonEmpty) {
-  blink::WebPluginParams params = CreateWebPluginParamsWithUrls();
+  blink::WebPluginParams params = CreateMinimalWebPluginParams();
+  params.attribute_names.push_back(blink::WebString("full-frame"));
+  params.attribute_values.push_back(blink::WebString("false"));
 
-  // The `ParsedParams` can still be valid without the background color
-  // attribute.
   absl::optional<ParsedParams> result = ParseWebPluginParams(params);
   ASSERT_TRUE(result.has_value());
-  EXPECT_EQ(kDummyOriginalUrl, result->original_url);
+
-  EXPECT_EQ(kDummySrcUrl, result->src_url);
+  EXPECT_TRUE(result->full_frame);
-  EXPECT_FALSE(result->background_color.has_value());
+}
-  EXPECT_FALSE(result->full_frame);
+
+TEST(ParsedParamsTest, ParseWebPluginParamsWithBackgroundColor) {
+  blink::WebPluginParams params = CreateMinimalWebPluginParams();
+  params.attribute_names.push_back(blink::WebString("background-color"));
+  params.attribute_values.push_back(blink::WebString("4283586137"));
+
+  absl::optional<ParsedParams> result = ParseWebPluginParams(params);
+  ASSERT_TRUE(result.has_value());
+
+  EXPECT_EQ(4283586137, result->background_color);
 }
 
 TEST(ParsedParamsTest, ParseWebPluginParamsWithInvalidBackgroundColor) {
-  blink::WebPluginParams params = CreateWebPluginParamsWithUrls();
+  blink::WebPluginParams params = CreateMinimalWebPluginParams();
   params.attribute_names.push_back(blink::WebString("background-color"));
   params.attribute_values.push_back(blink::WebString("red"));
 
-  // Expect the `ParsedParams` to be invalid because the background color's
-  // attribute value is in the wrong format.
   absl::optional<ParsedParams> result = ParseWebPluginParams(params);
   EXPECT_FALSE(result.has_value());
 }
 
+TEST(ParsedParamsTest, ParseWebPluginParamsWithJavascriptAllow) {
+  blink::WebPluginParams params = CreateMinimalWebPluginParams();
+  params.attribute_names.push_back(blink::WebString("javascript"));
+  params.attribute_values.push_back(blink::WebString("allow"));
+
+  absl::optional<ParsedParams> result = ParseWebPluginParams(params);
+  ASSERT_TRUE(result.has_value());
+
+  EXPECT_THAT(result->script_option,
+              AnyOf(PDFiumFormFiller::ScriptOption::kJavaScript,
+                    PDFiumFormFiller::ScriptOption::kJavaScriptAndXFA));
+}
+
+TEST(ParsedParamsTest, ParseWebPluginParamsWithJavascriptEmpty) {
+  blink::WebPluginParams params = CreateMinimalWebPluginParams();
+  params.attribute_names.push_back(blink::WebString("javascript"));
+  params.attribute_values.push_back(blink::WebString(""));
+
+  absl::optional<ParsedParams> result = ParseWebPluginParams(params);
+  ASSERT_TRUE(result.has_value());
+
+  EXPECT_EQ(PDFiumFormFiller::ScriptOption::kNoJavaScript,
+            result->script_option);
+}
+
+TEST(ParsedParamsTest, ParseWebPluginParamsWithJavascriptNonEmpty) {
+  blink::WebPluginParams params = CreateMinimalWebPluginParams();
+  params.attribute_names.push_back(blink::WebString("javascript"));
+  params.attribute_values.push_back(blink::WebString("true"));
+
+  absl::optional<ParsedParams> result = ParseWebPluginParams(params);
+  ASSERT_TRUE(result.has_value());
+
+  EXPECT_EQ(PDFiumFormFiller::ScriptOption::kNoJavaScript,
+            result->script_option);
+}
+
+TEST(ParsedParamsTest, ParseWebPluginParamsWithHasEdits) {
+  blink::WebPluginParams params = CreateMinimalWebPluginParams();
+  params.attribute_names.push_back(blink::WebString("has-edits"));
+  params.attribute_values.push_back(blink::WebString(""));
+
+  absl::optional<ParsedParams> result = ParseWebPluginParams(params);
+  ASSERT_TRUE(result.has_value());
+
+  EXPECT_TRUE(result->has_edits);
+}
+
+TEST(ParsedParamsTest, ParseWebPluginParamsWithHasEditsNonEmpty) {
+  blink::WebPluginParams params = CreateMinimalWebPluginParams();
+  params.attribute_names.push_back(blink::WebString("has-edits"));
+  params.attribute_values.push_back(blink::WebString("false"));
+
+  absl::optional<ParsedParams> result = ParseWebPluginParams(params);
+  ASSERT_TRUE(result.has_value());
+
+  EXPECT_TRUE(result->has_edits);
+}
+
 }  // namespace chrome_pdf

pdf/pdf_view_web_plugin.cc

@@ -308,9 +308,10 @@ bool PdfViewWebPlugin::InitializeCommon(
   if (!params.has_value())
     return false;
 
-  PerProcessInitializer::GetInstance().Acquire();
+  // TODO(crbug.com/1232152): Set crash keys like
+  // `ppapi::proxy::PDFResource::SetCrashData()`.
 
-  // TODO(crbug.com/1257666): Implement "has-edits" support.
+  PerProcessInitializer::GetInstance().Acquire();
   InitializeBase(
       engine ? std::move(engine)
              : std::make_unique<PDFiumEngine>(this, params->script_option),
@@ -318,9 +319,8 @@ bool PdfViewWebPlugin::InitializeCommon(
       /*src_url=*/params->src_url,
       /*original_url=*/params->original_url,
       /*full_frame=*/params->full_frame,
-      /*background_color=*/
+      /*background_color=*/params->background_color,
-      params->background_color.value_or(SK_ColorTRANSPARENT),
+      /*has_edits=*/params->has_edits);
-      /*has_edits=*/false);
   return true;
 }