256404 – Document leak on pages with text input forms such as google.com

Bug 256404 - Document leak on pages with text input forms such as google.com

Summary: Document leak on pages with text input forms such as google.com

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	Forms (show other bugs)
Version:	WebKit Nightly Build
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Chris Dumez

URL:
Keywords:	InRadar

Depends on:
Blocks:

Reported:	2023-05-05 18:14 PDT by Ryan Reno
Modified:	2023-06-19 18:36 PDT (History)
CC List:	7 users (show)

See Also:	258289

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Ryan Reno 2023-05-05 18:14:18 PDT

There's a document leak of google.com after navigating to a search results page.

Steps:
1. Open MiniBrowser from terminal or Xcode to about:blank
> run-minibrowser --url about:blank

2. Navigate to google.com
3. search for something. I searched for "New York City"
4. Navigate to about:blank
5. Trigger a low memory warning
> notifyutil -p org.WebKit.lowMemory

6. Look at active documents
> notifyutil -p com.apple.WebKit.showAllDocuments

Here's what I got:
1 live pages:
Page 0x107048100 with main document 0x141121000 about:blank
3 live documents:
Document 0x141120000 8-7cf3e1d8-e81c-4639-b00a-bf0017064947 (refCount 21, referencingNodeCount 137) https://www.google.com/
Document 0x141122000 8-039ca048-8080-4fe8-9593-9f869b17ab9c (refCount 1, referencingNodeCount 1) about:blank
Document 0x141121000 8-154d77ec-a511-4345-9944-6f5f72ac7684 (refCount 2, referencingNodeCount 3) about:blank

After navigating cross-site and triggering a process swap you'll see this:
LEAK: 1 Page
LEAK: 1 Frame
LEAK: 1 WebCoreNode

Comment 1 Radar WebKit Bug Importer 2023-05-05 18:14:31 PDT

<rdar://problem/108975202>

Comment 2 Ryan Reno 2023-05-05 18:22:19 PDT

The google.com document doesn't leak if you simply go from about:blank -> google -> about:blank and do the low memory warning. Only if you search for something from google.com first.

Comment 3 Ryan Reno 2023-05-07 17:51:12 PDT

This isn't google-specific. Any page with an input type=text element on it that has text typed in will leak.

>index.html
<!DOCTYPE html>
<form action="/simple.html" autocomplete="off" method="GET">
    <input type="text" name="text"/>
    <input type="submit"/>
</form>

>simple.html
<!DOCTYPE html>
This is a simple page


If you type any text into the form on index.html then click submit and then issue a low memory warning and showAllDocuments you will see the index.html document leaked.

According to this backtrace we're keeping an EditCommand around which holds a strong reference to the Document. More investigation is needed to figure out what's holding the EditCommand.

