New issue
Advanced search Search tips

Issue 892818 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug
Flaky-Test: PDFPluginDisabledTest.EmbedPdfPlaceholderWithCSP



Sign in to add a comment

PDFPluginDisabledTest.EmbedPdfPlaceholderWithCSP is flaky

Project Member Reported by Findit, Oct 5

Issue description

Labels: -Sheriff-Chromium
Owner: tommycli@chromium.org
Project Member

Comment 2 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

Sign in to add a comment