[unseasoned-pdf] Use a helper for parsing in PdfViewWebPlugin.

This CL adds a helper ParseWebPluginParams() which creates a new struct
`ParsedParams` from a given `blink::WebPluginParams`, and adds unit
tests for them.

By replacing the process of parsing `initial_params_` with the helper,
this CL increases PdfViewWebPlugin's test coverage.

Also fix an IWYU issue in //pdf/pdf_view_web_plugin_unittest.cc.

Bug: 1199558
Change-Id: Id2eca7bb046e987bb1b30aa34d8e03045a376f26
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2951783
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Hui Yingst <nigi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#891730}

pdf/BUILD.gn

@@ -118,6 +118,8 @@ if (enable_pdf) {
       "paint_manager.h",
       "paint_ready_rect.cc",
       "paint_ready_rect.h",
+      "parsed_params.cc",
+      "parsed_params.h",
       "pdf_engine.h",
       "pdf_init.cc",
       "pdf_init.h",
@@ -402,6 +404,7 @@ if (enable_pdf) {
       "document_loader_impl_unittest.cc",
       "draw_utils/coordinates_unittest.cc",
       "page_orientation_unittest.cc",
+      "parsed_params_unittest.cc",
       "pdf_transform_unittest.cc",
       "pdf_utils/dates_unittest.cc",
       "pdf_view_plugin_base_unittest.cc",

pdf/parsed_params.cc

@@ -0,0 +1,50 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "pdf/parsed_params.h"
+
+#include <string>
+
+#include "base/strings/string_number_conversions.h"
+#include "third_party/blink/public/platform/web_string.h"
+#include "third_party/blink/public/web/web_plugin_params.h"
+
+namespace chrome_pdf {
+
+ParsedParams::ParsedParams() = default;
+
+ParsedParams::ParsedParams(const ParsedParams& other) = default;
+
+ParsedParams::~ParsedParams() = default;
+
+absl::optional<ParsedParams> ParseWebPluginParams(
+    const blink::WebPluginParams& params) {
+  ParsedParams result;
+  for (size_t i = 0; i < params.attribute_names.size(); ++i) {
+    if (params.attribute_names[i] == "src") {
+      result.original_url = params.attribute_values[i].Utf8();
+    } else if (params.attribute_names[i] == "stream-url") {
+      result.stream_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)) {
+        return absl::nullopt;
+      }
+      result.background_color = background_color;
+    }
+  }
+
+  if (result.original_url.empty())
+    return absl::nullopt;
+
+  if (result.stream_url.empty())
+    result.stream_url = result.original_url;
+
+  return result;
+}
+
+}  // namespace chrome_pdf

pdf/parsed_params.h

@@ -0,0 +1,44 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef PDF_PARSED_PARAMS_H_
+#define PDF_PARSED_PARAMS_H_
+
+#include <string>
+
+#include "third_party/abseil-cpp/absl/types/optional.h"
+#include "third_party/skia/include/core/SkColor.h"
+
+namespace blink {
+struct WebPluginParams;
+}
+
+namespace chrome_pdf {
+
+struct ParsedParams {
+  ParsedParams();
+  ParsedParams(const ParsedParams& other);
+  ~ParsedParams();
+
+  // Document URL. Must not be empty.
+  std::string original_url;
+
+  // Document stream URL. Must not be empty.
+  std::string stream_url;
+
+  // The background color for the PDF viewer.
+  absl::optional<SkColor> background_color;
+
+  // Whether the plugin should occupy the entire frame.
+  bool full_frame = false;
+};
+
+// Creates an `ParsedParams` by parsing a `blink::WebPluginParams`. If
+// `blink::WebPluginParams` is invalid, returns absl::nullopt.
+absl::optional<ParsedParams> ParseWebPluginParams(
+    const blink::WebPluginParams& params);
+
+}  // namespace chrome_pdf
+
+#endif  // PDF_PARSED_PARAMS_H_

pdf/parsed_params_unittest.cc