RefTracker: Backtrace for token 29180 (http://localhost:14014/)
1   0x136b84738 WTF::RefTracker::trackRef(WTF::String const&)
2   0x28388d544 WebCore::Document::trackRef()
3   0x2830cb32c void WTF::RefTrackingTraits::ref<WebCore::Document>(WebCore::Document&)
4   0x2830cb2c8 WTF::Ref<WebCore::Document, WTF::RawPtrTraits<WebCore::Document>, WTF::RefDerefTraits>::Ref(WebCore::Document&)
5   0x280e0d988 WTF::Ref<WebCore::Document, WTF::RawPtrTraits<WebCore::Document>, WTF::RefDerefTraits>::Ref(WebCore::Document&)
6   0x283bfd374 WebCore::EditCommand::EditCommand(WebCore::Document&, WebCore::EditAction)
7   0x283bf3c0c WebCore::SimpleEditCommand::SimpleEditCommand(WebCore::Document&, WebCore::EditAction)
8   0x283c66b2c WebCore::InsertIntoTextNodeCommand::InsertIntoTextNodeCommand(WTF::Ref<WebCore::Text, WTF::RawPtrTraits<WebCore::Text>, WTF::RefDerefTraits>&&, unsigned int, WTF::String const&, WebCore::EditAction)
9   0x283c66ca4 WebCore::InsertIntoTextNodeCommand::InsertIntoTextNodeCommand(WTF::Ref<WebCore::Text, WTF::RawPtrTraits<WebCore::Text>, WTF::RefDerefTraits>&&, unsigned int, WTF::String const&, WebCore::EditAction)
10  0x283bd9bfc WebCore::InsertIntoTextNodeCommand::create(WTF::Ref<WebCore::Text, WTF::RawPtrTraits<WebCore::Text>, WTF::RefDerefTraits>&&, unsigned int, WTF::String const&, WebCore::EditAction)
11  0x283bd37c8 WebCore::CompositeEditCommand::insertTextIntoNode(WebCore::Text&, unsigned int, WTF::String const&)
12  0x283c6e1bc WebCore::InsertTextCommand::doApply()
13  0x283bd800c WebCore::CompositeEditCommand::applyCommandToComposite(WTF::Ref<WebCore::CompositeEditCommand, WTF::RawPtrTraits<WebCore::CompositeEditCommand>, WTF::RefDerefTraits>&&, WebCore::VisibleSelection const&)
14  0x283caf25c WebCore::TypingCommand::insertTextRunWithoutNewlines(WTF::String const&, bool)
15  0x283cd140c WebCore::TypingCommandLineOperation::operator()(unsigned long, unsigned long, bool) const
16  0x283caf114 void WebCore::forEachLineInString<WebCore::TypingCommandLineOperation>(WTF::String const&, WebCore::TypingCommandLineOperation const&)
17  0x283caeff0 WebCore::TypingCommand::insertText(WTF::String const&, bool)
18  0x283cadb8c WebCore::TypingCommand::insertTextAndNotifyAccessibility(WTF::String const&, bool)
19  0x283cad9d8 WebCore::TypingCommand::insertText(WebCore::Document&, WTF::String const&, WebCore::VisibleSelection const&, unsigned int, WebCore::TypingCommand::TextCompositionType)
20  0x283c1a078 WebCore::Editor::insertTextWithoutSendingTextEvent(WTF::String const&, bool, WebCore::TextEvent*)
21  0x283c18fa8 WebCore::Editor::handleTextEvent(WebCore::TextEvent&)
22  0x2847bb050 WebCore::EventHandler::defaultTextInputEventHandler(WebCore::TextEvent&)
23  0x283a9d920 WebCore::Node::defaultEventHandler(WebCore::Event&)
24  0x283e38f40 WebCore::HTMLInputElement::defaultEventHandler(WebCore::Event&)
25  0x2839fbf70 WebCore::callDefaultEventHandlersInBubblingOrder(WebCore::Event&, WebCore::EventPath const&)
26  0x2839fb6cc WebCore::EventDispatcher::dispatchEvent(WebCore::Node&, WebCore::Event&)
27  0x283a9d1b8 WebCore::Node::dispatchEvent(WebCore::Event&)
28  0x2847bad88 WebCore::EventHandler::handleTextInputEvent(WTF::String const&, WebCore::Event*, WebCore::TextEventInputType)
29  0x283c21b04 WebCore::Editor::insertText(WTF::String const&, WebCore::Event*, WebCore::TextEventInputType)
30  0x11a267e2c WebKit::WebPage::executeKeypressCommandsInternal(WTF::Vector<WebCore::KeypressCommand, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::KeyboardEvent*)
31  0x11a268fb0 WebKit::WebPage::handleEditingKeyboardEvent(WebCore::KeyboardEvent&)
32  0x11a17c67c WebKit::WebEditorClient::handleKeyboardEvent(WebCore::KeyboardEvent&)
33  0x283c18bd8 WebCore::Editor::handleKeyboardEvent(WebCore::KeyboardEvent&)
34  0x2847b96d4 WebCore::EventHandler::defaultKeyboardEventHandler(WebCore::KeyboardEvent&)
35  0x283a9d7f4 WebCore::Node::defaultEventHandler(WebCore::Event&)
36  0x283e38b04 WebCore::HTMLInputElement::defaultEventHandler(WebCore::Event&)
37  0x2839fbf70 WebCore::callDefaultEventHandlersInBubblingOrder(WebCore::Event&, WebCore::EventPath const&)
38  0x2839fb6cc WebCore::EventDispatcher::dispatchEvent(WebCore::Node&, WebCore::Event&)
39  0x283a9d1b8 WebCore::Node::dispatchEvent(WebCore::Event&)
40  0x2847b8620 WebCore::EventHandler::internalKeyEvent(WebCore::PlatformKeyboardEvent const&)
41  0x2847b7968 WebCore::EventHandler::keyEvent(WebCore::PlatformKeyboardEvent const&)
42  0x285731900 WebCore::UserInputBridge::handleKeyEvent(WebCore::PlatformKeyboardEvent const&, WebCore::InputSource)
43  0x11b0dfe1c WebKit::handleKeyEvent(WebKit::WebKeyboardEvent const&, WebCore::Page*)
44  0x11b0dfc14 WebKit::WebPage::keyEvent(WebKit::WebKeyboardEvent const&)
45  0x11b19ba14 auto void IPC::callMemberFunction<WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&)::'lambda'(auto&&...)::operator()<WebKit::WebKeyboardEvent>(auto&&...) const
46  0x11b19b92c decltype(std::declval<WebKit::WebPage>()(std::declval<WebKit::WebKeyboardEvent>())) std::__1::__invoke[abi:v160002]<void IPC::callMemberFunction<WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&)::'lambda'(auto&&...), WebKit::WebKeyboardEvent>(WebKit::WebPage&&, WebKit::WebKeyboardEvent&&)
47  0x11b19b8fc decltype(auto) std::__1::__apply_tuple_impl[abi:v160002]<void IPC::callMemberFunction<WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&)::'lambda'(auto&&...), std::__1::tuple<WebKit::WebKeyboardEvent>, 0ul>(WebKit::WebPage&&, WebKit::WebPage&&, std::__1::__tuple_indices<0ul>)
48  0x11b19b8bc decltype(auto) std::__1::apply[abi:v160002]<void IPC::callMemberFunction<WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&)::'lambda'(auto&&...), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage&&, WebKit::WebPage&&)
49  0x11b19b0fc void IPC::callMemberFunction<WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&)
50  0x11b15eb04 void IPC::handleMessage<Messages::WebPage::KeyEvent, WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&)>(IPC::Connection&, IPC::Decoder&, WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&))
51  0x11b1572c8 WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::Decoder&)
52  0x11b0ea820 WebKit::WebPage::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
53  0x11b6c2a04 IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&)
54  0x11a7ed8d8 WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
55  0x11b6967ac IPC::Connection::dispatchMessage(IPC::Decoder&)
56  0x11b696c40 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder>>)
57  0x11b696f7c IPC::Connection::dispatchOneIncomingMessage()
58  0x11b6b4e1c IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder>>)::$_17::operator()() const
59  0x11b6b4d5c WTF::Detail::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder>>)::$_17, void>::call()
60  0x138968d2c WTF::Function<void ()>::operator()() const
61  0x136b8bfe0 WTF::RunLoop::performWork()
62  0x136b90560 WTF::RunLoop::performWork(void*)
63  0x18215bb54 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
64  0x18215bae8 __CFRunLoopDoSource0
65  0x18215b858 __CFRunLoopDoSources0
66  0x18215a460 __CFRunLoopRun
67  0x182159a70 CFRunLoopRunSpecific
68  0x1831c7168 -[NSRunLoop(NSRunLoop) runMode:beforeDate:]
69  0x18323ee58 -[NSRunLoop(NSRunLoop) run]
70  0x181dadef0 _xpc_objc_main
71  0x181dbcb94 _xpc_main
72  0x181dada9c _xpc_copy_xpcservice_dictionary
73  0x1194c57cc WebKit::XPCServiceMain(int, char const**)
74  0x11b664e08 WKXPCServiceMain
75  0x102f53f9c main
76  0x181d02058 start

