Bug 79001 - undo manager retains RefPtr to nodes in documents that would otherwise have been GC'ed
Summary: undo manager retains RefPtr to nodes in documents that would otherwise have b...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks: 74059
  Show dependency treegraph
 
Reported: 2012-02-19 14:04 PST by Ryosuke Niwa
Modified: 2017-07-18 08:30 PDT (History)
7 users (show)

See Also:


Attachments
proof of concept wip patch (15.98 KB, patch)
2012-02-19 22:18 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2012-02-19 14:04:07 PST
Because WebKit's undo manager manages undo stack at the page level, all documents that have ever been edited on the same page will be kept in memory even if there were no other references to it.

This is pretty silly because we can't even undo editing actions on an unloaded frame/document even if the user had loaded it back.
Comment 1 Ryosuke Niwa 2012-02-19 14:06:43 PST
Since none of our editing actions do cross-frame editing, we should be able to delete all UndoStep objects that belong to an unloaded frame.
Comment 2 Alexey Proskuryakov 2012-02-19 20:41:05 PST
When are the undo steps currently cleared?
Comment 3 Ryosuke Niwa 2012-02-19 21:32:19 PST
(In reply to comment #2)
> When are the undo steps currently cleared?

It's cleared when the page is unloaded in FrameLoader::closeURL.
Comment 4 Ryosuke Niwa 2012-02-19 22:18:00 PST
Created attachment 127761 [details]
proof of concept wip patch
Comment 5 Ryosuke Niwa 2012-02-19 22:20:24 PST
Comment on attachment 127761 [details]
proof of concept wip patch

Here's my attempt to mitigate this issue. I'm removing all undo step objects when the frame is destroyed. For Mac port, this can be done by creating targets for each "host" document and then removing all targets when the corresponding frame is destroyed.

I probably need to hash out the details but it compiles & loads pages as is.