@@ -0,0 +1,119 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "pdf/parsed_params.h"
+
+#include <string>
+
+#include "base/strings/string_number_conversions.h"
+#include "testing/gtest/include/gtest/gtest.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"
+#include "third_party/skia/include/core/SkColor.h"
+
+namespace chrome_pdf {
+
+namespace {
+
+constexpr char kDummyOriginalUrl[] = "https://test.com/dummy.pdf";
+constexpr char kDummyStreamUrl[] = "chrome-extension://dummy-stream-url";
+
+constexpr SkColor kNewBackgroundColor = SkColorSetARGB(0xFF, 0x52, 0x56, 0x59);
+
+// `kNewBackgroundColor` as a decimal number in string format.
+constexpr char kNewBackgroundColorStr[] = "4283586137";
+
+// Creates a `blink::WebPluginParams` without any URL attributes, namely "src"
+// and "stream-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
+// "stream-url".
+blink::WebPluginParams CreateWebPluginParamsWithUrls() {
+  blink::WebPluginParams params;
+  params.attribute_names.push_back(blink::WebString("src"));
+  params.attribute_values.push_back(blink::WebString(kDummyOriginalUrl));
+  params.attribute_names.push_back(blink::WebString("stream-url"));
+  params.attribute_values.push_back(blink::WebString(kDummyStreamUrl));
+  return params;
+}
+
+}  // namespace
+
+TEST(ParsedParamsTest, ParseValidWebPluginParams) {
+  blink::WebPluginParams params = CreateWebPluginParamsWithoutUrl();
+  params.attribute_names.push_back(blink::WebString("src"));
+  params.attribute_values.push_back(blink::WebString(kDummyOriginalUrl));
+  params.attribute_names.push_back(blink::WebString("stream-url"));
+  params.attribute_values.push_back(blink::WebString(kDummyStreamUrl));
+
+  absl::optional<ParsedParams> result = ParseWebPluginParams(params);
+  ASSERT_TRUE(result.has_value());
+  EXPECT_EQ(kDummyOriginalUrl, result->original_url);
+  EXPECT_EQ(kDummyStreamUrl, result->stream_url);
+  ASSERT_TRUE(result->background_color.has_value());
+  EXPECT_EQ(kNewBackgroundColor, result->background_color.value());
+  EXPECT_TRUE(result->full_frame);
+}
+
+TEST(ParsedParamsTest, ParseWebPluginParamsWithoutOriginalUrl) {
+  blink::WebPluginParams params = CreateWebPluginParamsWithoutUrl();
+  params.attribute_names.push_back(blink::WebString("stream-url"));
+  params.attribute_values.push_back(blink::WebString(kDummyStreamUrl));
+
+  // Expect the `ParsedParams` to be invalid due to missing the original URL.
+  absl::optional<ParsedParams> result = ParseWebPluginParams(params);
+  EXPECT_FALSE(result.has_value());
+}
+
+TEST(ParseParsedParamsTest, ParseWebPluginParamsWithoutStreamUrl) {
+  blink::WebPluginParams params = CreateWebPluginParamsWithoutUrl();
+  params.attribute_names.push_back(blink::WebString("src"));
+  params.attribute_values.push_back(blink::WebString(kDummyOriginalUrl));
+
+  // Expect the `ParsedParams` to be valid and `stream_url` to be the same as
+  // `original_url`.
+  absl::optional<ParsedParams> result = ParseWebPluginParams(params);
+  ASSERT_TRUE(result.has_value());
+  EXPECT_EQ(kDummyOriginalUrl, result->original_url);
+  EXPECT_EQ(kDummyOriginalUrl, result->stream_url);
+  ASSERT_TRUE(result->background_color.has_value());
+  EXPECT_EQ(kNewBackgroundColor, result->background_color.value());
+  EXPECT_TRUE(result->full_frame);
+}
+
+TEST(ParsedParamsTest, ParseWebPluginParamsWithoutBackgroundColor) {
+  blink::WebPluginParams params = CreateWebPluginParamsWithUrls();
+
+  // 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(kDummyStreamUrl, result->stream_url);
+  EXPECT_FALSE(result->background_color.has_value());
+  EXPECT_FALSE(result->full_frame);
+}
+
+TEST(ParsedParamsTest, ParseWebPluginParamsWithInvalidBackgroundColor) {
+  blink::WebPluginParams params = CreateWebPluginParamsWithUrls();
+  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());
+}
+
+}  // namespace chrome_pdf

