[unseasoned-pdf] Swap plugin attributes for original URL and stream URL.

For the Pepper-free plugin, "src" attribute must be the stream URL.
However the current Pepper implementation uses "src" for the document's
original URL and "stream-url" for the stream URL.

To avoid switching attribute names depending on whether the Pepper-free
plugin is enabled, this CL adds a new plugin attribute "original-url"
for the document's original URL, and uses "src" for the plugin's source
URL (stream URL). Some variables such as `stream_url` have been renamed
"src_url" to avoid confusion.

For a internal PDF plugin, this change will result in WebPluginParams's
`url` field holding the plugin's stream URL instead of the original URL.
Since original URL is needed to determine the plugin's origin, this CL
resets `url` field with the original URL before creating a PDF plugin.

Fixed: 1223396
Change-Id: Iaa6b6874800ae7ae31e1e94fe0c3fb8764a44565
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2998157
Commit-Queue: Hui Yingst <nigi@chromium.org>
Reviewed-by: dpapad <dpapad@chromium.org>
Reviewed-by: K. Moon <kmoon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#898293}

chrome/browser/resources/pdf/controller.js

@@ -466,7 +466,7 @@ export class PluginController {
   async load(fileName, data) {
     const url = URL.createObjectURL(new Blob([data]));
     this.plugin_.removeAttribute('headers');
-    this.plugin_.setAttribute('stream-url', url);
+    this.plugin_.setAttribute('src', url);
     this.plugin_.setAttribute('has-edits', '');
     this.plugin_.style.display = 'block';
     try {

chrome/browser/resources/pdf/pdf_viewer_base.js

@@ -160,10 +160,10 @@ export class PDFViewerBaseElement extends PolymerElement {
    * @private
    */
   createPlugin_(isPrintPreview) {
-    // Create the plugin object dynamically so we can set its src. The plugin
-    // element is sized to fill the entire window and is set to be fixed
-    // positioning, acting as a viewport. The plugin renders into this viewport
-    // according to the scroll position of the window.
+    // Create the plugin object dynamically. The plugin element is sized to
+    // fill the entire window and is set to be fixed positioning, acting as a
+    // viewport. The plugin renders into this viewport according to the scroll
+    // position of the window.
     const plugin =
         /** @type {!HTMLEmbedElement} */ (document.createElement('embed'));
 
@@ -174,9 +174,8 @@ export class PDFViewerBaseElement extends PolymerElement {
     plugin.id = 'plugin';
     plugin.type = 'application/x-google-chrome-pdf';
 
-    plugin.setAttribute('src', this.originalUrl);
-    plugin.setAttribute(
-        'stream-url', this.browserApi.getStreamInfo().streamUrl);
+    plugin.setAttribute('original-url', this.originalUrl);
+    plugin.setAttribute('src', this.browserApi.getStreamInfo().streamUrl);
     let headers = '';
     for (const header in this.browserApi.getStreamInfo().responseHeaders) {
       headers += header + ': ' +

chrome/renderer/chrome_content_renderer_client.cc

@@ -1041,9 +1041,20 @@ WebPlugin* ChromeContentRendererClient::CreatePlugin(
           break;
         }
 
-#if BUILDFLAG(ENABLE_PDF_UNSEASONED)
         if (info.name ==
             ASCIIToUTF16(ChromeContentClient::kPDFInternalPluginName)) {
+          // For a PDF plugin, `params.url` holds the plugin's stream url. If
+          // `params` contains an 'original-url' attribute, reset `params.url`
+          // with its original URL value so that it can be used to determine
+          // the plugin's origin.
+          for (size_t i = 0; i < params.attribute_names.size(); ++i) {
+            if (params.attribute_names[i] == "original-url") {
+              params.url = GURL(params.attribute_values[i].Utf16());
+              break;
+            }
+          }
+
+#if BUILDFLAG(ENABLE_PDF_UNSEASONED)
           // Create unseasoned PDF plugin directly, for development purposes.
           // TODO(crbug.com/1123621): Implement a more permanent solution once
           // the new PDF viewer process model is approved and in place.
@@ -1054,12 +1065,11 @@ WebPlugin* ChromeContentRendererClient::CreatePlugin(
 #if BUILDFLAG(ENABLE_PRINTING)
           print_client =
               std::make_unique<ChromePdfViewWebPluginPrintClient>(render_frame);
-#endif
+#endif  // BUILDFLAG(ENABLE_PRINTING)
           return new chrome_pdf::PdfViewWebPlugin(
               std::move(pdf_service_remote), std::move(print_client), params);
-        }
 #endif  // BUILDFLAG(ENABLE_PDF_UNSEASONED)
-
+        }
         return render_frame->CreatePlugin(info, params);
       }
       case chrome::mojom::PluginStatus::kDisabled: {

chrome/test/data/pdf/basic_test.js

@@ -32,7 +32,7 @@ const tests = [
     chrome.test.assertEq('embed', plugin.localName);
 
     chrome.test.assertTrue(
-        plugin.getAttribute('src').indexOf('/pdf/test.pdf') !== -1);
+        plugin.getAttribute('original-url').indexOf('/pdf/test.pdf') !== -1);
     chrome.test.succeed();
   },
 

chrome/test/data/pdf/redirects_fail_test.js

@@ -16,8 +16,8 @@ function createPluginForUrl(streamUrl, url, headers, progressCallback) {
     }
   }, false);
 
-  plugin.setAttribute('src', url);
-  plugin.setAttribute('stream-url', streamUrl);
+  plugin.setAttribute('original-url', url);
+  plugin.setAttribute('src', streamUrl);
   plugin.setAttribute('full-frame', '');
   plugin.setAttribute('headers', headers);
   document.body.appendChild(plugin);

pdf/out_of_process_instance.cc

@@ -464,15 +464,15 @@ bool OutOfProcessInstance::Init(uint32_t argc,
   PDFiumFormFiller::ScriptOption script_option =
       PDFiumFormFiller::DefaultScriptOption();
   bool has_edits = false;
-  const char* stream_url = nullptr;
+  const char* src_url = nullptr;
   const char* original_url = nullptr;
   const char* top_level_url = nullptr;
   const char* headers = nullptr;
   for (uint32_t i = 0; i < argc; ++i) {
-    if (strcmp(argn[i], "src") == 0) {
+    if (strcmp(argn[i], "original-url") == 0) {
       original_url = argv[i];
-    } else if (strcmp(argn[i], "stream-url") == 0) {
-      stream_url = argv[i];
+    } else if (strcmp(argn[i], "src") == 0) {
+      src_url = argv[i];
     } else if (strcmp(argn[i], "top-level-url") == 0) {
       top_level_url = argv[i];
     } else if (strcmp(argn[i], "headers") == 0) {
@@ -492,11 +492,11 @@ bool OutOfProcessInstance::Init(uint32_t argc,
     }
   }
 
-  if (!original_url)
+  if (!src_url)
     return false;
 
-  if (!stream_url)
-    stream_url = original_url;
+  if (!original_url)
+    original_url = src_url;
 
   InitializeEngine(std::make_unique<PDFiumEngine>(this, script_option));
 
@@ -507,7 +507,7 @@ bool OutOfProcessInstance::Init(uint32_t argc,
   if (IsPrintPreview())
     return true;
 
-  LoadUrl(stream_url, /*is_print_preview=*/false);
+  LoadUrl(src_url, /*is_print_preview=*/false);
   set_url(original_url);
 
   // Not all edits go through the PDF plugin's form filler. The plugin instance

pdf/parsed_params.cc

@@ -22,10 +22,10 @@ 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") {
+    if (params.attribute_names[i] == "original-url") {
       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] == "src") {
+      result.src_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") {
@@ -38,11 +38,12 @@ absl::optional<ParsedParams> ParseWebPluginParams(
     }
   }
 
-  if (result.original_url.empty())
+  if (result.src_url.empty())
     return absl::nullopt;
 
-  if (result.stream_url.empty())
-    result.stream_url = result.original_url;
+  if (result.original_url.empty()) {
+    result.original_url = result.src_url;
+  }
 
   return result;
 }

pdf/parsed_params.h

@@ -21,11 +21,11 @@ struct ParsedParams {
   ParsedParams(const ParsedParams& other);
   ~ParsedParams();
 
-  // Document URL. Must not be empty.
+  // The document original URL. Must not be empty.
   std::string original_url;
 
-  // Document stream URL. Must not be empty.
-  std::string stream_url;
+  // The plugin source URL. Must not be empty.
+  std::string src_url;
 
   // The background color for the PDF viewer.
   absl::optional<SkColor> background_color;

pdf/parsed_params_unittest.cc

@@ -18,7 +18,7 @@ namespace chrome_pdf {
 namespace {
 
 constexpr char kDummyOriginalUrl[] = "https://test.com/dummy.pdf";
-constexpr char kDummyStreamUrl[] = "chrome-extension://dummy-stream-url";
+constexpr char kDummySrcUrl[] = "chrome-extension://dummy-source-url";
 
 constexpr SkColor kNewBackgroundColor = SkColorSetARGB(0xFF, 0x52, 0x56, 0x59);
 
@@ -26,8 +26,8 @@ constexpr SkColor kNewBackgroundColor = SkColorSetARGB(0xFF, 0x52, 0x56, 0x59);
 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.
+// 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"));
@@ -38,13 +38,13 @@ blink::WebPluginParams CreateWebPluginParamsWithoutUrl() {
 }
 
 // Creates a `blink::WebPluginParams` with only the URL attributes: "src" and
-// "stream-url".
+// "original-url".
 blink::WebPluginParams CreateWebPluginParamsWithUrls() {
   blink::WebPluginParams params;
-  params.attribute_names.push_back(blink::WebString("src"));
+  params.attribute_names.push_back(blink::WebString("original-url"));
   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));
+  params.attribute_names.push_back(blink::WebString("src"));
+  params.attribute_values.push_back(blink::WebString(kDummySrcUrl));
   return params;
 }
 
@@ -52,41 +52,41 @@ blink::WebPluginParams CreateWebPluginParamsWithUrls() {
 
 TEST(ParsedParamsTest, ParseValidWebPluginParams) {
   blink::WebPluginParams params = CreateWebPluginParamsWithoutUrl();
-  params.attribute_names.push_back(blink::WebString("src"));
+  params.attribute_names.push_back(blink::WebString("original-url"));
   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));
+  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(kDummyStreamUrl, result->stream_url);
+  EXPECT_EQ(kDummySrcUrl, result->src_url);
   ASSERT_TRUE(result->background_color.has_value());
   EXPECT_EQ(kNewBackgroundColor, result->background_color.value());
   EXPECT_TRUE(result->full_frame);
 }
 
-TEST(ParsedParamsTest, ParseWebPluginParamsWithoutOriginalUrl) {
+TEST(ParsedParamsTest, ParseWebPluginParamsWithoutSourceUrl) {
   blink::WebPluginParams params = CreateWebPluginParamsWithoutUrl();
-  params.attribute_names.push_back(blink::WebString("stream-url"));
-  params.attribute_values.push_back(blink::WebString(kDummyStreamUrl));
+  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 original URL.
+  // 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, ParseWebPluginParamsWithoutStreamUrl) {
+TEST(ParseParsedParamsTest, ParseWebPluginParamsWithoutOriginalUrl) {
   blink::WebPluginParams params = CreateWebPluginParamsWithoutUrl();
   params.attribute_names.push_back(blink::WebString("src"));
-  params.attribute_values.push_back(blink::WebString(kDummyOriginalUrl));
+  params.attribute_values.push_back(blink::WebString(kDummySrcUrl));
 
-  // Expect the `ParsedParams` to be valid and `stream_url` to be the same as
-  // `original_url`.
+  // 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(kDummyOriginalUrl, result->original_url);
-  EXPECT_EQ(kDummyOriginalUrl, result->stream_url);
+  EXPECT_EQ(kDummySrcUrl, result->original_url);
+  EXPECT_EQ(kDummySrcUrl, result->src_url);
   ASSERT_TRUE(result->background_color.has_value());
   EXPECT_EQ(kNewBackgroundColor, result->background_color.value());
   EXPECT_TRUE(result->full_frame);
@@ -100,7 +100,7 @@ TEST(ParsedParamsTest, ParseWebPluginParamsWithoutBackgroundColor) {
   absl::optional<ParsedParams> result = ParseWebPluginParams(params);
   ASSERT_TRUE(result.has_value());
   EXPECT_EQ(kDummyOriginalUrl, result->original_url);
-  EXPECT_EQ(kDummyStreamUrl, result->stream_url);
+  EXPECT_EQ(kDummySrcUrl, result->src_url);
   EXPECT_FALSE(result->background_color.has_value());
   EXPECT_FALSE(result->full_frame);
 }

pdf/pdf_view_web_plugin.cc

@@ -264,7 +264,7 @@ bool PdfViewWebPlugin::InitializeCommon(
   PerProcessInitializer::GetInstance().Acquire();
   InitializeEngine(std::make_unique<PDFiumEngine>(
       this, PDFiumFormFiller::ScriptOption::kNoJavaScript));
-  LoadUrl(params->stream_url, /*is_print_preview=*/false);
+  LoadUrl(params->src_url, /*is_print_preview=*/false);
   set_url(params->original_url);
   post_message_sender_.set_container(Container());
   return true;