Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: a11b943d3bbeb317297b9bef6a6b0aaa2e628be4
https://github.com/WebKit/WebKit/commit/a11b943d3bbeb317297b9bef6a6b0aaa2e628be4
Author: Ryosuke Niwa <[email protected]>
Date: 2026-09-15 (Tue, 15 Sep 2026)
Changed paths:
M Source/WebKit/UIProcess/WebEditCommandProxy.cpp
M Source/WebKit/UIProcess/WebEditCommandProxy.h
M Source/WebKit/UIProcess/WebPageProxy.cpp
M Source/WebKit/UIProcess/WebPageProxy.h
M Source/WebKit/UIProcess/WebPageProxy.messages.in
M Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.cpp
M Source/WebKit/WebProcess/WebPage/WebPage.cpp
M Source/WebKit/WebProcess/WebPage/WebPage.h
M Source/WebKit/WebProcess/WebPage/WebPage.messages.in
M Tools/TestWebKitAPI/Tests/WebKit/WKWebView/SiteIsolation.mm
Log Message:
-----------
[Site Isolation] Undo and redo do not work for edits made in a cross-origin
iframe
https://bugs.webkit.org/show_bug.cgi?id=324199
rdar://168324268
Reviewed by Sihui Liu.
Undo state is split across processes: the UI process owns the platform undo
stack (NSUndoManager on Cocoa,
DefaultUndoController elsewhere), while each WebUndoStep lives in the WebPage
of the web content process whose Editor
created it. The UI process kept no record of which process registered a given
step, so under site isolation every
message that has to reach the owner of a step was sent to the main frame's
process instead.
WebEditCommandProxy::unapply() and reapply() sent WebPage::UnapplyEditCommand
and ReapplyEditCommand to
legacyMainFrameProcess() with webPageIDInMainFrameProcess(), so a step
registered by a cross-origin iframe was looked
up in the wrong WebPage's m_undoStepMap and silently dropped.
removeEditCommand() sent DidRemoveEditCommand the same
way, so a subframe process never dropped its m_undoStepMap entry and never got
didRemoveFromUndoManager().
executeUndoRedo() drained all of m_pendingUndoRedo into the synchronous reply
of whichever process asked, handing one
process steps belonging to another. And since WebUndoStepID is an
ObjectIdentifier generated from a per-process
counter, identifiers collide across processes, so removePendingUndoRedo()
matching on the identifier alone could
retire the wrong entry.
The only reason undo appeared to work at all is the synchronous ExecuteUndoRedo
reply, which exists so that
document.execCommand('undo') updates the DOM before it returns. When the frame
that asked happened to own the step,
that reply applied it and the misdirected asynchronous message was a harmless
no-op. Undo triggered from another
frame's process, or from the platform undo manager (the Edit menu, which has no
synchronous reply at all), was lost:
the command moved to the redo stack in the UI process while the content was
never reverted.
Record the registering process and the PageIdentifier its WebPage is registered
under on WebEditCommandProxy, taken
from the IPC::Connection that registerEditCommandForUndo() already receives,
and send Unapply/ReapplyEditCommand and
DidRemoveEditCommand there. Caching that identifier rather than recomputing it
in unapply()/reapply() is deliberate:
webPageIDInProcess() falls back to m_webPageID when the given process has no
RemotePageProxy, and a
WebEditCommandProxy can outlive its remote page because the platform undo stack
goes on holding WKEditCommands until
something clears it. Recomputing would then address a subframe process with the
main frame WebPage's identifier, which
names no WebPage in that process, instead of the identifier the step was
actually registered under.
unapply() and reapply() bail out unless the owning process can still be sent to
and still hosts a WebPage under that
identifier, which hasWebPageInProcess() answers. Without that they would move
the command to the redo stack and enable
Redo in the Edit menu for an operation that cannot happen: after the iframe's
process crashes and the frame is replaced
by a placeholder, or after its RemotePageProxy and WebPage are gone while the
process is kept alive by another page, in
which case the web process drops the message silently and the asynchronous
reply handler never runs. Checking this
lazily rather than invalidating the commands when the remote page goes away is
deliberate.
RemotePageProxy::disconnect() calls isNoLongerAssociatedWithRemotePage()
unconditionally, but the ProvisionalPageProxy
m_shouldReuseMainFrame path clears the drawing area first precisely so that no
PageClose is sent and the WebPage
survives in that process under the same identifier, so invalidating there would
permanently kill live commands if the
provisional navigation were then cancelled. The lazy check recovers on its own
once that window closes.
Give each unapply or reapply request a sequence number, counted per process,
and keep the request in
m_pendingUndoRedo until the web process acknowledges it. executeUndoRedo()
hands the calling process every request
still outstanding to it, in send order, so that all of them, including the ones
this undo or redo just produced, are
applied before its execCommand() returns. Steps owned by another process are
left to their own asynchronous message,
which is the only copy they get. WebPage applies a request only if its sequence
is at or past
m_nextUndoRedoSequenceToApply, which discards whichever copy arrives second.
Acknowledgements arrive in send order, so applied requests are always a prefix
of sent ones, outstanding requests are a
contiguous suffix, and the returned run is contiguous in that process's
sequence space. The reply therefore only needs
the first sequence and the rest follow from position, which keeps the reply's
shape unchanged apart from replacing the
page-wide undoVersion with a per-process firstSequence. There is an assertion
on the contiguity in the drain loop.
This is what the undoVersion watermark could not do, in either direction. It
was bumped again in executeUndoRedo() and
the reply carried one version for the whole batch, so applying the batch pushed
the web process's watermark past the
versions of older outstanding entries. Draining every entry then applied a step
twice whenever the web process had
already applied it and its asynchronous reply had been overtaken by the sync
request, which is dispatched out of
SyncMessageState and so can be handled first. Returning only the entries
produced by the current call is not a fix
either: the watermark would then drop an older asynchronous message that had
not been applied yet and lose that undo
outright. Nor can the reply simply be skipped when something is outstanding,
since a synchronous reply is handled by
processIncomingSyncReply() and overtakes queued messages, so its steps would be
unapplied ahead of older ones, and
EditCommandComposition::unapply() run out of undo stack order corrupts the
document rather than merely duplicating an
edit. Sequence numbers per request avoid all three: every copy is identified
individually, so order comes from the
reply and duplicates are dropped on arrival, and execCommand() stays
synchronous in every case.
Added four API tests: undo and redo of an edit made in a cross-origin iframe
driven from the main frame's process, the
same driven from the platform undo manager (the path with no synchronous reply
to fall back on), undo of steps
registered by two processes on a single undo stack, checked to unapply in order
and each in its own process, and undo
after the iframe's process is killed, which must not leave Redo enabled. The
reordering window itself is a timing race
with no deterministic trigger and is not covered by a test.
Tests: SiteIsolation.UndoAndRedoEditInCrossOriginIframeFromMainFrame
SiteIsolation.UndoAndRedoEditInCrossOriginIframeFromPlatformUndoManager
SiteIsolation.UndoAfterCrossOriginIframeProcessCrashesDoesNotOfferRedo
SiteIsolation.UndoEditsRegisteredByMultipleProcesses
* Source/WebKit/UIProcess/WebEditCommandProxy.cpp:
(WebKit::WebEditCommandProxy::WebEditCommandProxy):
(WebKit::WebEditCommandProxy::process):
(WebKit::WebEditCommandProxy::unapply):
(WebKit::WebEditCommandProxy::reapply):
* Source/WebKit/UIProcess/WebEditCommandProxy.h:
(WebKit::WebEditCommandProxy::create):
(WebKit::WebEditCommandProxy::pageIDInProcess):
(WebKit::WebEditCommandProxy::invalidate):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::registerEditCommandForUndo):
(WebKit::WebPageProxy::executeUndoRedo):
(WebKit::WebPageProxy::addPendingUndoRedo):
(WebKit::WebPageProxy::removePendingUndoRedo):
(WebKit::WebPageProxy::removeEditCommand):
(WebKit::WebPageProxy::hasWebPageInProcess):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.messages.in:
* Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.cpp:
(WebKit::applyUndoRedo):
(WebKit::WebEditorClient::undo):
(WebKit::WebEditorClient::redo):
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::unapplyEditCommand):
(WebKit::WebPage::reapplyEditCommand):
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/WebPage.messages.in:
* Tools/TestWebKitAPI/Tests/WebKit/WKWebView/SiteIsolation.mm:
(TestWebKitAPI::insertTextInFrame):
(TestWebKitAPI::waitForTextContentInFrame):
(TestWebKitAPI::TEST(SiteIsolation,
UndoAndRedoEditInCrossOriginIframeFromMainFrame)):
(TestWebKitAPI::TEST(SiteIsolation,
UndoAndRedoEditInCrossOriginIframeFromPlatformUndoManager)):
(TestWebKitAPI::TEST(SiteIsolation,
UndoAfterCrossOriginIframeProcessCrashesDoesNotOfferRedo)):
(TestWebKitAPI::TEST(SiteIsolation, UndoEditsRegisteredByMultipleProcesses)):
Canonical link: https://commits.webkit.org/321198@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
