Bug 13634 – Page Cache should support pages with plug-ins (again)

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[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.

[Dave Hyatt]

The b/f cache already works with plug-ins.

[Brady Eidson]

<rdar://problem/5190122>

[Brady Eidson]

This worked in Safari 2.0

Regressed in <http://trac.webkit.org/projects/webkit/changeset/12250>. Bug reference in the change log seems wrong.

[Dave Hyatt]

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.

[Cameron Zwarich (cpst)]

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]

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]

Created attachment 118652 [details]
Patch v1 - Kill the plugin, recreate it, and layout test.

[WebKit Review Bot]

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]

(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().

[Brady Eidson]

Created attachment 118814 [details]
Patch v2 - Fixed naming in new Node.h method

[Brady Eidson]

(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 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]

Landed in r102619

[Kenneth Russell]

Note that a Windows build fix for this patch was committed in http://trac.webkit.org/changeset/102628 .

[Brady Eidson]

(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.