Combination of PDF Download Setting and iFrame causes problems.
Reported by
aaron...@gmail.com,
Sep 20
|
||||||||
Issue descriptionChrome Version : 69.0.3497.100 OS Version: 10.0 URLs (if applicable) : https://pdfobject.com/static.html Other browsers tested: Firefox, Edge, IE Add OK or FAIL after other browsers where you have tested this issue: Safari: N/A Firefox: Works IE/Edge: N/A (don't have this setting) What steps will reproduce the problem? 1. Turn on "Download PDF files instead of automatically opening them in Chrome" setting 2. go to above link 3. What is the expected result? example PDFs should download What happens instead of that? Nothing, and the open button does nothing. Please provide any additional information below. Attach a screenshot if possible. Happy to install old versions to test when this regressed from, but don't know the easiest way of doing that. UserAgentString: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36
,
Sep 21
,
Sep 21
@mattm On the PDF object site, yes, I see that multiple file download warning and even when I say ok, go ahead, it doesn't actually download the files as far as I can tell. I have Adobe Reader set up as the default PDF handler, yes. This bug also applies to an internal app where I work, which I can't show details of, but also renders a PDF inside an iframe, and has the same behavior in terms of the open button, but since there's only 1 PDF on a given page, it does not have that warning, just a broken open button.
,
Sep 21
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
,
Sep 21
Indeed there is something wrong with this. Related to 878871.
,
Sep 25
Any chance we could bump the priority up on this Bug? We're using an enterprise business intelligence product at our business that has this same issue after updating to Chrome 69. It's causing major issues since we have a admin policy that forces PDFs to download instead of view in browser. Our work around will be to allow in browser viewing of pdfs in iFrames, etc.
,
Sep 25
Indeed - it's a P2 at least for us. I am actively working to bring this to a resolution. Sorry for the delay.
,
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
,
Oct 5
,
Oct 5
This bug requires manual review: We are only 10 days from stable. Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 5
Let's verify this in canary first.
,
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
,
Oct 8
seems like this is reverted? Is there any M70 merge required?
,
Oct 8
abdulsyed: I'm going to withdraw the merge request for now until I can stick the landing. Thanks for the consideration.
,
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
,
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
,
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
,
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
,
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
,
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
,
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 |
||||||||
Comment 1 by mattm@chromium.org
, Sep 21Labels: Needs-Feedback