Comment 4 Wenson Hsieh 2023-05-07 17:53:30 PDT

Perhaps this is related to the undo stack? (i.e. `EditCommandComposition` and related classes).

Comment 5 Ryan Reno 2023-05-07 17:56:54 PDT

Maybe! Editing code is completely new to me so I've got some digging to do. Thanks for the pointer.

Comment 6 Ryan Reno 2023-05-09 19:36:31 PDT

Yes, seems related to undo stack.

WebPage holds a RefPtr to a WebUndoStep in a map. WebUndoStep holds a strong Ref to an UndoStep. In this case an EditCommandComposition derived class. The EditCommandComposition holds a RefPtr to the Document.

The EditCommandComposition also has a Vector of RefPtrs to EditCommands. These edit commands also hold strong Refs to the Document.

It looks like the UndoStep is never cleared out of the WebPage's map and so there are TypingCommand objects refing the Document which are stuck in a command list. I think if the (Web)UndoStep were destroyed the document wouldn't have refs remaining.

Here's the creation stack trace of that WebUndoStep:

RefTracker: Backtrace for token 13711 (EditCommandComposition)
1   0x1344beef8 WTF::RefTracker::trackRef(WTF::String const&)
2   0x14e670a08 WebCore::EditCommandComposition::trackRef() const
3   0x1184a64d0 void WTF::RefTrackingTraits::ref<WebCore::UndoStep>(WebCore::UndoStep&)
4   0x1184a6444 WTF::Ref<WebCore::UndoStep, WTF::RawPtrTraits<WebCore::UndoStep>, WTF::RefDerefTraits>::Ref(WebCore::UndoStep&)
5   0x11849524c WTF::Ref<WebCore::UndoStep, WTF::RawPtrTraits<WebCore::UndoStep>, WTF::RefDerefTraits>::Ref(WebCore::UndoStep&)
6   0x118495128 WebKit::WebEditorClient::registerUndoStep(WebCore::UndoStep&)
7   0x14e6b28f4 WebCore::Editor::appliedEditing(WebCore::CompositeEditCommand&)
8   0x14e7421cc WebCore::TypingCommand::typingAddedToOpenCommand(WebCore::TypingCommand::ETypingCommand)
9   0x14e7424cc WebCore::TypingCommand::insertTextRunWithoutNewlines(WTF::String const&, bool)
10  0x14e764564 WebCore::TypingCommandLineOperation::operator()(unsigned long, unsigned long, bool) const
11  0x14e742370 void WebCore::forEachLineInString<WebCore::TypingCommandLineOperation>(WTF::String const&, WebCore::TypingCommandLineOperation const&)
12  0x14e74224c WebCore::TypingCommand::insertText(WTF::String const&, bool)
13  0x14e740de8 WebCore::TypingCommand::insertTextAndNotifyAccessibility(WTF::String const&, bool)
14  0x14e741998 WebCore::TypingCommand::doApply()
15  0x14e654b3c WebCore::CompositeEditCommand::apply()
16  0x14e72ec50 WebCore::TextInsertionBaseCommand::applyTextInsertionCommand(WebCore::LocalFrame*, WebCore::TextInsertionBaseCommand&, WebCore::VisibleSelection const&, WebCore::VisibleSelection const&)
17  0x14e740cb4 WebCore::TypingCommand::insertText(WebCore::Document&, WTF::String const&, WebCore::VisibleSelection const&, unsigned int, WebCore::TypingCommand::TextCompositionType)
18  0x14e6acba4 WebCore::Editor::insertTextWithoutSendingTextEvent(WTF::String const&, bool, WebCore::TextEvent*)
19  0x14e6ababc WebCore::Editor::handleTextEvent(WebCore::TextEvent&)
20  0x14f252d48 WebCore::EventHandler::defaultTextInputEventHandler(WebCore::TextEvent&)
21  0x14e52e2e8 WebCore::Node::defaultEventHandler(WebCore::Event&)
22  0x14e8cc004 WebCore::HTMLInputElement::defaultEventHandler(WebCore::Event&)
23  0x14e48ccdc WebCore::callDefaultEventHandlersInBubblingOrder(WebCore::Event&, WebCore::EventPath const&)
24  0x14e48c438 WebCore::EventDispatcher::dispatchEvent(WebCore::Node&, WebCore::Event&)
25  0x14e52db80 WebCore::Node::dispatchEvent(WebCore::Event&)
26  0x14f252a80 WebCore::EventHandler::handleTextInputEvent(WTF::String const&, WebCore::Event*, WebCore::TextEventInputType)
27  0x14e6b4658 WebCore::Editor::insertText(WTF::String const&, WebCore::Event*, WebCore::TextEventInputType)
28  0x1179b99a0 WebKit::WebPage::executeKeypressCommandsInternal(WTF::Vector<WebCore::KeypressCommand, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::KeyboardEvent*)
29  0x1179bab24 WebKit::WebPage::handleEditingKeyboardEvent(WebCore::KeyboardEvent&)
30  0x1178cd2e0 WebKit::WebEditorClient::handleKeyboardEvent(WebCore::KeyboardEvent&)
31  0x14e6ab6e8 WebCore::Editor::handleKeyboardEvent(WebCore::KeyboardEvent&)
32  0x14f2513cc WebCore::EventHandler::defaultKeyboardEventHandler(WebCore::KeyboardEvent&)
33  0x14e52e1bc WebCore::Node::defaultEventHandler(WebCore::Event&)
34  0x14e8cbbc8 WebCore::HTMLInputElement::defaultEventHandler(WebCore::Event&)
35  0x14e48ccdc WebCore::callDefaultEventHandlersInBubblingOrder(WebCore::Event&, WebCore::EventPath const&)
36  0x14e48c438 WebCore::EventDispatcher::dispatchEvent(WebCore::Node&, WebCore::Event&)
37  0x14e52db80 WebCore::Node::dispatchEvent(WebCore::Event&)
38  0x14f250318 WebCore::EventHandler::internalKeyEvent(WebCore::PlatformKeyboardEvent const&)
39  0x14f24f660 WebCore::EventHandler::keyEvent(WebCore::PlatformKeyboardEvent const&)
40  0x1501c5dcc WebCore::UserInputBridge::handleKeyEvent(WebCore::PlatformKeyboardEvent const&, WebCore::InputSource)
41  0x11882cfc8 WebKit::handleKeyEvent(WebKit::WebKeyboardEvent const&, WebCore::Page*)
42  0x11882cdc0 WebKit::WebPage::keyEvent(WebKit::WebKeyboardEvent const&)
43  0x1188e9e30 auto void IPC::callMemberFunction<WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&)::'lambda'(auto&&...)::operator()<WebKit::WebKeyboardEvent>(auto&&...) const
44  0x1188e9d48 decltype(std::declval<WebKit::WebPage>()(std::declval<WebKit::WebKeyboardEvent>())) std::__1::__invoke[abi:v160002]<void IPC::callMemberFunction<WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&)::'lambda'(auto&&...), WebKit::WebKeyboardEvent>(WebKit::WebPage&&, WebKit::WebKeyboardEvent&&)
45  0x1188e9d18 decltype(auto) std::__1::__apply_tuple_impl[abi:v160002]<void IPC::callMemberFunction<WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&)::'lambda'(auto&&...), std::__1::tuple<WebKit::WebKeyboardEvent>, 0ul>(WebKit::WebPage&&, WebKit::WebPage&&, std::__1::__tuple_indices<0ul>)
46  0x1188e9cd8 decltype(auto) std::__1::apply[abi:v160002]<void IPC::callMemberFunction<WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&)::'lambda'(auto&&...), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage&&, WebKit::WebPage&&)
47  0x1188e9518 void IPC::callMemberFunction<WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&)
48  0x1188ac370 void IPC::handleMessage<Messages::WebPage::KeyEvent, WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&)>(IPC::Connection&, IPC::Decoder&, WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&))
49  0x1188a4aac WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::Decoder&)
50  0x118837a1c WebKit::WebPage::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
51  0x118e1144c IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&)
52  0x117f42b30 WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
53  0x118de5420 IPC::Connection::dispatchMessage(IPC::Decoder&)
54  0x118de58b4 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder>>)
55  0x118de5bf0 IPC::Connection::dispatchOneIncomingMessage()
56  0x118e04028 IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder>>)::$_17::operator()() const
57  0x118e03f68 WTF::Detail::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder>>)::$_17, void>::call()
58  0x134422750 WTF::Function<void ()>::operator()() const
59  0x1344c67c4 WTF::RunLoop::performWork()
60  0x1344cb180 WTF::RunLoop::performWork(void*)
[snip]

