Bug 75449 - REGRESSION: Inline PDF that are cached fail to appear in iframe
Summary: REGRESSION: Inline PDF that are cached fail to appear in iframe
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Frames (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Unspecified
: P1 Major
Assignee: zalan
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2012-01-02 14:53 PST by Chris Petersen
Modified: 2013-09-19 13:11 PDT (History)
7 users (show)

See Also:


Attachments
Simple iframe test that uses a inline PDF (15.11 KB, application/zip)
2012-01-02 14:56 PST, Chris Petersen
no flags Details
Patch (6.03 KB, patch)
2013-09-11 10:13 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (6.07 KB, patch)
2013-09-12 07:46 PDT, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Petersen 2012-01-02 14:53:53 PST
Summary: Inline PDF that are cached fail to appear in iframe

1) Launch Webkit revision 103908 under Mac OS X 10.7.2 and go to http://www.cs.tut.fi/~jkorpela/html/iframe-pdf.html
2) Notice inline PDFs load in the iframes
3) Press Back button to navigate away from page
4) Press forward to navigate back to page. Notice the pdf file doesn't render in iframe

I have also attached a simple iframe test.
Comment 1 Chris Petersen 2012-01-02 14:56:09 PST
Created attachment 120895 [details]
Simple iframe test that uses a inline PDF

Simple iframe test that uses a inline PDF
Comment 2 Alexey Proskuryakov 2012-01-03 11:00:40 PST
This sounds like a regression from enabling page cache for pages with plug-ins. An assertion fails in debug builds:

ASSERTION FAILED: m_frameCount + 1 == frameCount
/Users/ap/Safari/OpenSource/Source/WebCore/page/Page.cpp(1058) : void WebCore::Page::checkFrameCountConsistency() const
1   0x104e0a79a WebCore::Page::checkFrameCountConsistency() const
2   0x103f59f39 WebCore::Page::frameCount() const
3   0x1045fcfa3 WebCore::HTMLPlugInImageElement::allowedToLoadFrameURL(WTF::String const&)
4   0x104596bb9 WebCore::HTMLEmbedElement::updateWidget(WebCore::PluginCreationOption)
Comment 3 Alexey Proskuryakov 2012-01-03 11:00:59 PST
<rdar://problem/10637392>
Comment 4 Alexey Proskuryakov 2013-09-03 14:59:36 PDT
This is still an issue.
Comment 5 zalan 2013-09-08 12:11:45 PDT
It looks like we miss some initialisation when we load the content with loadDifferentDocumentItem() instead of loadURL(). When the loader is changed so that it ignores the existing history item for the <iframe>, the pdf renders fine.
While looking into this, I found another (somewhat related) history bug -> bug 121009
Comment 6 zalan 2013-09-09 08:55:13 PDT
The actual reason for the blank pdf content is that when we recreate the PDFPlugin (while restoring the cached frame), we forget to re-initialize the object properly.
What happens here is:
1. Initial load -> Frame loading ends up calling PDFPlugin::pdfDocumentDidLoad() which creates and initializes the pdf document using the rawData() (m_data) 
2. While navigating away, we destroy the plugin renderers -> PDFPlugin object gets deleted.
3. Navigating back -> new PDFPlugin object is created but we never pass the raw data back -> no pdf document -> blank content.
(When the raw data is kept around and assigned it back to the newly created PDFPlugin object, the PDF content renders fine)
Comment 7 zalan 2013-09-10 07:05:41 PDT
WebFrameLoaderClient::m_frameCameFromPageCache does not get set to true for subframes. Patch is coming up.
Comment 8 zalan 2013-09-11 10:13:57 PDT
Created attachment 211323 [details]
Patch
Comment 9 zalan 2013-09-12 07:46:30 PDT
Created attachment 211428 [details]
Patch
Comment 10 zalan 2013-09-12 07:47:07 PDT
Comment on attachment 211428 [details]
Patch

ews testing
Comment 11 WebKit Commit Bot 2013-09-12 08:45:56 PDT
Comment on attachment 211428 [details]
Patch

Clearing flags on attachment: 211428

Committed r155615: <http://trac.webkit.org/changeset/155615>
Comment 12 WebKit Commit Bot 2013-09-12 08:45:59 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Alexey Proskuryakov 2013-09-12 12:48:57 PDT
This caused a lot of test flakiness, filed bug 121245. Not a fault with this patch.

However, I'm also seeing the test fail sometimes locally on WK1 10.8. The actual result is white subframe instead of green.

For whatever reason, bots only see fast/history/history-back-while-pdf-in-pagecache.html fail on Lion.

Zalan, could you please look into this?
Comment 14 zalan 2013-09-12 12:50:27 PDT
(In reply to comment #13)
> This caused a lot of test flakiness, filed bug 121245. Not a fault with this patch.
> 
> However, I'm also seeing the test fail sometimes locally on WK1 10.8. The actual result is white subframe instead of green.
> 
> For whatever reason, bots only see fast/history/history-back-while-pdf-in-pagecache.html fail on Lion.
> 
> Zalan, could you please look into this?

It could be a timing issue (on history.back()). I'll look into it.
Comment 15 zalan 2013-09-13 05:52:45 PDT
I can make it fail on Mountain Lion (locally), if I set the notifyDone timeout short enough. I suspect that this timeout value makes the testcase flaky. Unfortunately I can't confirm it on Lion. Any suggestion? (or does the bug 121245 take care of the Lion failure?)
Comment 16 Alexey Proskuryakov 2013-09-13 08:25:48 PDT
(In reply to comment #15)
> I can make it fail on Mountain Lion (locally), if I set the notifyDone timeout short enough. I suspect that this timeout value makes the testcase flaky. Unfortunately I can't confirm it on Lion. Any suggestion? 

Given that test bots have the opposite issue - they only see this on Lion - I'd suggest just taking care of what you can reproduce, and seeing if that helps.

> (or does the bug 121245 take care of the Lion failure?)

It won't, that fix only prevents PDF tests from affecting drawing in subsequent tests.
Comment 17 Alexey Proskuryakov 2013-09-17 16:33:51 PDT
Any update on this? Turns out that Windows was also affected, until marked as failing in r156007.
Comment 18 zalan 2013-09-18 10:17:10 PDT
(In reply to comment #17)
> Any update on this? Turns out that Windows was also affected, until marked as failing in r156007.

Trying to modify the testcase to make it non-flaky instead of adding new functionality to the test framework. (->internal event to signal the iframe pdf load on history back)
Comment 19 zalan 2013-09-19 13:11:32 PDT
flaky testcase is tracked here: bug 121628