Bug 13634 - Page Cache should support pages with plug-ins (again)
Summary: Page Cache should support pages with plug-ins (again)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar, Regression
Depends on: 13636
Blocks: 4123 74357 74581 76891
  Show dependency treegraph
 
Reported: 2007-05-08 22:59 PDT by mitz
Modified: 2012-01-23 18:50 PST (History)
14 users (show)

See Also:


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-
Details | Formatted Diff | Diff
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-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.