There are also a couple of stack traces of EditCommands being created which ref the document but that are never cleared out of the command list that this UndoStep derived class holds.

Comment 7 Ryan Reno 2023-05-10 16:04:58 PDT

I spoke with Wenson and Brady today - they helped me understand the undo stack and UI process side of things.

I think there's two issues here:

1. The UIProcess never commands the WebProcess to clear the undo command state which keeps UndoSteps (which reference the document) alive.
2. Text nodes inserted into the DOM to receive text typed by the user are not released which keep the document alive.

A possible fix for 1. (as suggested by Wenson) is to send a command to the WebProcess from the UIProcess to clear undo commands after a main frame navigation is completed.

I'm still tracking down the reason why the Text node is kept alive. This is the backtrace from when the Text incremented the Document's referencingNodeCount but didn't clear it:

RefTracker: Backtrace for node 0x116004fd0 (http://localhost:14001/)
1   0x138751cbc WTF::RefTracker::trackDocRef(void*, WTF::String const&)
2   0x2838ae730 WebCore::Document::incrementReferencingNodeCount(WebCore::Node*)
3   0x283a82358 WebCore::Node::Node(WebCore::Document&, WTF::OptionSet<WebCore::Node::NodeFlag>)
4   0x2838301cc WebCore::CharacterData::CharacterData(WebCore::Document&, WTF::String&&, WTF::OptionSet<WebCore::Node::NodeFlag>)
5   0x283830150 WebCore::Text::Text(WebCore::Document&, WTF::String&&, WTF::OptionSet<WebCore::Node::NodeFlag>)
6   0x283b51f8c WebCore::Text::Text(WebCore::Document&, WTF::String&&, WTF::OptionSet<WebCore::Node::NodeFlag>)
7   0x283b51fe8 WebCore::Text::createEditingText(WebCore::Document&, WTF::String&&)
8   0x2838b1908 WebCore::Document::createEditingTextNode(WTF::String&&)
9   0x283c602b4 WebCore::InsertTextCommand::positionInsideTextNode(WebCore::Position const&)
10  0x283c60c20 WebCore::InsertTextCommand::doApply()
11  0x283bc8310 WebCore::CompositeEditCommand::applyCommandToComposite(WTF::Ref<WebCore::CompositeEditCommand, WTF::RawPtrTraits<WebCore::CompositeEditCommand>, WTF::RefDerefTraits>&&, WebCore::VisibleSelection const&)
12  0x283ca23d0 WebCore::TypingCommand::insertTextRunWithoutNewlines(WTF::String const&, bool)
13  0x283cc447c WebCore::TypingCommandLineOperation::operator()(unsigned long, unsigned long, bool) const
14  0x283ca2288 void WebCore::forEachLineInString<WebCore::TypingCommandLineOperation>(WTF::String const&, WebCore::TypingCommandLineOperation const&)
15  0x283ca2164 WebCore::TypingCommand::insertText(WTF::String const&, bool)
16  0x283ca0d00 WebCore::TypingCommand::insertTextAndNotifyAccessibility(WTF::String const&, bool)
17  0x283ca18b0 WebCore::TypingCommand::doApply()
18  0x283bb49f8 WebCore::CompositeEditCommand::apply()
19  0x283c8eb68 WebCore::TextInsertionBaseCommand::applyTextInsertionCommand(WebCore::LocalFrame*, WebCore::TextInsertionBaseCommand&, WebCore::VisibleSelection const&, WebCore::VisibleSelection const&)
20  0x283ca0bcc WebCore::TypingCommand::insertText(WebCore::Document&, WTF::String const&, WebCore::VisibleSelection const&, unsigned int, WebCore::TypingCommand::TextCompositionType)
21  0x283c0cabc WebCore::Editor::insertTextWithoutSendingTextEvent(WTF::String const&, bool, WebCore::TextEvent*)
22  0x283c0b9d4 WebCore::Editor::handleTextEvent(WebCore::TextEvent&)
23  0x2847b2c68 WebCore::EventHandler::defaultTextInputEventHandler(WebCore::TextEvent&)
24  0x283a8e1a4 WebCore::Node::defaultEventHandler(WebCore::Event&)
25  0x283e2bf1c WebCore::HTMLInputElement::defaultEventHandler(WebCore::Event&)
26  0x2839ecb74 WebCore::callDefaultEventHandlersInBubblingOrder(WebCore::Event&, WebCore::EventPath const&)
27  0x2839ec2d0 WebCore::EventDispatcher::dispatchEvent(WebCore::Node&, WebCore::Event&)
28  0x283a8da3c WebCore::Node::dispatchEvent(WebCore::Event&)
29  0x2847b29a0 WebCore::EventHandler::handleTextInputEvent(WTF::String const&, WebCore::Event*, WebCore::TextEventInputType)
30  0x283c14570 WebCore::Editor::insertText(WTF::String const&, WebCore::Event*, WebCore::TextEventInputType)
31  0x11bc25970 WebKit::WebPage::executeKeypressCommandsInternal(WTF::Vector<WebCore::KeypressCommand, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::KeyboardEvent*)
32  0x11bc26af4 WebKit::WebPage::handleEditingKeyboardEvent(WebCore::KeyboardEvent&)
33  0x11bb392b0 WebKit::WebEditorClient::handleKeyboardEvent(WebCore::KeyboardEvent&)
34  0x283c0b600 WebCore::Editor::handleKeyboardEvent(WebCore::KeyboardEvent&)
35  0x2847b12ec WebCore::EventHandler::defaultKeyboardEventHandler(WebCore::KeyboardEvent&)
36  0x283a8e078 WebCore::Node::defaultEventHandler(WebCore::Event&)
37  0x283e2bae0 WebCore::HTMLInputElement::defaultEventHandler(WebCore::Event&)
38  0x2839ecb74 WebCore::callDefaultEventHandlersInBubblingOrder(WebCore::Event&, WebCore::EventPath const&)
39  0x2839ec2d0 WebCore::EventDispatcher::dispatchEvent(WebCore::Node&, WebCore::Event&)
40  0x283a8da3c WebCore::Node::dispatchEvent(WebCore::Event&)
41  0x2847b0238 WebCore::EventHandler::internalKeyEvent(WebCore::PlatformKeyboardEvent const&)
42  0x2847af580 WebCore::EventHandler::keyEvent(WebCore::PlatformKeyboardEvent const&)
43  0x285725cec WebCore::UserInputBridge::handleKeyEvent(WebCore::PlatformKeyboardEvent const&, WebCore::InputSource)
44  0x11ca98fa8 WebKit::handleKeyEvent(WebKit::WebKeyboardEvent const&, WebCore::Page*)
45  0x11ca98da0 WebKit::WebPage::keyEvent(WebKit::WebKeyboardEvent const&)
46  0x11cb55e10 auto void IPC::callMemberFunction<WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&)::'lambda'(auto&&...)::operator()<WebKit::WebKeyboardEvent>(auto&&...) const
47  0x11cb55d28 decltype(std::declval<WebKit::WebPage>()(std::declval<WebKit::WebKeyboardEvent>())) std::__1::__invoke[abi:v160002]<void IPC::callMemberFunction<WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&)::'lambda'(auto&&...), WebKit::WebKeyboardEvent>(WebKit::WebPage&&, WebKit::WebKeyboardEvent&&)
48  0x11cb55cf8 decltype(auto) std::__1::__apply_tuple_impl[abi:v160002]<void IPC::callMemberFunction<WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&)::'lambda'(auto&&...), std::__1::tuple<WebKit::WebKeyboardEvent>, 0ul>(WebKit::WebPage&&, WebKit::WebPage&&, std::__1::__tuple_indices<0ul>)
49  0x11cb55cb8 decltype(auto) std::__1::apply[abi:v160002]<void IPC::callMemberFunction<WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&)::'lambda'(auto&&...), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage&&, WebKit::WebPage&&)
50  0x11cb554f8 void IPC::callMemberFunction<WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&), std::__1::tuple<WebKit::WebKeyboardEvent>&&)
51  0x11cb18350 void IPC::handleMessage<Messages::WebPage::KeyEvent, WebKit::WebPage, WebKit::WebPage, void (WebKit::WebKeyboardEvent const&)>(IPC::Connection&, IPC::Decoder&, WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::WebKeyboardEvent const&))
52  0x11cb10a8c WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::Decoder&)
53  0x11caa39fc WebKit::WebPage::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
54  0x11d07d42c IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&)
55  0x11c1aeb08 WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
56  0x11d051400 IPC::Connection::dispatchMessage(IPC::Decoder&)
57  0x11d051894 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder>>)
58  0x11d051bd0 IPC::Connection::dispatchOneIncomingMessage()
59  0x11d070008 IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder>>)::$_17::operator()() const
60  0x11d06ff48 WTF::Detail::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder>>)::$_17, void>::call()
61  0x1386b5390 WTF::Function<void ()>::operator()() const
62  0x13875f9d0 WTF::RunLoop::performWork()
63  0x13876438c WTF::RunLoop::performWork(void*)
[snip]

