New issue
Advanced search Search tips

Issue 878871 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Since Chrome 68.x, PDF downloads fail because Chrome think the Content Security Policy is blocking the download.

Reported by patrickf...@gmail.com, Aug 29

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36

Example URL:
https://smart911.rave411.com/rcv/login

Steps to reproduce the problem:
Please enable the chrome option "Download PDF files instead of automatically opening them in Chrome" under PDF documents.

1)When in click on a ticket from the ticket list on the left side of the page.
2)This will open a smartlet in the middle of the page. (Header of smartlet is Smart911 Profile.
3)On the addresses section of the smartlet, click on first address (74 Winslow ave) which will display meta data for that address.
4) Click the "view uploads document" link next to the label "Building Plans", this will download a pdf file.
5)Click the large "CLOSE DOCUMENT" button.

What is the expected behavior?
PDF downloads to the /downloads folder.

What went wrong?
Erroneously the browser (since 68.x) blocks the PDF download due to what it thinks is a breach of the Content Security Policy.

Did this work before? Yes Works in all Chrome versions 67.x and back. Stopped working in 68.x.

Chrome version: 68.0.3440.106  Channel: stable
OS Version: 10.0
Flash Version: 

Here is the Content Security Policy.

Content-Security-Policy: default-src 'self'; font-src 'self' data: *.gstatic.com; frame-src 'self' data: blob: www.nbcnews.com www.youtube.com; connect-src 'self' *.mapbox.com wss://smart911.rave411.com translate.googleapis.com; child-src 'self' *.youtube.com wss://smart911.rave411.com; object-src 'self' 'unsafe-inline' blob: data: smart911.rave411.com; style-src 'self' 'unsafe-inline' *.googleapis.com; img-src * blob: data: *.mapbox.com *.rave411.com; script-src 'self' 'unsafe-inline' 'unsafe-eval' *.google.com *.google-analytics.com *.googleapis.com *.mapbox.com www.sc.pages05.net
 
Chrome-CSP3.png
355 KB View Download
StepsToRepro.webm
6.3 MB View Download
Labels: Needs-Triage-M68 Needs-Bisect
Cc: swarnasree.mukkala@chromium.org
Labels: Triaged-ET Needs-Feedback
Tried testing the issue on reported chrome version #68.0.3440.106 using windows 10, by following the below steps.

Steps:
=====
1.Launched chrome.
2.Navigated to chrome://settings.
3.Enabled the chrome option "Download PDF files instead of automatically opening them in Chrome" under PDF documents.
4.Navigated to 'https://smart911.rave411.com/rcv/login', but the site did not open and error 'The site cannot be reached' is displayed.

Attached screencast for referece.
@reporter : Could you please check the attched screencast and let us know if anything is being missed here. Request you to provide a sample URL/file that reproduces the issue so that it would be really helpful for triaging the issue.

Thanks.!
878871.mp4
2.7 MB View Download
swarnasree.mukkala@chromium.org, thanks for having a look. The site is definitely up and the important thing when reproducing this is to have the Dev Tools JS Console open where you will see the exception as soon as you click the link to open the PDF.

All I can ask is to try again as the link is correct and site is up and running. https://smart911.rave411.com/rcv/login
Project Member

Comment 4 by sheriffbot@chromium.org, Aug 30

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: Needs-Feedback
Tried testing the issue on reported chrome version #68.0.3440.106, by using Windows 10 following steps as per comment#0.
Observed that the site cannot be reached.

Attached screencast for reference.
@reporter: Could you please provide any other sample URL/File that reproduces the issue, so that it would be really helpful for further triaging.
Thanks.!
878871(1).mp4
1.9 MB View Download
swarnasree.mukkala@chromium.org, I'm at a loss. The site is up and running, I can get to the site from my AWS instances, from my work site and home. This seems like a network issue from your side. The error from the browser seems like it never even made it to the site. Can you try some of our other sites?
https://www.smart911.com
https://smart911.rave411.com
https://www.rave411.com
https://www.getrave.com

Let me know if any or all work?

Thank you,
Patrick


Project Member

Comment 7 by sheriffbot@chromium.org, Aug 31

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Components: -Internals>Network UI>Browser>Downloads Internals>Plugins>PDF Blink>SecurityFeature>ContentSecurityPolicy
Labels: Needs-Feedback
As per comment #6, tried opening the given links observed that the first two links gets the error as "The site cannot be reached" and second two links opens. Below are the observations when these latter two links opens.

