Summary: | Introduce IDL files for runtime-enabled UndoManager and UndoItem JavaScript API | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||||
Component: | HTML Editing | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bdakin, commit-queue, megan_gardner, mitz, rniwa, thorton, webkit-bug-importer, wenson_hsieh | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 193328 | ||||||||||
Bug Blocks: | 193111 | ||||||||||
Attachments: |
|
Description
Wenson Hsieh
2019-01-03 09:06:45 PST
Created attachment 358282 [details]
WIP
Created attachment 358847 [details]
Patch
Comment on attachment 358847 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358847&action=review > Source/WebCore/page/UndoItem.idl:35 > + Constructor(DOMString label, UndoItemInit initDict) We might wanna consider making label simply an attribute of UndoItemInit. > Source/WebCore/page/UndoManager.h:37 > +class UndoManager : public RefCounted<UndoManager> { > +public: Maybe make it fast allocated & iso-heaped? > Source/WebCore/page/UndoManager.h:43 > + virtual ~UndoManager() = default; I don't think we need this. Just add ImplementationLacksVTable to IDL. > Source/WebCore/page/UndoManager.idl:27 > + EnabledAtRuntime=UndoManagerAPI Add GenerateIsReachable=ImplDocument. Add a test if possible too. > LayoutTests/ChangeLog:13 > + * editing/undo/UndoManager/undo-manager-interfaces-expected.txt: Added. > + * editing/undo/UndoManager/undo-manager-interfaces.html: Added. Why don't we just add it to editing/undo-manager/ Comment on attachment 358847 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358847&action=review > Source/WebCore/page/UndoItem.idl:34 > + EnabledAtRuntime=UndoManagerAPI, In the future, you'd need to add JSCustomMarkFunction here, then in JSUndoManagerCustom.cpp you need to invoke wrapped().callback().visitJSFunction in JSMutationObserver::visitAdditionalChildren. Then you have to make these objects reachable from the document/frame... Comment on attachment 358847 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358847&action=review Thanks for the review! >> Source/WebCore/page/UndoItem.idl:34 >> + EnabledAtRuntime=UndoManagerAPI, > > In the future, you'd need to add JSCustomMarkFunction here, > then in JSUndoManagerCustom.cpp you need to invoke wrapped().callback().visitJSFunction in JSMutationObserver::visitAdditionalChildren. > Then you have to make these objects reachable from the document/frame... Sounds good — I'll augment the next patch (for addItem support) with this change. >> Source/WebCore/page/UndoItem.idl:35 >> + Constructor(DOMString label, UndoItemInit initDict) > > We might wanna consider making label simply an attribute of UndoItemInit. Ok! I moved the label attribute into UndoItemInit. >> Source/WebCore/page/UndoManager.h:37 >> +public: > > Maybe make it fast allocated & iso-heaped? 👍 (Also did this to UndoItem) >> Source/WebCore/page/UndoManager.h:43 >> + virtual ~UndoManager() = default; > > I don't think we need this. > Just add ImplementationLacksVTable to IDL. 👍 (I also fixed this for UndoItem) >> LayoutTests/ChangeLog:13 >> + * editing/undo/UndoManager/undo-manager-interfaces.html: Added. > > Why don't we just add it to editing/undo-manager/ 👍 Comment on attachment 358847 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358847&action=review >> Source/WebCore/page/UndoManager.idl:27 >> + EnabledAtRuntime=UndoManagerAPI > > Add GenerateIsReachable=ImplDocument. Add a test if possible too. 👍 Fixed, and also added a test to verify that the wrapper survives GC. Created attachment 358873 [details]
Patch for EWS
Comment on attachment 358873 [details]
Patch for EWS
It looks like commit queue is unresponsive :(
Committed <https://trac.webkit.org/changeset/239864/webkit>. |