Comment 8 Wenson Hsieh 2023-05-10 16:13:08 PDT

(In reply to Ryan Reno from comment #7)
> 
> A possible fix for 1. (as suggested by Wenson) is to send a command to the
> WebProcess from the UIProcess to clear undo commands after a main frame
> navigation is completed.
> 

👍🏻

Just a minor clarification though — we should consider clearing commands that target the web view from the undo stack when main frame navigation *commits*, rather than when it completes.

Comment 9 Ryosuke Niwa 2023-05-10 16:17:42 PDT

(In reply to Wenson Hsieh from comment #8)
> (In reply to Ryan Reno from comment #7)
> > 
> > A possible fix for 1. (as suggested by Wenson) is to send a command to the
> > WebProcess from the UIProcess to clear undo commands after a main frame
> > navigation is completed.
> > 
> 
> 👍🏻
> 
> Just a minor clarification though — we should consider clearing commands
> that target the web view from the undo stack when main frame navigation
> *commits*, rather than when it completes.

That would mean the user won't be able to undo/redo edits once navigation happens forward or backward. I'm not certain that would be acceptable. At least in the case the page is put into back forward cache, we should probably hold onto those undo stack items.

Comment 10 Chris Dumez 2023-05-10 16:30:52 PDT

Cannot EditCommandComposition old a WeakPtr to the Document instead of a RefPtr to break the cycle?

Comment 11 Chris Dumez 2023-05-10 16:31:10 PDT

(In reply to Chris Dumez from comment #10)
> Cannot EditCommandComposition old a WeakPtr to the Document instead of a
> RefPtr to break the cycle?

old -> hold

Comment 12 Ryosuke Niwa 2023-05-10 16:32:20 PDT

Undo stack needs to keep relevant modes alive or else we won't be able to undo / redo operation.

Comment 13 Wenson Hsieh 2023-05-10 16:33:30 PDT

> That would mean the user won't be able to undo/redo edits once navigation happens forward or backward. I'm not certain that would be acceptable. At least in the case the page is put into back forward cache, we should probably hold onto those undo stack items.

Oh, that's a good point — we should definitely avoid removing items from the undo stack in that case. I wonder if we need a mechanism to purge a subset of items from the undo stack only when the associated page is ejected from the cache?

(In reply to Chris Dumez from comment #11)
> (In reply to Chris Dumez from comment #10)
> > Cannot EditCommandComposition old a WeakPtr to the Document instead of a
> > RefPtr to break the cycle?
> 
> old -> hold

I think the issue with this is that `EditCommandComposition` needs to hold arbitrary (possibly disconnected) nodes, which will strongly reference the document. Making these node references weak would break undo/redo for some editing operations that replace nodes, I think.

Comment 14 Ryosuke Niwa 2023-05-10 16:38:12 PDT

Right. Consider an operation to remove a node for example. To undo this removal we need to inset back the original node that got removed since other steps that ran before or after this undo step may reference the removed node as an insertion point, etc... our undo / redo stack currently relies on the exact same DOM structure to be restored at each step.

Comment 15 Chris Dumez 2023-05-10 16:59:06 PDT

Would it help if `UndoManager::removeAllItems()` did a client delegate call to clear the corresponding WebUndoItems from the WebKit2 layer?

Comment 16 Chris Dumez 2023-05-10 16:59:48 PDT

(In reply to Chris Dumez from comment #15)
> Would it help if `UndoManager::removeAllItems()` did a client delegate call
> to clear the corresponding WebUndoItems from the WebKit2 layer?

and UndoManager::removeItem().

Comment 17 Wenson Hsieh 2023-05-10 17:01:20 PDT

(In reply to Chris Dumez from comment #15)
> Would it help if `UndoManager::removeAllItems()` did a client delegate call
> to clear the corresponding WebUndoItems from the WebKit2 layer?

That `UndoManager` class is only used for the (off-by-default) JS API for adding/removing items to the undo stack. Currently, this is only used by Mail and HTML Notes. I don't think we exercise that code in the scenario Ryan is encountering.

Comment 18 Ryan Reno 2023-05-11 09:12:54 PDT

We should be able to destroy the undo stack after a memory pressure notification or when otherwise clearing the associated page from the cache, right?

Comment 19 Chris Dumez 2023-05-11 12:01:31 PDT

I believe I have a simple fix. I am cleaning up the patch and working on a test.

Comment 20 Chris Dumez 2023-05-11 12:13:58 PDT

Pull request: https://github.com/WebKit/WebKit/pull/13764

Comment 21 EWS 2023-05-12 13:25:10 PDT

Committed 264022@main (b9a32bf41135): <https://commits.webkit.org/264022@main>

Reviewed commits have been landed. Closing PR #13764 and removing active labels.

Comment 22 EWS 2023-06-09 10:00:16 PDT

Committed 259548.818@safari-7615-branch (9fe8826eb6a3): <https://commits.webkit.org/259548.818@safari-7615-branch>

Reviewed commits have been landed. Closing PR #636 and removing active labels.

Comment 23 Fujii Hironori 2023-06-19 18:36:33 PDT

Filed: Bug 258289 – fast/editing/document-leak-altered-text-field.html is flaky