Bug 13634

Summary: Page Cache should support pages with plug-ins (again)
Product: WebKit Reporter: mitz
Component: Page LoadingAssignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: adrian.sutton, ap, beidson, christian.webkit, dglazkov, emacemac7, joepeck, kbr, matafagafo, skyul, tonikitoo, vestbo, webkit.review.bot, zwarich
Priority: P1 Keywords: InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 13636    
Bug Blocks: 4123, 74357, 74581, 76891    
Attachments:
Description Flags
Patch v1 - Kill the plugin, recreate it, and layout test.
beidson: commit-queue-
Patch v2 - Fixed naming in new Node.h method andersca: review+, beidson: commit-queue-

Description mitz 2007-05-08 22:59:51 PDT
[Filed for completeness' sake, as other major reasons why pages are not cached have separate bugs now].

The back/forward cache needs to work with pages that have plug-ins.
Comment 1 Dave Hyatt 2007-05-08 23:41:31 PDT
The b/f cache already works with plug-ins.
Comment 2 Brady Eidson 2007-05-08 23:45:12 PDT
<rdar://problem/5190122>
Comment 3 Brady Eidson 2007-05-09 00:05:55 PDT
This worked in Safari 2.0
Comment 4 mitz 2007-05-09 00:09:30 PDT
Regressed in <http://trac.webkit.org/projects/webkit/changeset/12250>. Bug reference in the change log seems wrong.
Comment 5 Dave Hyatt 2007-05-09 00:28:02 PDT
Wow, what the heck happened here? I'm amazed the original bug got an r+.
Comment 6 mitz 2007-05-09 00:53:59 PDT
The change in r12250 affected a small fraction of plug-in-containing pages, namely those using <embed> but not using <object>. Bug 13636 prevents pages using <object> from being cached even in Safari 2.
Comment 7 Cameron Zwarich (cpst) 2008-06-09 21:37:32 PDT
Firefox 3 supports this as far as I can tell, so we should see how they do it. However, it might be better to fix bug 13631 first.
Comment 8 Brady Eidson 2009-09-15 16:51:21 PDT
Plug-ins have changed a lot since we originally cached them in Safari 2 and earlier.  Removing them from the view hierarchy is no longer sufficient enough to stop them.

Also, now that we support pages with frames in the page cache, this task is now different from restoring our original behavior.

We'll probably need to start out by exploring a solution like what Firefox does, which is to manually destroy the plug-ins when leaving the page, then re-instantiate them when returning.

I plan to start working on this shortly.
Comment 9 Brady Eidson 2011-12-09 15:09:07 PST
Created attachment 118652 [details]
Patch v1 - Kill the plugin, recreate it, and layout test.
Comment 10 WebKit Review Bot 2011-12-09 20:02:03 PST
Comment on attachment 118652 [details]
Patch v1 - Kill the plugin, recreate it, and layout test.

Attachment 118652 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10831497

New failing tests:
plugins/netscape-plugin-page-cache-works.html
Comment 11 Brady Eidson 2011-12-12 10:03:45 PST
(In reply to comment #10)
> (From update of attachment 118652 [details])
> Attachment 118652 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/10831497
> 
> New failing tests:
> plugins/netscape-plugin-page-cache-works.html

Will add to their skipped list before landing.
Comment 12 mitz 2011-12-12 10:11:47 PST
Comment on attachment 118652 [details]
Patch v1 - Kill the plugin, recreate it, and layout test.

View in context: https://bugs.webkit.org/attachment.cgi?id=118652&action=review

> Source/WebCore/dom/Node.h:695
> +    void unsetHasCustomStyleForRenderer() { setFlag(false, HasCustomStyleForRendererFlag); }

Other similar functions use the verb “clear” rather than “unset”, and call clearFlag().
Comment 13 Brady Eidson 2011-12-12 10:34:23 PST
Created attachment 118814 [details]
Patch v2 - Fixed naming in new Node.h method
Comment 14 Brady Eidson 2011-12-12 10:34:48 PST
(In reply to comment #13)
> Created an attachment (id=118814) [details]
> Patch v2 - Fixed naming in new Node.h method

Also, forgot there's no Chromium skipped list here, so I can't skip the new test for them.
Comment 15 WebKit Review Bot 2011-12-12 11:33:44 PST
Comment on attachment 118814 [details]
Patch v2 - Fixed naming in new Node.h method

Attachment 118814 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10851024

New failing tests:
plugins/netscape-plugin-page-cache-works.html
Comment 16 Brady Eidson 2011-12-12 14:01:36 PST
Landed in r102619
Comment 17 Kenneth Russell 2011-12-12 15:07:44 PST
Note that a Windows build fix for this patch was committed in http://trac.webkit.org/changeset/102628 .
Comment 18 Brady Eidson 2011-12-12 15:23:21 PST
(In reply to comment #17)
> Note that a Windows build fix for this patch was committed in http://trac.webkit.org/changeset/102628 .

Sorry for the break, and thanks for noting the fix here.