[webkit-reviews] review requested: [Bug 32848] Avoid reloading iframe on re-parenting between documents. : [Attachment 45416] Proposed patch, v2.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Dec 22 20:52:03 PST 2009


Dmitry Titov <dimich at chromium.org> has asked  for review:
Bug 32848: Avoid reloading iframe on re-parenting between documents.
https://bugs.webkit.org/show_bug.cgi?id=32848

Attachment 45416: Proposed patch, v2.
https://bugs.webkit.org/attachment.cgi?id=45416&action=review

------- Additional Comments from Dmitry Titov <dimich at chromium.org>
1. Added check for document being 'attached()', and added a test where document
loaded by XMLHttpRequest is used to adopt the iframe, verifying that iframe
still unloads in this scenario.
2. Moved keepAliveOnRemovalFromTree() to 'protected' from 'public'.
3. Added a one-shot timer that makes sure that if script did adoptNode() but
did not added the iframe into the tree, the iframe gets unloaded - to avoid
active content running in non-attached iframe. This can cause more issues and
memory leaks.

I think enabling live iframe reparenting opens up some space for issues - for
example, I think the frame tree is not maintained right and other issues
perhaps are still to be discovered once the active reparenting is actually
used. It might be easier and faster to move step by step so if this patch is
reasonable then I'd add more tests and fixes in separate patches, to keep
things smaller. Obviously, one thing to look for would be compatibility issues.


More information about the webkit-reviews mailing list