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-

mitz
Reported 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.
Attachments
Patch v1 - Kill the plugin, recreate it, and layout test. (29.47 KB, patch)
2011-12-09 15:09 PST, Brady Eidson 		beidson: commit-queue-
DetailsFormatted DiffDiff
Patch v2 - Fixed naming in new Node.h method (29.40 KB, patch)
2011-12-12 10:34 PST, Brady Eidson 		andersca: review+
beidson: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Dave Hyatt
Comment 1 2007-05-08 23:41:31 PDT
The b/f cache already works with plug-ins.
Brady Eidson
Comment 2 2007-05-08 23:45:12 PDT
<rdar://problem/5190122>
Brady Eidson
Comment 3 2007-05-09 00:05:55 PDT
This worked in Safari 2.0
mitz
Comment 4 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.
Dave Hyatt
Comment 5 2007-05-09 00:28:02 PDT
Wow, what the heck happened here? I'm amazed the original bug got an r+.
mitz
Comment 6 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.
Cameron Zwarich (cpst)
Comment 7 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.
Brady Eidson
Comment 8 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.
Brady Eidson
Comment 9 2011-12-09 15:09:07 PST
Created attachment 118652 [details] Patch v1 - Kill the plugin, recreate it, and layout test.
WebKit Review Bot
Comment 10 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
Brady Eidson
Comment 11 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.
mitz
Comment 12 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().
Brady Eidson
Comment 13 2011-12-12 10:34:23 PST
Created attachment 118814 [details] Patch v2 - Fixed naming in new Node.h method
Brady Eidson
Comment 14 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.
WebKit Review Bot
Comment 15 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
Brady Eidson
Comment 16 2011-12-12 14:01:36 PST
Landed in r102619
Kenneth Russell
Comment 17 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 .
Brady Eidson
Comment 18 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.
Note You need to log in before you can comment on or make changes to this bug.