Observations:
===========
1.They navigate to a page where a page loads with a text box to enter a site value.
2.Entered the site value as "smart911" .
3.They navigate to the page where it asks for a login with username and password, tried searching how to register but could not find it out. 
Note: It happens for all sites which appear in suggestions while entering "smart911" in site value

Attached screencast for reference.
@reporter : Could you please provide sample credentials to login into these sites.
Thanks.!
878871(2).mp4
1.9 MB View Download
Microsoft Windows [Version 10.0.17134.228]
(c) 2018 Microsoft Corporation. All rights reserved.

C:\Users\Pat>nslookup smart911.rave411.com
Server:  google-public-dns-a.google.com
Address:  8.8.8.8

Non-authoritative answer:
Name:    smart911.rave411.com
Address:  205.237.106.30

Can you do an NSLookup on the 2 site that you cannot connect to and compare results to mine shown below?

C:\Users\Pat>nslookup www.smart911.com
Server:  google-public-dns-a.google.com
Address:  8.8.8.8

Non-authoritative answer:
Name:    www.smart911.com
Address:  75.98.95.168
Can you do an NSLookup on the 2 sites that you cannot connect to and compare results to mine shown below?

Microsoft Windows [Version 10.0.17134.228]
(c) 2018 Microsoft Corporation. All rights reserved.

C:\Users\Pat>nslookup smart911.rave411.com
Server:  google-public-dns-a.google.com
Address:  8.8.8.8

Non-authoritative answer:
Name:    smart911.rave411.com
Address:  205.237.106.30


C:\Users\Pat>nslookup www.smart911.com
Server:  google-public-dns-a.google.com
Address:  8.8.8.8

Non-authoritative answer:
Name:    www.smart911.com
Address:  75.98.95.168
Project Member

Comment 12 by sheriffbot@chromium.org, Sep 3

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: Needs-Feedback
@reporter: Before proceeding to further investigation of the issue, could you please confirm if the issue is seen on recent chrome version #69.0.3947.81, as recently the stable has been rolled out to M-69.You can download the latest chrome builds from the link "https://www.chromium.org/getting-involved/dev-channel".

Thanks.!
Same issue in 70.0.3534.4 (Official Build) dev (64-bit) (cohort: Dev)
Project Member

Comment 15 by sheriffbot@chromium.org, Sep 5

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Needs-Bisect
Owner: tommycli@chromium.org
Status: Assigned (was: Unconfirmed)
The issue seems to similar to issue:  879149 . Requesting dev team to check the issue and confirm whether both issues are similar or not and assigning this issue to tommycli for further inputs on the issue.
As per comment#2, comment#5 and comment#9 unable to test the issue at TE-end, hence removing Needs-Bisect label. 

Thanks.!
The provided login link doesn't work. See screenshot.

That being said, if you are trying to force a PDF download, I suggest the following techniques:

I suggest one of four ways to accomplish that (ordered from most preferred to least preferred)

 1. Use the HTTP header Content-Disposition: attachment on the server side: 
    https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition

 2. Or use Content-Type: application/octet-stream on the server 

 3. Or use the HTML5 download attribute: References: 
    https://stackoverflow.com/questions/3077242/force-download-a-pdf-link-using-javascript-ajax-jquery
    https://www.w3schools.com/Tags/att_a_download.asp

 4. There's another pure-Javascript approach listed here: 
    https://stackoverflow.com/questions/3077242/force-download-a-pdf-link-using-javascript-ajax-jquery/29266135#29266135

With any of the above recommendations users should no longer need to Enable chrome://settings/content/pdfDocuments for it to work.

Screenshot from 2018-09-06 08-10-16.png
46.3 KB View Download
Please try again, you just happen to try and connect when server instance was down. Sorry about that. 

This Content Security Policy we currently have is completely proper and only fails with Chrome starting with version 68.x. All other browsers work, IE, Edge, Firefox and Opera.
What should the login credentials be?

I think likely the Content Security Policy warning is a red herring. Are you trying to use an IFRAME to trigger a PDF download?
Creds: chrome / Chromepass!
2018-09-06_11h39_04.png
196 KB View Download
The Content Security Policy error is causing the download to fail. That same console error (not a warning) did not show in chrome 67.x and earlier. The error says it is "refusing" to load data.
Project Member

Comment 22 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad61fb278ef03510d0e5e2070018a05adc34489c

