Summary: | Page Cache should support pages with plug-ins (again) | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | mitz | ||||||
Component: | Page Loading | Assignee: | 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
mitz
2007-05-08 22:59:51 PDT
The b/f cache already works with plug-ins. This worked in Safari 2.0 Regressed in <http://trac.webkit.org/projects/webkit/changeset/12250>. Bug reference in the change log seems wrong. Wow, what the heck happened here? I'm amazed the original bug got an r+. 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. 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. 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. Created attachment 118652 [details]
Patch v1 - Kill the plugin, recreate it, and layout test.
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 (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 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(). Created attachment 118814 [details]
Patch v2 - Fixed naming in new Node.h method
(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 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 Landed in r102619 Note that a Windows build fix for this patch was committed in http://trac.webkit.org/changeset/102628 . (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. |