pdf/pdf_view_web_plugin.cc

@@ -30,6 +30,7 @@
 #include "content/public/renderer/render_frame.h"
 #include "net/cookies/site_for_cookies.h"
 #include "pdf/accessibility_structs.h"
+#include "pdf/parsed_params.h"
 #include "pdf/pdf_engine.h"
 #include "pdf/pdf_init.h"
 #include "pdf/pdfium/pdfium_engine.h"
@@ -59,7 +60,6 @@
 #include "third_party/blink/public/web/web_local_frame.h"
 #include "third_party/blink/public/web/web_plugin_container.h"
 #include "third_party/blink/public/web/web_plugin_params.h"
-#include "third_party/skia/include/core/SkColor.h"
 #include "third_party/skia/include/core/SkRect.h"
 #include "third_party/skia/include/core/SkRefCnt.h"
 #include "ui/base/cursor/cursor.h"
@@ -188,42 +188,23 @@ bool PdfViewWebPlugin::InitializeCommon(
     std::unique_ptr<ContainerWrapper> container_wrapper) {
   container_wrapper_ = std::move(container_wrapper);
 
-  std::string original_url;
-  std::string stream_url;
-  // TODO(https://crbug.com/1199558): Make a helper method to parse
-  // `initial_params_` without processing the results, and add a unit test for
-  // it.
-  for (size_t i = 0; i < initial_params_.attribute_names.size(); ++i) {
-    if (initial_params_.attribute_names[i] == "src") {
-      original_url = initial_params_.attribute_values[i].Utf8();
-    } else if (initial_params_.attribute_names[i] == "stream-url") {
-      stream_url = initial_params_.attribute_values[i].Utf8();
-    } else if (initial_params_.attribute_names[i] == "full-frame") {
-      set_full_frame(true);
-    } else if (initial_params_.attribute_names[i] == "background-color") {
-      SkColor background_color;
-      if (!base::StringToUint(initial_params_.attribute_values[i].Utf8(),
-                              &background_color)) {
-        return false;
-      }
-      SetBackgroundColor(background_color);
-    }
-  }
+  absl::optional<ParsedParams> params = ParseWebPluginParams(initial_params_);
 
-  // Contents of `initial_params_` no longer needed.
+  // The contents of `initial_params_` are no longer needed.
   initial_params_ = {};
 
-  if (original_url.empty())
+  if (!params.has_value())
     return false;
 
-  if (stream_url.empty())
-    stream_url = original_url;
+  set_full_frame(params->full_frame);
+  if (params->background_color.has_value())
+    SetBackgroundColor(params->background_color.value());
 
   PerProcessInitializer::GetInstance().Acquire();
   InitializeEngine(std::make_unique<PDFiumEngine>(
       this, PDFiumFormFiller::ScriptOption::kNoJavaScript));
-  LoadUrl(stream_url, /*is_print_preview=*/false);
-  set_url(original_url);
+  LoadUrl(params->stream_url, /*is_print_preview=*/false);
+  set_url(params->original_url);
   post_message_sender_.set_container(Container());
   return true;
 }

pdf/pdf_view_web_plugin_unittest.cc

@@ -17,6 +17,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/platform/web_string.h"
 #include "third_party/blink/public/platform/web_text_input_type.h"
+#include "third_party/blink/public/platform/web_vector.h"
 #include "third_party/blink/public/web/web_associated_url_loader.h"
 #include "third_party/blink/public/web/web_plugin_params.h"
 #include "third_party/skia/include/core/SkBitmap.h"