commit ad61fb278ef03510d0e5e2070018a05adc34489c
Author: Tommy C. Li <tommycli@chromium.org>
Date: Fri Oct 05 17:53:23 2018

Click to Open PDF: Put placeholder HTML in IFRAMEs directly

Previously, when the user has a disabled PDF plugin (or no PDF plugin)
and the website has an IFRAMEed PDF, when Click to Open PDF was enabled,
we would inject a fake <object> tag into the IFRAME to force a PDF
plugin placeholder to appear.

This approach is problematic with CSP, as CSP may forbid OBJECT tags
from loading in IFRAMEs.

This CL instead injects the placeholder HTML directly into the IFRAME.

In the IFRAME case, the button is now a plain link, which should allow
the user to click it and download the PDF even if CSP forbids <object>
tags or JavaScript.

It still may trigger a CSP warning if IFRAME is prohibited from running
JavaScript, but that only hurts keyboard-accessibility, and doesn't
prevent the mainstream use case from working.

This CL also prevents the <object> tag from auto-opening the PDF after
download, which was probably a mistake, since that overrides the user
configurable "Always open with system viewer" option.

Bug: 887752,  879149 , 878871
Change-Id: I900c08331d2cfc96ea7cd1cd46ea594445b0920b
Reviewed-on: https://chromium-review.googlesource.com/c/1246956
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597192}
[modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/browser/download/chrome_download_manager_delegate.cc
[modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/browser/pdf/pdf_extension_test.cc
[modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/browser/plugins/pdf_iframe_navigation_throttle.cc
[modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/browser/plugins/pdf_plugin_placeholder_observer.cc
[modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/common/BUILD.gn
[modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/common/DEPS
[delete] https://crrev.com/d92a8619b6a7c29294823c7ccefcbb1d7b43912c/chrome/common/pdf_uma.cc
[add] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/common/pdf_util.cc
[rename] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/common/pdf_util.h
[modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/renderer/plugins/pdf_plugin_placeholder.cc
[modify] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/renderer/resources/plugins/pdf_plugin.html
[add] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/test/data/pdf/pdf_embed_csp.html
[add] https://crrev.com/ad61fb278ef03510d0e5e2070018a05adc34489c/chrome/test/data/pdf/pdf_iframe_csp.html

Project Member

Comment 23 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2a68a0f0255d80a83b49e2709f30818418960f46

commit 2a68a0f0255d80a83b49e2709f30818418960f46
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Fri Oct 05 21:43:19 2018

Revert "Click to Open PDF: Put placeholder HTML in IFRAMEs directly"

This reverts commit ad61fb278ef03510d0e5e2070018a05adc34489c.

Reason for revert: New PDFPluginDisabledTest.IframePdfPlaceholderWithCSP test is failing in https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.13%20Tests%20%28dbg%29/5617



Original change's description:
> Click to Open PDF: Put placeholder HTML in IFRAMEs directly
> 
> Previously, when the user has a disabled PDF plugin (or no PDF plugin)
> and the website has an IFRAMEed PDF, when Click to Open PDF was enabled,
> we would inject a fake <object> tag into the IFRAME to force a PDF
> plugin placeholder to appear.
> 
> This approach is problematic with CSP, as CSP may forbid OBJECT tags
> from loading in IFRAMEs.
> 
> This CL instead injects the placeholder HTML directly into the IFRAME.
> 
> In the IFRAME case, the button is now a plain link, which should allow
> the user to click it and download the PDF even if CSP forbids <object>
> tags or JavaScript.
> 
> It still may trigger a CSP warning if IFRAME is prohibited from running
> JavaScript, but that only hurts keyboard-accessibility, and doesn't
> prevent the mainstream use case from working.
> 
> This CL also prevents the <object> tag from auto-opening the PDF after
> download, which was probably a mistake, since that overrides the user
> configurable "Always open with system viewer" option.
> 
> Bug: 887752,  879149 , 878871
> Change-Id: I900c08331d2cfc96ea7cd1cd46ea594445b0920b
> Reviewed-on: https://chromium-review.googlesource.com/c/1246956
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Reviewed-by: Lei Zhang <thestig@chromium.org>
> Commit-Queue: Tommy Li <tommycli@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#597192}

TBR=thestig@chromium.org,tommycli@chromium.org,jochen@chromium.org,mkwst@chromium.org

Change-Id: I8a5554c0489ec76a4adb35ca93afef9c4354316e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 887752,  879149 , 878871
Reviewed-on: https://chromium-review.googlesource.com/c/1265826
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597315}
[modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/browser/download/chrome_download_manager_delegate.cc
[modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/browser/pdf/pdf_extension_test.cc
[modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/browser/plugins/pdf_iframe_navigation_throttle.cc
[modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/browser/plugins/pdf_plugin_placeholder_observer.cc
[modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/common/BUILD.gn
[modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/common/DEPS
[add] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/common/pdf_uma.cc
[rename] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/common/pdf_uma.h
[delete] https://crrev.com/7dad1c4475b73eacba0029db21683bcda832f8c7/chrome/common/pdf_util.cc
[modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/renderer/plugins/pdf_plugin_placeholder.cc
[modify] https://crrev.com/2a68a0f0255d80a83b49e2709f30818418960f46/chrome/renderer/resources/plugins/pdf_plugin.html
[delete] https://crrev.com/7dad1c4475b73eacba0029db21683bcda832f8c7/chrome/test/data/pdf/pdf_embed_csp.html
[delete] https://crrev.com/7dad1c4475b73eacba0029db21683bcda832f8c7/chrome/test/data/pdf/pdf_iframe_csp.html

Project Member

Comment 24 by bugdroid1@chromium.org, Oct 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/49eacb581082e7a2469b613ad329e5dd75904616

commit 49eacb581082e7a2469b613ad329e5dd75904616
Author: Tommy C. Li <tommycli@chromium.org>
Date: Tue Oct 09 21:19:26 2018

Reland "Click to Open PDF: Put placeholder HTML in IFRAMEs directly"

This is a reland of ad61fb278ef03510d0e5e2070018a05adc34489c

Original change's description:
> Click to Open PDF: Put placeholder HTML in IFRAMEs directly
> 
> Previously, when the user has a disabled PDF plugin (or no PDF plugin)
> and the website has an IFRAMEed PDF, when Click to Open PDF was enabled,
> we would inject a fake <object> tag into the IFRAME to force a PDF
> plugin placeholder to appear.
> 
> This approach is problematic with CSP, as CSP may forbid OBJECT tags
> from loading in IFRAMEs.
> 
> This CL instead injects the placeholder HTML directly into the IFRAME.
> 
> In the IFRAME case, the button is now a plain link, which should allow
> the user to click it and download the PDF even if CSP forbids <object>
> tags or JavaScript.
> 
> It still may trigger a CSP warning if IFRAME is prohibited from running
> JavaScript, but that only hurts keyboard-accessibility, and doesn't
> prevent the mainstream use case from working.
> 
> This CL also prevents the <object> tag from auto-opening the PDF after
> download, which was probably a mistake, since that overrides the user
> configurable "Always open with system viewer" option.
> 
> Bug: 887752,  879149 , 878871
> Change-Id: I900c08331d2cfc96ea7cd1cd46ea594445b0920b
> Reviewed-on: https://chromium-review.googlesource.com/c/1246956
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Reviewed-by: Lei Zhang <thestig@chromium.org>
> Commit-Queue: Tommy Li <tommycli@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#597192}

Bug: 887752,  879149 , 878871
Change-Id: I2778524eac784186b141d7caee3801af3863fd2e
Reviewed-on: https://chromium-review.googlesource.com/c/1269442
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598083}
[modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/browser/download/chrome_download_manager_delegate.cc
[modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/browser/pdf/pdf_extension_test.cc
[modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/browser/plugins/pdf_iframe_navigation_throttle.cc
[modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/browser/plugins/pdf_plugin_placeholder_observer.cc
[modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/common/BUILD.gn
[modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/common/DEPS
[delete] https://crrev.com/47e1665f4f428d7c42ef2373f8e6a9ad8848a990/chrome/common/pdf_uma.cc
[add] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/common/pdf_util.cc
[rename] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/common/pdf_util.h
[modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/renderer/plugins/pdf_plugin_placeholder.cc
[modify] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/renderer/resources/plugins/pdf_plugin.html
[add] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/test/data/pdf/pdf_embed_csp.html
[add] https://crrev.com/49eacb581082e7a2469b613ad329e5dd75904616/chrome/test/data/pdf/pdf_iframe_csp.html

Project Member

Comment 25 by bugdroid1@chromium.org, Oct 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f

commit d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Wed Oct 10 06:26:18 2018

Revert "Reland "Click to Open PDF: Put placeholder HTML in IFRAMEs directly""

This reverts commit 49eacb581082e7a2469b613ad329e5dd75904616.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 598083 as the
culprit for flakes in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vNDllYWNiNTgxMDgyZTdhMjQ2OWI2MTNhZDMyOWU1ZGQ3NTkwNDYxNgw

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.linux/linux-xenial-rel/4027

Sample Failed Step: network_service_browser_tests on Ubuntu-16.04

Sample Flaky Test: PDFPluginDisabledTest.EmbedPdfPlaceholderWithCSP

Original change's description:
> Reland "Click to Open PDF: Put placeholder HTML in IFRAMEs directly"
> 
> This is a reland of ad61fb278ef03510d0e5e2070018a05adc34489c
> 
> Original change's description:
> > Click to Open PDF: Put placeholder HTML in IFRAMEs directly
> > 
> > Previously, when the user has a disabled PDF plugin (or no PDF plugin)
> > and the website has an IFRAMEed PDF, when Click to Open PDF was enabled,
> > we would inject a fake <object> tag into the IFRAME to force a PDF
> > plugin placeholder to appear.
> > 
> > This approach is problematic with CSP, as CSP may forbid OBJECT tags
> > from loading in IFRAMEs.
> > 
> > This CL instead injects the placeholder HTML directly into the IFRAME.
> > 
> > In the IFRAME case, the button is now a plain link, which should allow
> > the user to click it and download the PDF even if CSP forbids <object>
> > tags or JavaScript.
> > 
> > It still may trigger a CSP warning if IFRAME is prohibited from running
> > JavaScript, but that only hurts keyboard-accessibility, and doesn't
> > prevent the mainstream use case from working.
> > 
> > This CL also prevents the <object> tag from auto-opening the PDF after
> > download, which was probably a mistake, since that overrides the user
> > configurable "Always open with system viewer" option.
> > 
> > Bug: 887752,  879149 , 878871
> > Change-Id: I900c08331d2cfc96ea7cd1cd46ea594445b0920b
> > Reviewed-on: https://chromium-review.googlesource.com/c/1246956
> > Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> > Reviewed-by: Lei Zhang <thestig@chromium.org>
> > Commit-Queue: Tommy Li <tommycli@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#597192}
> 
> Bug: 887752,  879149 , 878871
> Change-Id: I2778524eac784186b141d7caee3801af3863fd2e
> Reviewed-on: https://chromium-review.googlesource.com/c/1269442
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Commit-Queue: Tommy Li <tommycli@chromium.org>
> Commit-Queue: Lei Zhang <thestig@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#598083}

Change-Id: Ic7554c10dea8131f70f9ff2d6dbb7202e49a142f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 887752,  879149 , 878871, 892818
Reviewed-on: https://chromium-review.googlesource.com/c/1271929
Cr-Commit-Position: refs/heads/master@{#598224}
[modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/browser/download/chrome_download_manager_delegate.cc
[modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/browser/pdf/pdf_extension_test.cc
[modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/browser/plugins/pdf_iframe_navigation_throttle.cc
[modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/browser/plugins/pdf_plugin_placeholder_observer.cc
[modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/common/BUILD.gn
[modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/common/DEPS
[add] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/common/pdf_uma.cc
[rename] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/common/pdf_uma.h
[delete] https://crrev.com/c27ea6d66efa8d360b86092d746f804ee81b7c8f/chrome/common/pdf_util.cc
[modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/renderer/plugins/pdf_plugin_placeholder.cc
[modify] https://crrev.com/d7eb8b3b3348dd9a702831b443a00e8a61ac5b7f/chrome/renderer/resources/plugins/pdf_plugin.html
[delete] https://crrev.com/c27ea6d66efa8d360b86092d746f804ee81b7c8f/chrome/test/data/pdf/pdf_embed_csp.html
[delete] https://crrev.com/c27ea6d66efa8d360b86092d746f804ee81b7c8f/chrome/test/data/pdf/pdf_iframe_csp.html

Project Member

Comment 26 by bugdroid1@chromium.org, Oct 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a

commit ce661f8f18e8d2a20a0e8dbae259ad8912aa166a
Author: Tommy C. Li <tommycli@chromium.org>
Date: Wed Oct 10 23:33:02 2018

Reland "Reland "Click to Open PDF: Put placeholder HTML in IFRAMEs directly""

This is a reland of 49eacb581082e7a2469b613ad329e5dd75904616

Original change's description:
> Reland "Click to Open PDF: Put placeholder HTML in IFRAMEs directly"
>
> This is a reland of ad61fb278ef03510d0e5e2070018a05adc34489c
>
> Original change's description:
> > Click to Open PDF: Put placeholder HTML in IFRAMEs directly
> >
> > Previously, when the user has a disabled PDF plugin (or no PDF plugin)
> > and the website has an IFRAMEed PDF, when Click to Open PDF was enabled,
> > we would inject a fake <object> tag into the IFRAME to force a PDF
> > plugin placeholder to appear.
> >
> > This approach is problematic with CSP, as CSP may forbid OBJECT tags
> > from loading in IFRAMEs.
> >
> > This CL instead injects the placeholder HTML directly into the IFRAME.
> >
> > In the IFRAME case, the button is now a plain link, which should allow
> > the user to click it and download the PDF even if CSP forbids <object>
> > tags or JavaScript.
> >
> > It still may trigger a CSP warning if IFRAME is prohibited from running
> > JavaScript, but that only hurts keyboard-accessibility, and doesn't
> > prevent the mainstream use case from working.
> >
> > This CL also prevents the <object> tag from auto-opening the PDF after
> > download, which was probably a mistake, since that overrides the user
> > configurable "Always open with system viewer" option.
> >
> > Bug: 887752,  879149 , 878871
> > Change-Id: I900c08331d2cfc96ea7cd1cd46ea594445b0920b
> > Reviewed-on: https://chromium-review.googlesource.com/c/1246956
> > Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> > Reviewed-by: Lei Zhang <thestig@chromium.org>
> > Commit-Queue: Tommy Li <tommycli@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#597192}
>
> Bug: 887752,  879149 , 878871
> Change-Id: I2778524eac784186b141d7caee3801af3863fd2e
> Reviewed-on: https://chromium-review.googlesource.com/c/1269442
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Commit-Queue: Tommy Li <tommycli@chromium.org>
> Commit-Queue: Lei Zhang <thestig@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#598083}

TBR=jochen@chromium.org

Bug: 887752,  879149 , 878871
Change-Id: I6e6053e32a5d8acbeb18a32ba067a885ce915fe2
Reviewed-on: https://chromium-review.googlesource.com/c/1273985
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598570}
[modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/browser/download/chrome_download_manager_delegate.cc
[modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/browser/plugins/pdf_iframe_navigation_throttle.cc
[modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/browser/plugins/pdf_plugin_placeholder_observer.cc
[modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/common/BUILD.gn
[modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/common/DEPS
[delete] https://crrev.com/5ea805f821e4922f5ae39e2ed214ef7d210f4a8e/chrome/common/pdf_uma.cc
[add] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/common/pdf_util.cc
[rename] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/common/pdf_util.h
[modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/renderer/plugins/pdf_plugin_placeholder.cc
[modify] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/renderer/resources/plugins/pdf_plugin.html
[add] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/test/data/pdf/pdf_embed_csp.html
[add] https://crrev.com/ce661f8f18e8d2a20a0e8dbae259ad8912aa166a/chrome/test/data/pdf/pdf_iframe_csp.html

Project Member

Comment 27 by bugdroid1@chromium.org, Oct 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8eb4c77037ba65b78883c5fca2b40411fb8c055c

commit 8eb4c77037ba65b78883c5fca2b40411fb8c055c
Author: Tommy C. Li <tommycli@chromium.org>
Date: Mon Oct 15 20:39:52 2018

Click to Open PDF: Add IFRAME CSP browsertest.

This CL is a followup to:
https://chromium-review.googlesource.com/c/chromium/src/+/1273985

It adds the IFRAME test case, which did not seem flaky.

The EMBED test case is still in-progress to deflake.

Bug: 887752,  879149 , 878871
Change-Id: Iee52d4314539b2d457812a220a42051caeb1bab0
Reviewed-on: https://chromium-review.googlesource.com/c/1281262
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599738}
[modify] https://crrev.com/8eb4c77037ba65b78883c5fca2b40411fb8c055c/chrome/browser/pdf/pdf_extension_test.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Nov 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fac0d44345c4f0a7fc8f4f1464e7e79e1cb0e284

commit fac0d44345c4f0a7fc8f4f1464e7e79e1cb0e284
Author: Tommy C. Li <tommycli@chromium.org>
Date: Thu Nov 15 16:56:48 2018

Plugins: Move WaitForPluginPlaceholder to a PluginTestUtils support

Previously, only PluginPowerSaverBrowserTest had access to the ability
to await the placeholder to be ready.

Now that we want to do this for a PDF placeholder browser test as well,
move this functionality to a test support class.

This is patch 1/3 for this objective.

Bug: 887752,  879149 , 878871
Change-Id: Ib0bc70f21d25ec35874025c0952f7c42555fce5b
Reviewed-on: https://chromium-review.googlesource.com/c/1335781
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608399}
[modify] https://crrev.com/fac0d44345c4f0a7fc8f4f1464e7e79e1cb0e284/chrome/browser/BUILD.gn
[modify] https://crrev.com/fac0d44345c4f0a7fc8f4f1464e7e79e1cb0e284/chrome/browser/plugins/plugin_power_saver_browsertest.cc
[add] https://crrev.com/fac0d44345c4f0a7fc8f4f1464e7e79e1cb0e284/chrome/browser/plugins/plugin_test_utils.cc
[add] https://crrev.com/fac0d44345c4f0a7fc8f4f1464e7e79e1cb0e284/chrome/browser/plugins/plugin_test_utils.h

Project Member

Comment 29 by bugdroid1@chromium.org, Nov 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d89b22434f718dd0f82833b5a38ebc92444a99ea

commit d89b22434f718dd0f82833b5a38ebc92444a99ea
Author: Tommy C. Li <tommycli@chromium.org>
Date: Thu Nov 15 16:57:52 2018

Plugins: Move NotifyPlaceholderReadyForTestingCallback to base class

Move the
LoadablePluginPlaceholder::DidFinishIconRepositionForTestingCallback
method to the PluginPlaceholderBase base class so that the
PDFPluginPlaceholder can also use it.

This is a followup to:
https://chromium-review.googlesource.com/c/chromium/src/+/1335781

This is patch 2/3 for this objective.

Bug: 887752,  879149 , 878871
Change-Id: I14f2205d0d8d7c5fcd14fc2eac84e2b7fd1d52dc
Reviewed-on: https://chromium-review.googlesource.com/c/1335997
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608401}
[modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/chrome/renderer/plugins/chrome_plugin_placeholder.cc
[modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/chrome/renderer/plugins/pdf_plugin_placeholder.cc
[modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/chrome/renderer/resources/plugins/blocked_plugin.html
[modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/chrome/renderer/resources/plugins/pdf_plugin.html
[modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/chrome/renderer/resources/plugins/plugin_poster.html
[modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/chrome/renderer/resources/plugins/prefer_html_plugin.html
[modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/components/plugins/renderer/loadable_plugin_placeholder.cc
[modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/components/plugins/renderer/loadable_plugin_placeholder.h
[modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/components/plugins/renderer/plugin_placeholder.cc
[modify] https://crrev.com/d89b22434f718dd0f82833b5a38ebc92444a99ea/components/plugins/renderer/plugin_placeholder.h

Project Member

Comment 30 by bugdroid1@chromium.org, Nov 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1c886c53eae3a8378502c4e185e29bfa709d4b97

commit 1c886c53eae3a8378502c4e185e29bfa709d4b97
Author: Tommy C. Li <tommycli@chromium.org>
Date: Thu Nov 15 17:13:03 2018

Click to Open PDF: Add EMBED browsertest

This re-adds the previously approved and committed browsertest, but this
time with a fix for the flakiness.

The body of the test remains the same, but with an extra call to:
PluginTestUtils::WaitForPlaceholderReady(GetActiveWebContents(),
                                         "pdf_embed");

There is also an update the command line flags and an extra include.

This is patch 3/3 for this objective.

Bug: 887752,  879149 , 878871
Change-Id: Ie8c70d68fb3e3ebf9e70e5d9f2f1aa2f37b190f4
Reviewed-on: https://chromium-review.googlesource.com/c/1336086
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608411}
[modify] https://crrev.com/1c886c53eae3a8378502c4e185e29bfa709d4b97/chrome/browser/pdf/pdf_extension_test.cc
[modify] https://crrev.com/1c886c53eae3a8378502c4e185e29bfa709d4b97/chrome/test/data/pdf/pdf_embed_csp.html

Sign in to add a comment