Attachment 359905 Details for Bug 190009 – Fix WPE/GTK/Win

[patch] Fix WPE/GTK/Win

bug-190009-20190123105624.patch (text/plain), 37.23 KB, created by Wenson Hsieh on 2019-01-23 10:56:27 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-01-23 10:56:27 PST

Size: 37.23 KB

patch

obsolete

>Subversion Revision: 240342
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 6c5b00f9a12f147c8f77e731164a039c4528f661..589b5fbc326ed3ea66f19449cb04264c8247123c 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,38 @@
>+2019-01-23  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        Need a way for JavaScript (or bundle) code to participate in undo
>+        https://bugs.webkit.org/show_bug.cgi?id=190009
>+        <rdar://problem/44807048>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Finish hooking up `UndoManager::addItems()` to CustomUndoStep.
>+
>+        Tests: editing/undo-manager/undo-manager-add-item.html
>+               editing/undo-manager/undo-manager-delete-stale-undo-items.html
>+               editing/undo-manager/undo-manager-item-labels.html
>+               editing/undo-manager/undo-manager-undo-redo-after-garbage-collection.html
>+
>+        * editing/CompositeEditCommand.h:
>+        * editing/CustomUndoStep.cpp:
>+        (WebCore::CustomUndoStep::invalidate):
>+
>+        Add a method to invalidate CustomUndoStep. This clears out the pointer to the undo item, and also invalidates
>+        the UndoItem, removing it from its UndoManager.
>+
>+        * editing/CustomUndoStep.h:
>+        * editing/Editor.cpp:
>+        (WebCore::Editor::registerCustomUndoStep):
>+
>+        Add a helper method to register a CustomUndoStep as a platform undoable step.
>+
>+        * editing/Editor.h:
>+        * editing/UndoStep.h:
>+        * page/UndoManager.cpp:
>+        (WebCore::UndoManager::addItem):
>+
>+        Create a CustomUndoStep with the given UndoItem, and register it with the platform undo manager.
>+
> 2019-01-23  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Introduce UndoStep::label() and adopt it in WebKitLegacy and WebKit
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index ea380fb0140d8bed0d82c11f64fc1b631cb5f3d4..125fab93996b4df80d8cf132ee82ad6d48c390c0 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-01-23  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        Need a way for JavaScript (or bundle) code to participate in undo
>+        https://bugs.webkit.org/show_bug.cgi?id=190009
>+        <rdar://problem/44807048>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::WebPage::addWebUndoStep):
>+        (WebKit::WebPage::removeWebEditCommand):
>+
>+        Invalidate undo steps when removing them from WebPage. Invalidation is a no-op for editing actions that come
>+        from the UA, but for custom undo steps backed by an UndoItem, we clear out the custom undo step's pointer to its
>+        UndoItem and additionally disconnect the UndoItem from its UndoManager.
>+
>+        * WebProcess/WebPage/WebUndoStep.h:
>+        (WebKit::WebUndoStep::invalidate):
>+
> 2019-01-23  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Introduce UndoStep::label() and adopt it in WebKitLegacy and WebKit
>diff --git a/Source/WebCore/editing/CompositeEditCommand.h b/Source/WebCore/editing/CompositeEditCommand.h
>index ff1e61c6ee4597f0ad5eba29b9bd48ac60c94fbb..bc63553c367cd707b47dc9bd471ed8720edc8b70 100644
>--- a/Source/WebCore/editing/CompositeEditCommand.h
>+++ b/Source/WebCore/editing/CompositeEditCommand.h
>@@ -89,6 +89,7 @@ private:
>     EditCommandComposition(Document&, const VisibleSelection& startingSelection, const VisibleSelection& endingSelection, EditAction);
> 
>     String label() const final;
>+    void invalidate() final { }
> 
>     RefPtr<Document> m_document;
>     VisibleSelection m_startingSelection;
>diff --git a/Source/WebCore/editing/CustomUndoStep.cpp b/Source/WebCore/editing/CustomUndoStep.cpp
>index 06807522642228727686cbcddde7e272fd00d20a..b2b0b3adfe5f15598f169ecb9cfa86deda3931ef 100644
>--- a/Source/WebCore/editing/CustomUndoStep.cpp
>+++ b/Source/WebCore/editing/CustomUndoStep.cpp
>@@ -74,4 +74,10 @@ String CustomUndoStep::label() const
>     return m_undoItem->label();
> }
> 
>+void CustomUndoStep::invalidate()
>+{
>+    if (auto undoItem = std::exchange(m_undoItem, nullptr))
>+        undoItem->invalidate();
>+}
>+
> } // namespace WebCore
>diff --git a/Source/WebCore/editing/CustomUndoStep.h b/Source/WebCore/editing/CustomUndoStep.h
>index d9ead13a4a727cbfbbb80b731fb44cd010bf4cde..4283342f75e3bfa841342f3d5da256b9f56b4294 100644
>--- a/Source/WebCore/editing/CustomUndoStep.h
>+++ b/Source/WebCore/editing/CustomUndoStep.h
>@@ -49,6 +49,7 @@ private:
>     EditAction editingAction() const final { return EditAction::Unspecified; }
>     String label() const final;
> 
>+    void invalidate() final;
>     bool isValid() const;
> 
>     WeakPtr<UndoItem> m_undoItem;
>diff --git a/Source/WebCore/editing/Editor.cpp b/Source/WebCore/editing/Editor.cpp
>index 37556426af1ee0c4688fe32d18c8ab0cbc358445..8fd53ba98acb8a2e4791153fc84440640bc357a5 100644
>--- a/Source/WebCore/editing/Editor.cpp
>+++ b/Source/WebCore/editing/Editor.cpp
>@@ -39,6 +39,7 @@
> #include "ClipboardEvent.h"
> #include "CompositionEvent.h"
> #include "CreateLinkCommand.h"
>+#include "CustomUndoStep.h"
> #include "DataTransfer.h"
> #include "DeleteSelectionCommand.h"
> #include "DictationAlternative.h"
>@@ -1740,6 +1741,13 @@ void Editor::redo()
>         client()->redo();
> }
> 
>+void Editor::registerCustomUndoStep(Ref<CustomUndoStep>&& undoStep)
>+{
>+    ASSERT(RuntimeEnabledFeatures::sharedFeatures().undoManagerAPIEnabled());
>+    if (auto* client = this->client())
>+        client->registerUndoStep(WTFMove(undoStep));
>+}
>+
> void Editor::didBeginEditing()
> {
>     if (client())
>diff --git a/Source/WebCore/editing/Editor.h b/Source/WebCore/editing/Editor.h
>index 05cc1b82aeac140b40930df6f37ea54955c63f46..4547d4715f9833a867ce7f6a833378c5363d03bb 100644
>--- a/Source/WebCore/editing/Editor.h
>+++ b/Source/WebCore/editing/Editor.h
>@@ -58,6 +58,7 @@ class AlternativeTextController;
> class ArchiveResource;
> class DataTransfer;
> class CompositeEditCommand;
>+class CustomUndoStep;
> class DeleteButtonController;
> class EditCommand;
> class EditCommandComposition;
>@@ -324,6 +325,8 @@ public:
>     bool canRedo() const;
>     void redo();
> 
>+    void registerCustomUndoStep(Ref<CustomUndoStep>&&);
>+
>     void didBeginEditing();
>     void didEndEditing();
>     void willWriteSelectionToPasteboard(Range*);
>diff --git a/Source/WebCore/editing/UndoStep.h b/Source/WebCore/editing/UndoStep.h
>index 05009cb575501b5ec2d5e82645f81eb8366a8442..185aa4d489e3be9195e5fd1d96e3468537bd1eea 100644
>--- a/Source/WebCore/editing/UndoStep.h
>+++ b/Source/WebCore/editing/UndoStep.h
>@@ -44,6 +44,7 @@ public:
>     virtual void reapply() = 0;
>     virtual EditAction editingAction() const = 0;
>     virtual String label() const = 0;
>+    virtual void invalidate() = 0;
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/page/UndoManager.cpp b/Source/WebCore/page/UndoManager.cpp
>index c168acaaadfb96b847140329b66e73a55123bd61..4c6e69a4a0a3e03d29f4921d119a643014d2a10a 100644
>--- a/Source/WebCore/page/UndoManager.cpp
>+++ b/Source/WebCore/page/UndoManager.cpp
>@@ -26,6 +26,8 @@
> #include "config.h"
> #include "UndoManager.h"
> 
>+#include "CustomUndoStep.h"
>+#include "Frame.h"
> #include "UndoItem.h"
> #include <wtf/IsoMallocInlines.h>
> 
>@@ -42,9 +44,17 @@ UndoManager::~UndoManager() = default;
> 
> void UndoManager::addItem(Ref<UndoItem>&& item)
> {
>-    UNUSED_PARAM(m_document);
>+    if (m_items.contains(item.ptr())) {
>+        // FIXME: Support adding the same UndoItem to an UndoManager multiple times.
>+        return;
>+    }
>+
>+    auto frame = makeRefPtr(m_document.frame());
>+    if (!frame)
>+        return;
> 
>     item->setUndoManager(this);
>+    frame->editor().registerCustomUndoStep(CustomUndoStep::create(item));
>     m_items.add(WTFMove(item));
> }
> 
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index 9f8bb91bdd906a45acce8bc234a130fb7263c4a4..c7081af37832deb468425a5ddfbb588db46ecfc4 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -3792,12 +3792,14 @@ WebUndoStep* WebPage::webUndoStep(WebUndoStepID stepID)
> 
> void WebPage::addWebUndoStep(WebUndoStepID stepID, Ref<WebUndoStep>&& entry)
> {
>-    m_undoStepMap.set(stepID, WTFMove(entry));
>+    auto addResult = m_undoStepMap.set(stepID, WTFMove(entry));
>+    ASSERT_UNUSED(addResult, addResult.isNewEntry);
> }
> 
> void WebPage::removeWebEditCommand(WebUndoStepID stepID)
> {
>-    m_undoStepMap.remove(stepID);
>+    if (auto undoStep = m_undoStepMap.take(stepID))
>+        undoStep->invalidate();
> }
> 
> bool WebPage::isAlwaysOnLoggingAllowed() const
>diff --git a/Source/WebKit/WebProcess/WebPage/WebUndoStep.h b/Source/WebKit/WebProcess/WebPage/WebUndoStep.h
>index 211fd99e5e6c696fa7c399f7f7edd038ab8a8d8a..47fbf5db606c0d11220733ea8f7291d1e9ffc42b 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebUndoStep.h
>+++ b/Source/WebKit/WebProcess/WebPage/WebUndoStep.h
>@@ -39,6 +39,8 @@ public:
>     WebCore::UndoStep& step() const { return m_step.get(); }
>     WebUndoStepID stepID() const { return m_stepID; }
> 
>+    void invalidate() { m_step->invalidate(); }
>+
> private:
>     WebUndoStep(Ref<WebCore::UndoStep>&& step, WebUndoStepID stepID)
>         : m_step(WTFMove(step))
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index a0c6b0d0ed5e1ef6b9060c7b98fda543985bdd6f..adeb8b1627bc6b4d7567323248f6e8b5fe0f5c43 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,35 @@
>+2019-01-23  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        Need a way for JavaScript (or bundle) code to participate in undo
>+        https://bugs.webkit.org/show_bug.cgi?id=190009
>+        <rdar://problem/44807048>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add UIScriptController helpers to grab the platform undo and redo action labels. Currently only implemented for
>+        Cocoa platforms in WebKit2. See other ChangeLogs for more detail.
>+
>+        * DumpRenderTree/ios/UIScriptControllerIOS.mm:
>+        (WTR::UIScriptController::undoActionName const):
>+        (WTR::UIScriptController::redoActionName const):
>+        (WTR::UIScriptController::platformUndoManager const):
>+        * DumpRenderTree/mac/UIScriptControllerMac.mm:
>+        (WTR::UIScriptController::undoActionName const):
>+        (WTR::UIScriptController::redoActionName const):
>+        (WTR::UIScriptController::platformUndoManager const):
>+        * TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
>+        * TestRunnerShared/UIScriptContext/UIScriptController.cpp:
>+        (WTR::UIScriptController::undoActionName const):
>+        (WTR::UIScriptController::redoActionName const):
>+        * TestRunnerShared/UIScriptContext/UIScriptController.h:
>+        * WebKitTestRunner/UIScriptControllerCocoa.mm:
>+        (WTR::UIScriptController::undoActionName const):
>+        (WTR::UIScriptController::redoActionName const):
>+        * WebKitTestRunner/ios/UIScriptControllerIOS.mm:
>+        (WTR::UIScriptController::platformUndoManager const):
>+        * WebKitTestRunner/mac/UIScriptControllerMac.mm:
>+        (WTR::UIScriptController::platformUndoManager const):
>+
> 2019-01-23  Jonathan Bedard  <jbedard@apple.com>
> 
>         webkitpy: Use correct config for --iphone-simulator and --ipad-simulator
>diff --git a/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm b/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm
>index 8a1125f4ef9198452669e0419c589a8902e8a670..77e3266bd9d843f325775913508dbfa95d558b45 100644
>--- a/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm
>+++ b/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm
>@@ -450,6 +450,21 @@ void UIScriptController::setKeyboardInputModeIdentifier(JSStringRef)
> {
> }
> 
>+JSRetainPtr<JSStringRef> UIScriptController::undoActionName() const
>+{
>+    return nullptr;
>+}
>+
>+JSRetainPtr<JSStringRef> UIScriptController::redoActionName() const
>+{
>+    return nullptr;
>+}
>+
>+NSUndoManager *UIScriptController::platformUndoManager() const
>+{
>+    return nil;
>+}
>+
> }
> 
> #endif // PLATFORM(IOS_FAMILY)
>diff --git a/Tools/DumpRenderTree/mac/UIScriptControllerMac.mm b/Tools/DumpRenderTree/mac/UIScriptControllerMac.mm
>index c445145f33bef0cc1949e3d5aae0f533bee142d7..b806846137aa21f9c6fe63ea35bafbfc213329af 100644
>--- a/Tools/DumpRenderTree/mac/UIScriptControllerMac.mm
>+++ b/Tools/DumpRenderTree/mac/UIScriptControllerMac.mm
>@@ -215,6 +215,21 @@ void UIScriptController::toggleCapsLock(JSValueRef callback)
>     doAsyncTask(callback);
> }
> 
>+JSRetainPtr<JSStringRef> UIScriptController::undoActionName() const
>+{
>+    return nullptr;
>+}
>+
>+JSRetainPtr<JSStringRef> UIScriptController::redoActionName() const
>+{
>+    return nullptr;
>+}
>+
>+NSUndoManager *UIScriptController::platformUndoManager() const
>+{
>+    return nil;
>+}
>+
> }
> 
> #endif // PLATFORM(MAC)
>diff --git a/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl b/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
>index 5424ba8a8cfbe4ad814c0dabe6117337eedf967c..c9a4bbf1c5eb1a5f11dd280214c3cad9e0a5064e 100644
>--- a/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
>+++ b/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
>@@ -290,8 +290,11 @@ interface UIScriptController {
>     void makeWindowContentViewFirstResponder();
>     readonly attribute boolean isWindowContentViewFirstResponder;
> 
>+    object attachmentInfo(DOMString attachmentIdentifier);
>+
>+    // Editing
>     void drawSquareInEditableImage();
>     readonly attribute long numberOfStrokesInEditableImage;
>-
>-    object attachmentInfo(DOMString attachmentIdentifier);
>+    readonly attribute DOMString undoActionName;
>+    readonly attribute DOMString redoActionName;
> };
>diff --git a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp
>index a5baa06cd533200599f3af56893e375f6e0233c1..cd36bb096491c86dd26330a97475dfb604d2a9b2 100644
>--- a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp
>+++ b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp
>@@ -569,6 +569,16 @@ void UIScriptController::overridePreference(JSStringRef, JSStringRef)
> {
> }
> 
>+JSRetainPtr<JSStringRef> UIScriptController::undoActionName() const
>+{
>+    return nullptr;
>+}
>+
>+JSRetainPtr<JSStringRef> UIScriptController::redoActionName() const
>+{
>+    return nullptr;
>+}
>+
> #endif // !PLATFORM(COCOA)
> 
> #if !PLATFORM(MAC)
>diff --git a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
>index d3fb1cbb09250a8279dfcb28bbf8c9027f7021cb..ff7a8e9541690d26e036d835d488375ab2720e8f 100644
>--- a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
>+++ b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
>@@ -31,6 +31,8 @@
> #include <wtf/Optional.h>
> #include <wtf/Ref.h>
> 
>+OBJC_CLASS NSUndoManager;
>+
> namespace WebCore {
> class FloatRect;
> }
>@@ -206,6 +208,9 @@ public:
>     void drawSquareInEditableImage();
>     long numberOfStrokesInEditableImage();
> 
>+    JSRetainPtr<JSStringRef> undoActionName() const;
>+    JSRetainPtr<JSStringRef> redoActionName() const;
>+
>     JSObjectRef attachmentInfo(JSStringRef attachmentIdentifier);
> 
> private:
>@@ -225,6 +230,10 @@ private:
>     void platformClearAllCallbacks();
>     void platformPlayBackEventStream(JSStringRef, JSValueRef);
> 
>+#if PLATFORM(COCOA)
>+    NSUndoManager *platformUndoManager() const;
>+#endif
>+
>     JSClassRef wrapperClass() final;
> 
>     JSObjectRef objectFromRect(const WebCore::FloatRect&) const;
>diff --git a/Tools/WebKitTestRunner/UIScriptControllerCocoa.mm b/Tools/WebKitTestRunner/UIScriptControllerCocoa.mm
>index a7b98e404b83102294937f646f64ecf8925712f6..73a6c98ae6fecf1a375bdace9ac8bdb03efb364f 100644
>--- a/Tools/WebKitTestRunner/UIScriptControllerCocoa.mm
>+++ b/Tools/WebKitTestRunner/UIScriptControllerCocoa.mm
>@@ -192,5 +192,15 @@ JSObjectRef UIScriptController::calendarType() const
>     return nullptr;
> #endif
> }
>-    
>+
>+JSRetainPtr<JSStringRef> UIScriptController::undoActionName() const
>+{
>+    return JSStringCreateWithCFString((__bridge CFStringRef)platformUndoManager().undoActionName);
>+}
>+
>+JSRetainPtr<JSStringRef> UIScriptController::redoActionName() const
>+{
>+    return JSStringCreateWithCFString((__bridge CFStringRef)platformUndoManager().redoActionName);
>+}
>+
> } // namespace WTR
>diff --git a/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm b/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
>index 4aa94290578748cf81bc5355076dfa27a8a81c5d..e654039454ea01978c8ce4505596781bba875fbe 100644
>--- a/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
>+++ b/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
>@@ -971,6 +971,11 @@ JSObjectRef UIScriptController::attachmentInfo(JSStringRef jsAttachmentIdentifie
>     return JSValueToObject(m_context->jsContext(), [JSValue valueWithObject:attachmentInfoDictionary inContext:[JSContext contextWithJSGlobalContextRef:m_context->jsContext()]].JSValueRef, nullptr);
> }
> 
>+NSUndoManager *UIScriptController::platformUndoManager() const
>+{
>+    return [(UIView *)[TestController::singleton().mainWebView()->platformView() valueForKeyPath:@"_currentContentView"] undoManager];
>+}
>+
> }
> 
> #endif // PLATFORM(IOS_FAMILY)
>diff --git a/Tools/WebKitTestRunner/mac/UIScriptControllerMac.mm b/Tools/WebKitTestRunner/mac/UIScriptControllerMac.mm
>index 218881152a895cddb6a7f40716f1900a5d77ac3b..757fa06bd78f1833f13b38cbc9b030f93419763f 100644
>--- a/Tools/WebKitTestRunner/mac/UIScriptControllerMac.mm
>+++ b/Tools/WebKitTestRunner/mac/UIScriptControllerMac.mm
>@@ -209,4 +209,9 @@ void UIScriptController::toggleCapsLock(JSValueRef callback)
>     doAsyncTask(callback);
> }
> 
>+NSUndoManager *UIScriptController::platformUndoManager() const
>+{
>+    return TestController::singleton().mainWebView()->platformView().undoManager;
>+}
>+
> } // namespace WTR
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 9b4117c12bc36b1937f90e029033365ee848e1e5..4fbed4f294d6e2422d26b9971706cbc84dbfbba1 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,42 @@
>+2019-01-23  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        Need a way for JavaScript (or bundle) code to participate in undo
>+        https://bugs.webkit.org/show_bug.cgi?id=190009
>+        <rdar://problem/44807048>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a few new layout tests covering `UndoManager.addItem()`.
>+
>+        * editing/undo-manager/undo-manager-add-item-expected.txt: Added.
>+        * editing/undo-manager/undo-manager-add-item.html: Added.
>+
>+        Add a test that exercises the new API in both the top-level context and a child frame.
>+
>+        * editing/undo-manager/undo-manager-delete-stale-undo-items-expected.txt: Added.
>+        * editing/undo-manager/undo-manager-delete-stale-undo-items.html: Added.
>+
>+        Add a test to verify that after adding undo items, undoing, and then performing other edit actions, garbage
>+        collection will destroy JS wrappers for the previously added UndoItems, since these undo items' handlers can no
>+        longer be invoked.
>+
>+        * editing/undo-manager/undo-manager-item-labels-expected.txt: Added.
>+        * editing/undo-manager/undo-manager-item-labels.html: Added.
>+
>+        Add a test verifying that the undo and redo action labels are updated correctly when undoing and redoing.
>+
>+        * editing/undo-manager/undo-manager-undo-redo-after-garbage-collection-expected.txt: Added.
>+        * editing/undo-manager/undo-manager-undo-redo-after-garbage-collection.html: Added.
>+
>+        Add a test to verify that triggering garbage collection after adding an undo item without keeping references to
>+        the item (or its undo/redo handlers) doesn't break the API.
>+
>+        * resources/ui-helper.js:
>+        (window.UIHelper.undoAndRedoActionNames):
>+        (window.UIHelper):
>+
>+        Add a helper method to grab the platform's current undo and redo action names.
>+
> 2019-01-22  Simon Fraser  <simon.fraser@apple.com>
> 
>         Compositing updates need to reparent scrolling tree nodes with a changed ancestor
>diff --git a/LayoutTests/editing/undo-manager/undo-manager-add-item-expected.txt b/LayoutTests/editing/undo-manager/undo-manager-add-item-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..956da89916c7bf440816a551efeacd7340d8136d
>--- /dev/null
>+++ b/LayoutTests/editing/undo-manager/undo-manager-add-item-expected.txt
>@@ -0,0 +1,32 @@
>+This test verifies that UndoManager.addItem() can be used to add undo items to the platform undo stack.
>+
>+
>+After adding an undo item (mainframe):
>+PASS undoName is "mainframe"
>+PASS redoName is ""
>+After performing undo (mainframe):
>+** UNDO **
>+PASS undoName is ""
>+PASS redoName is "mainframe"
>+After performing redo (mainframe):
>+** REDO **
>+PASS undoName is "mainframe"
>+PASS redoName is ""
>+After adding an undo item (subframe):
>+PASS undoName is "subframe"
>+PASS redoName is ""
>+After performing undo (subframe):
>+PASS undoName is "mainframe"
>+PASS redoName is "subframe"
>+After performing redo (subframe):
>+PASS undoName is "subframe"
>+PASS redoName is ""
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>+
>+--------
>+Frame: ''
>+--------
>+UNDO, REDO
>diff --git a/LayoutTests/editing/undo-manager/undo-manager-add-item.html b/LayoutTests/editing/undo-manager/undo-manager-add-item.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..fec61890b1b81ff7e503e8bb35cb858d92a7c592
>--- /dev/null
>+++ b/LayoutTests/editing/undo-manager/undo-manager-add-item.html
>@@ -0,0 +1,93 @@
>+<!DOCTYPE html> 
>+<html>
>+    <head>
>+        <script src="../../resources/js-test.js"></script>
>+        <script src="../../resources/ui-helper.js"></script>
>+        <script>
>+        function addUndoItem()
>+        {
>+            document.undoManager.addItem(new UndoItem({
>+                label: "mainframe",
>+                undo: () => debug("** UNDO **"),
>+                redo: () => debug("** REDO **")
>+            }));
>+        }
>+        </script>
>+    </head>
>+    <body>
>+        <p>This test verifies that <code>UndoManager.addItem()</code> can be used to add undo items to the platform undo stack.</p>
>+        <iframe id="frame" srcdoc="
>+            <body>
>+                <pre id='output'></pre>
>+            </body>
>+            <script>
>+            function appendOutput(string)
>+            {
>+                if (output.textContent.length)
>+                    output.textContent += ', ';
>+                output.textContent += string;
>+            }
>+
>+            function addUndoItem()
>+            {
>+                document.undoManager.addItem(new UndoItem({
>+                    label: 'subframe',
>+                    undo: () => appendOutput('UNDO'),
>+                    redo: () => appendOutput('REDO')
>+                }));
>+            }
>+            </script>
>+        "></iframe>
>+        <pre id='console'></pre>
>+    </body>
>+    <script>
>+        jsTestIsAsync = true;
>+        undoName = null;
>+        redoName = null;
>+
>+        if (window.testRunner)
>+            testRunner.dumpChildFramesAsText();
>+
>+        addEventListener("load", async () => {
>+            debug("After adding an undo item (mainframe):");
>+            addUndoItem();
>+            [undoName, redoName] = await UIHelper.undoAndRedoActionNames();
>+            shouldBeEqualToString("undoName", "mainframe");
>+            shouldBeEqualToString("redoName", "");
>+
>+            debug("After performing undo (mainframe):");
>+            document.execCommand("undo");
>+            [undoName, redoName] = await UIHelper.undoAndRedoActionNames();
>+            shouldBeEqualToString("undoName", "");
>+            shouldBeEqualToString("redoName", "mainframe");
>+
>+            debug("After performing redo (mainframe):");
>+            document.execCommand("redo");
>+            [undoName, redoName] = await UIHelper.undoAndRedoActionNames();
>+            shouldBeEqualToString("undoName", "mainframe");
>+            shouldBeEqualToString("redoName", "");
>+
>+            const frameWindow = frame.contentWindow;
>+
>+            debug("After adding an undo item (subframe):");
>+            frameWindow.addUndoItem();
>+            [undoName, redoName] = await UIHelper.undoAndRedoActionNames();
>+            shouldBeEqualToString("undoName", "subframe");
>+            shouldBeEqualToString("redoName", "");
>+
>+            debug("After performing undo (subframe):");
>+            document.execCommand("undo");
>+            [undoName, redoName] = await UIHelper.undoAndRedoActionNames();
>+            shouldBeEqualToString("undoName", "mainframe");
>+            shouldBeEqualToString("redoName", "subframe");
>+
>+            debug("After performing redo (subframe):");
>+            document.execCommand("redo");
>+            [undoName, redoName] = await UIHelper.undoAndRedoActionNames();
>+            shouldBeEqualToString("undoName", "subframe");
>+            shouldBeEqualToString("redoName", "");
>+
>+            finishJSTest();
>+        });
>+    </script>
>+</html>
>diff --git a/LayoutTests/editing/undo-manager/undo-manager-delete-stale-undo-items-expected.txt b/LayoutTests/editing/undo-manager/undo-manager-delete-stale-undo-items-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..77e1cc2da5633ee71a9912a1ce0ebf61acf5ecb8
>--- /dev/null
>+++ b/LayoutTests/editing/undo-manager/undo-manager-delete-stale-undo-items-expected.txt
>@@ -0,0 +1,10 @@
>+E
>+Verifies that JSUndoItems are deleted when they are no longer needed by the platform clipboard. This test requires WebKitTestRunner.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+PASS objectCountBeforeClearingUndoStack - objectCountAfterClearingUndoStack is >= 580
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/editing/undo-manager/undo-manager-delete-stale-undo-items.html b/LayoutTests/editing/undo-manager/undo-manager-delete-stale-undo-items.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..e52890056f593656c1d7954e98c80275ae934ca6
>--- /dev/null
>+++ b/LayoutTests/editing/undo-manager/undo-manager-delete-stale-undo-items.html
>@@ -0,0 +1,66 @@
>+<!DOCTYPE html> 
>+<html>
>+    <meta charset="utf8">
>+    <head>
>+        <script src="../../resources/js-test.js"></script>
>+        <script src="../editing.js"></script>
>+        <script>
>+        jsTestIsAsync = true;
>+
>+        function objectCountAfterSimulatingMemoryPressureAndGarbageCollection() {
>+            internals.beginSimulatedMemoryPressure();
>+            internals.endSimulatedMemoryPressure();
>+            GCController.collect();
>+            return GCController.getJSObjectCount();
>+        }
>+
>+        async function runTest() {
>+            description("Verifies that JSUndoItems are deleted when they are no longer needed by the platform clipboard. This test requires WebKitTestRunner.");
>+
>+            if (!window.GCController)
>+                return;
>+
>+            editor = document.getElementById("editor");
>+            editor.focus();
>+
>+            const undoItemCount = 200;
>+
>+            for (let i = 0; i < undoItemCount; ++i) {
>+                document.undoManager.addItem(new UndoItem({
>+                    label: "",
>+                    undo: () => { },
>+                    redo: () => { }
>+                }));
>+            }
>+
>+            for (let i = 0; i < undoItemCount; ++i)
>+                document.execCommand("Undo");
>+
>+            objectCountBeforeClearingUndoStack = objectCountAfterSimulatingMemoryPressureAndGarbageCollection();
>+
>+            selectAllCommand();
>+            typeCharacterCommand("E");
>+
>+            // Wait until almost all of the wrappers are collected. For each UndoItem, we expect a total of 3 wrappers:
>+            // one each for the undo and redo handlers, and one for the UndoItem itself. However, we also give ourselves
>+            // some wiggle room for additional wrappers created when calling some testing helper functions. This is
>+            // still a useful test, since it will time out in the case where either our undo items or their undo/redo
>+            // handlers are not properly relinquished once they're no longer needed.
>+            const minimumNumberOfWrappersToCollectBeforePassing = 3 * undoItemCount - 20;
>+            objectCountAfterClearingUndoStack = objectCountBeforeClearingUndoStack;
>+            while (objectCountBeforeClearingUndoStack - objectCountAfterClearingUndoStack < minimumNumberOfWrappersToCollectBeforePassing) {
>+                await new Promise(resolve => setTimeout(resolve, 100));
>+                objectCountAfterClearingUndoStack = objectCountAfterSimulatingMemoryPressureAndGarbageCollection();
>+            }
>+
>+            shouldBeGreaterThanOrEqual("objectCountBeforeClearingUndoStack - objectCountAfterClearingUndoStack", `${minimumNumberOfWrappersToCollectBeforePassing}`);
>+            finishJSTest();
>+        }
>+        </script>
>+    </head>
>+    <body onload="runTest()">
>+        <div contenteditable id="editor"></div>
>+        <pre id="description"></pre>
>+        <pre id="console"></pre>
>+    </body>
>+</html>
>diff --git a/LayoutTests/editing/undo-manager/undo-manager-item-labels-expected.txt b/LayoutTests/editing/undo-manager/undo-manager-item-labels-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..d6d75c163a74464f687931bf0aa2cc8cbdd3ce30
>--- /dev/null
>+++ b/LayoutTests/editing/undo-manager/undo-manager-item-labels-expected.txt
>@@ -0,0 +1,36 @@
>+
>+Verifies that setting the label attribute of UndoItem affects the undo and redo action names in the platform undo manager.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+After typing:
>+PASS undoName is "Typing"
>+PASS redoName is ""
>+After adding the first UndoItem:
>+PASS undoName is "First ð¥"
>+PASS redoName is ""
>+After adding the second UndoItem:
>+PASS undoName is "Second ð¥"
>+PASS redoName is ""
>+After undoing:
>+PASS undoName is "First ð¥"
>+PASS redoName is "Second ð¥"
>+After undoing again:
>+PASS undoName is "Typing"
>+PASS redoName is "First ð¥"
>+After redoing:
>+PASS undoName is "First ð¥"
>+PASS redoName is "Second ð¥"
>+After redoing again:
>+PASS undoName is "Second ð¥"
>+PASS redoName is ""
>+After undoing and then pasting:
>+PASS undoName is "Paste"
>+PASS redoName is ""
>+After undoing the paste command:
>+PASS undoName is "First ð¥"
>+PASS redoName is "Paste"
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/editing/undo-manager/undo-manager-item-labels.html b/LayoutTests/editing/undo-manager/undo-manager-item-labels.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..cd61d4c70d7ba20762c3744ff356107fe1e5d4cb
>--- /dev/null
>+++ b/LayoutTests/editing/undo-manager/undo-manager-item-labels.html
>@@ -0,0 +1,95 @@
>+<!DOCTYPE html> 
>+<html>
>+    <meta charset="utf8">
>+    <head>
>+        <script src="../../resources/js-test.js"></script>
>+        <script src="../../resources/ui-helper.js"></script>
>+        <script src="../editing.js"></script>
>+        <script>
>+        jsTestIsAsync = true;
>+        undoName = null;
>+        redoName = null;
>+
>+        async function runTest() {
>+            description("Verifies that setting the label attribute of UndoItem affects the undo and redo action names "
>+                + "in the platform undo manager.");
>+
>+            field.focus();
>+
>+            debug("After typing:");
>+            document.execCommand("InsertText", true, "hello");
>+            [undoName, redoName] = await UIHelper.undoAndRedoActionNames();
>+            shouldBeEqualToString("undoName", "Typing");
>+            shouldBeEqualToString("redoName", "");
>+
>+            function createUndoItemWithLabel(labelString) {
>+                return new UndoItem({
>+                    label: labelString,
>+                    undo: () => { },
>+                    redo: () => { }
>+                });
>+            }
>+
>+            debug("After adding the first UndoItem:");
>+            const firstItem = createUndoItemWithLabel("First ð¥");
>+            document.undoManager.addItem(firstItem);
>+            [undoName, redoName] = await UIHelper.undoAndRedoActionNames();
>+            shouldBeEqualToString("undoName", firstItem.label);
>+            shouldBeEqualToString("redoName", "");
>+
>+            debug("After adding the second UndoItem:");
>+            const secondItem = createUndoItemWithLabel("Second ð¥");
>+            document.undoManager.addItem(secondItem);
>+            [undoName, redoName] = await UIHelper.undoAndRedoActionNames();
>+            shouldBeEqualToString("undoName", secondItem.label);
>+            shouldBeEqualToString("redoName", "");
>+
>+            debug("After undoing:");
>+            document.execCommand("undo");
>+            [undoName, redoName] = await UIHelper.undoAndRedoActionNames();
>+            shouldBeEqualToString("undoName", firstItem.label);
>+            shouldBeEqualToString("redoName", secondItem.label);
>+
>+            debug("After undoing again:");
>+            document.execCommand("undo");
>+            [undoName, redoName] = await UIHelper.undoAndRedoActionNames();
>+            shouldBeEqualToString("undoName", "Typing");
>+            shouldBeEqualToString("redoName", firstItem.label);
>+
>+            debug("After redoing:");
>+            document.execCommand("redo");
>+            [undoName, redoName] = await UIHelper.undoAndRedoActionNames();
>+            shouldBeEqualToString("undoName", firstItem.label);
>+            shouldBeEqualToString("redoName", secondItem.label);
>+
>+            debug("After redoing again:");
>+            document.execCommand("redo");
>+            [undoName, redoName] = await UIHelper.undoAndRedoActionNames();
>+            shouldBeEqualToString("undoName", secondItem.label);
>+            shouldBeEqualToString("redoName", "");
>+
>+            debug("After undoing and then pasting:");
>+            document.execCommand("undo");
>+            selectAllCommand();
>+            copyCommand();
>+            pasteCommand();
>+            [undoName, redoName] = await UIHelper.undoAndRedoActionNames();
>+            shouldBeEqualToString("undoName", "Paste");
>+            shouldBeEqualToString("redoName", "");
>+
>+            debug("After undoing the paste command:");
>+            document.execCommand("undo");
>+            [undoName, redoName] = await UIHelper.undoAndRedoActionNames();
>+            shouldBeEqualToString("undoName", firstItem.label);
>+            shouldBeEqualToString("redoName", "Paste");
>+
>+            finishJSTest();
>+        }
>+        </script>
>+    </head>
>+    <body onload="runTest()">
>+        <input id="field"></input>
>+        <pre id="description"></pre>
>+        <pre id="console"></pre>
>+    </body>
>+</html>
>diff --git a/LayoutTests/editing/undo-manager/undo-manager-undo-redo-after-garbage-collection-expected.txt b/LayoutTests/editing/undo-manager/undo-manager-undo-redo-after-garbage-collection-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..7c9392adc4d3d9059907c7ecb58e15ba2845ca3b
>--- /dev/null
>+++ b/LayoutTests/editing/undo-manager/undo-manager-undo-redo-after-garbage-collection-expected.txt
>@@ -0,0 +1,11 @@
>+After redo
>+Verifies that undo and redo callback handlers survive garbage collection. This test requires WebKitTestRunner.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+PASS text.textContent is "After undo"
>+PASS text.textContent is "After redo"
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/editing/undo-manager/undo-manager-undo-redo-after-garbage-collection.html b/LayoutTests/editing/undo-manager/undo-manager-undo-redo-after-garbage-collection.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..cad0ced4cee833cda19cee03cca14b42a4aa4821
>--- /dev/null
>+++ b/LayoutTests/editing/undo-manager/undo-manager-undo-redo-after-garbage-collection.html
>@@ -0,0 +1,35 @@
>+<!DOCTYPE html> 
>+<html>
>+    <meta charset="utf8">
>+    <head>
>+        <script src="../../resources/js-test.js"></script>
>+        <script src="../editing.js"></script>
>+        <script>
>+        function runTest() {
>+            description("Verifies that undo and redo callback handlers survive garbage collection. This test requires WebKitTestRunner.");
>+
>+            text = document.getElementById("text");
>+
>+            document.undoManager.addItem(new UndoItem({
>+                label: "Test action",
>+                undo: () => text.textContent = "After undo",
>+                redo: () => text.textContent = "After redo"
>+            }));
>+
>+            if (window.GCController)
>+                GCController.collect();
>+
>+            document.execCommand("Undo");
>+            shouldBeEqualToString("text.textContent", "After undo");
>+
>+            document.execCommand("Redo");
>+            shouldBeEqualToString("text.textContent", "After redo");
>+        }
>+        </script>
>+    </head>
>+    <body onload="runTest()">
>+        <div id="text">Initial state</div>
>+        <pre id="description"></pre>
>+        <pre id="console"></pre>
>+    </body>
>+</html>
>diff --git a/LayoutTests/resources/ui-helper.js b/LayoutTests/resources/ui-helper.js
>index 1884db7659650572ddcaaafc295b50202be6b564..6e8b7f19a5734d55bfc5b9134e9b84d6209a7a7e 100644
>--- a/LayoutTests/resources/ui-helper.js
>+++ b/LayoutTests/resources/ui-helper.js
>@@ -566,4 +566,13 @@ window.UIHelper = class UIHelper {
>             resolve({ x: offsetX, y: offsetY });
>         }));
>     }
>+
>+    static undoAndRedoActionNames()
>+    {
>+        if (!this.isWebKit2())
>+            return Promise.resolve();
>+
>+        const script = "JSON.stringify([uiController.undoActionName, uiController.redoActionName])";
>+        return new Promise(resolve => testRunner.runUIScript(script, result => resolve(JSON.parse(result))));
>+    }
> }

Flags:

rniwa: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 190009: 359897 | 359905 | 359919 | 360113 | 360117