Changeset 240315 in webkit


Ignore:
Timestamp:
Jan 22, 2019 6:06:53 PM (5 years ago)
Author:
Wenson Hsieh
Message:

Add some bindings-related bookkeeping to UndoManager and UndoItem
​https://bugs.webkit.org/show_bug.cgi?id=193111
<rdar://problem/44807048>

Reviewed by Ryosuke Niwa.

This patch is work in progress towards supporting UndoManager.addItem(). Here, we add helper methods to
UndoItem and UndoManager which later patches will exercise, as well as introduce some custom bindings to
properly handle the case where UndoItems are given anonymous JavaScript functions (see below for more details).

No new tests, because there is no script-observable change in behavior yet. When addItems() is hooked up, I
will write a test to verify that the undo and redo JavaScript functions survive garbage collection.

  • Sources.txt:
  • WebCore.xcodeproj/project.pbxproj:
  • bindings/js/JSUndoItemCustom.cpp:

(WebCore::JSUndoItem::visitAdditionalChildren):

Have each JSUndoItem visit its undo and redo callback functions to ensure that the JavaScript wrapper objects
for these functions are not garbage collected underneath the item.

(WebCore::JSUndoItemOwner::isReachableFromOpaqueRoots):

Consider the undo item wrapper reachable from opaque roots if it is associated with its UndoManager's Document.
This ensures that if script isn't holding on to a reference to the wrapper (for instance, by calling
UndoManager.addItem(new UndoItem({ ... }))), we still protect the corresponding JSUndoItem as long as the
UndoManager's Document is alive. In the case where the undo item is not associated with a document, either (1)
script is keeping a reference to it, in which case it will be trivially reachable, or (2) script won't be able
to observe the destruction of the wrapper anyways (e.g. calling new UndoItem({ ... }) by itself).

  • dom/Document.cpp:

(WebCore::Document::prepareForDestruction):

Invalidate all undo items when the document is about to go away.

  • page/UndoItem.cpp:

(WebCore::UndoItem::setUndoManager):
(WebCore::UndoItem::invalidate):
(WebCore::UndoItem::isValid const):

Add a few helpers, to be used in a future patch. We consider an UndoItem valid if it has been added to an
UndoManager, and is thus associated with a document.

(WebCore::UndoItem::document const):

  • page/UndoItem.h:
  • page/UndoItem.idl:
  • page/UndoManager.cpp:

(WebCore::UndoManager::UndoManager):
(WebCore::UndoManager::addItem):

Have an UndoManager keep its UndoItems alive. These UndoItems remain in this set until either the document will
be destroyed, or the corresponding undo action is no longer needed because the platform undo stack has changed
(this latter behavior is yet to be implemented).

(WebCore::UndoManager::removeItem):
(WebCore::UndoManager::removeAllItems):

  • page/UndoManager.h:

(WebCore::UndoManager::UndoManager): Deleted.

  • page/scrolling/ScrollingTreeScrollingNode.cpp:

Unified build fix.

Location:
trunk/Source/WebCore
Files:
8 edited
2 copied

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r240313 r240315  
     12019-01-22  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        Add some bindings-related bookkeeping to UndoManager and UndoItem
     4        https://bugs.webkit.org/show_bug.cgi?id=193111
     5        <rdar://problem/44807048>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        This patch is work in progress towards supporting `UndoManager.addItem()`. Here, we add helper methods to
     10        UndoItem and UndoManager which later patches will exercise, as well as introduce some custom bindings to
     11        properly handle the case where UndoItems are given anonymous JavaScript functions (see below for more details).
     12
     13        No new tests, because there is no script-observable change in behavior yet. When `addItems()` is hooked up, I
     14        will write a test to verify that the undo and redo JavaScript functions survive garbage collection.
     15
     16        * Sources.txt:
     17        * WebCore.xcodeproj/project.pbxproj:
     18        * bindings/js/JSUndoItemCustom.cpp:
     19        (WebCore::JSUndoItem::visitAdditionalChildren):
     20
     21        Have each JSUndoItem visit its undo and redo callback functions to ensure that the JavaScript wrapper objects
     22        for these functions are not garbage collected underneath the item.
     23
     24        (WebCore::JSUndoItemOwner::isReachableFromOpaqueRoots):
     25
     26        Consider the undo item wrapper reachable from opaque roots if it is associated with its UndoManager's Document.
     27        This ensures that if script isn't holding on to a reference to the wrapper (for instance, by calling
     28        `UndoManager.addItem(new UndoItem({ ... }))`), we still protect the corresponding JSUndoItem as long as the
     29        UndoManager's Document is alive. In the case where the undo item is not associated with a document, either (1)
     30        script is keeping a reference to it, in which case it will be trivially reachable, or (2) script won't be able
     31        to observe the destruction of the wrapper anyways (e.g. calling `new UndoItem({ ... })` by itself).
     32
     33        * dom/Document.cpp:
     34        (WebCore::Document::prepareForDestruction):
     35
     36        Invalidate all undo items when the document is about to go away.
     37
     38        * page/UndoItem.cpp:
     39        (WebCore::UndoItem::setUndoManager):
     40        (WebCore::UndoItem::invalidate):
     41        (WebCore::UndoItem::isValid const):
     42
     43        Add a few helpers, to be used in a future patch. We consider an UndoItem valid if it has been added to an
     44        UndoManager, and is thus associated with a document.
     45
     46        (WebCore::UndoItem::document const):
     47        * page/UndoItem.h:
     48        * page/UndoItem.idl:
     49        * page/UndoManager.cpp:
     50        (WebCore::UndoManager::UndoManager):
     51        (WebCore::UndoManager::addItem):
     52
     53        Have an UndoManager keep its UndoItems alive. These UndoItems remain in this set until either the document will
     54        be destroyed, or the corresponding undo action is no longer needed because the platform undo stack has changed
     55        (this latter behavior is yet to be implemented).
     56
     57        (WebCore::UndoManager::removeItem):
     58        (WebCore::UndoManager::removeAllItems):
     59        * page/UndoManager.h:
     60        (WebCore::UndoManager::UndoManager): Deleted.
     61        * page/scrolling/ScrollingTreeScrollingNode.cpp:
     62
     63        Unified build fix.
     64
    1652019-01-22  Fujii Hironori  <Hironori.Fujii@sony.com>
    266

  • trunk/Source/WebCore/Sources.txt

    r240230 r240315  
    527527bindings/js/JSTreeWalkerCustom.cpp
    528528bindings/js/JSTypedOMCSSStyleValueCustom.cpp
     529bindings/js/JSUndoItemCustom.cpp
    529530bindings/js/JSVideoTrackCustom.cpp
    530531bindings/js/JSVideoTrackListCustom.cpp
    … …  
    15371538page/SuspendableTimer.cpp
    15381539page/TextIndicator.cpp
     1540page/UndoItem.cpp
    15391541page/UndoManager.cpp
    15401542page/UserContentProvider.cpp

  • trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj

    r240272 r240315  
    70997099                319BDE531E7A86C100BA296D /* JSRTCPeerConnectionState.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSRTCPeerConnectionState.h; sourceTree = "<group>"; };
    71007100                319FBD5D15D2F444009640A6 /* CachedImageClient.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CachedImageClient.h; sourceTree = "<group>"; };
    7101                 31A088C41E737B2C003B6609 /* JSWebGPURenderingContextCustom.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSWebGPURenderingContextCustom.cpp; sourceTree = "<group>"; };
    7102                 31A088C51E737B2C003B6609 /* JSWebGPURenderPassAttachmentDescriptorCustom.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSWebGPURenderPassAttachmentDescriptorCustom.cpp; sourceTree = "<group>"; };
    71037101                31A0891B1E738D59003B6609 /* JSWebGPUBuffer.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSWebGPUBuffer.cpp; sourceTree = "<group>"; };
    71047102                31A0891D1E738D59003B6609 /* JSWebGPUBuffer.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSWebGPUBuffer.h; sourceTree = "<group>"; };
    … …  
    1513215130                F4D9817D2195FBF6008230FC /* ChangeListTypeCommand.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ChangeListTypeCommand.h; sourceTree = "<group>"; };
    1513315131                F4D9817E2195FBF6008230FC /* ChangeListTypeCommand.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = ChangeListTypeCommand.cpp; sourceTree = "<group>"; };
     15132                F4E1965A21F2395000285078 /* JSUndoItemCustom.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSUndoItemCustom.cpp; sourceTree = "<group>"; };
     15133                F4E1965F21F26E4E00285078 /* UndoItem.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = UndoItem.cpp; sourceTree = "<group>"; };
    1513415134                F4E57EDA213F3F5F004EA98E /* FontAttributeChanges.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = FontAttributeChanges.h; sourceTree = "<group>"; };
    1513515135                F4E57EDF213F434A004EA98E /* WebCoreNSFontManagerExtras.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = WebCoreNSFontManagerExtras.h; sourceTree = "<group>"; };
    … …  
    1717017170                                1CECB3B821F50D1000F44542 /* WHLSLNativeFunctionWriter.cpp */,
    1717117171                                1CECB3B921F50D1000F44542 /* WHLSLNativeFunctionWriter.h */,
     17172                                1CECB3C621F59C8700F44542 /* WHLSLNativeTypeWriter.cpp */,
     17173                                1CECB3C721F59C8700F44542 /* WHLSLNativeTypeWriter.h */,
    1717217174                                1CECB3B121F2B98500F44542 /* WHLSLTypeNamer.cpp */,
    1717317175                                1CECB3B021F2B98500F44542 /* WHLSLTypeNamer.h */,
    17174                                 1CECB3C621F59C8700F44542 /* WHLSLNativeTypeWriter.cpp */,
    17175                                 1CECB3C721F59C8700F44542 /* WHLSLNativeTypeWriter.h */,
    1717617176                        );
    1717717177                        path = Metal;
    … …  
    2043520435                                2D4F96F11A1ECC240098BF88 /* TextIndicator.cpp */,
    2043620436                                2D4F96F21A1ECC240098BF88 /* TextIndicator.h */,
     20437                                F4E1965F21F26E4E00285078 /* UndoItem.cpp */,
    2043720438                                2ECDBAD521D8906300F00ECD /* UndoItem.h */,
    2043820439                                2ECDBAD721D8906300F00ECD /* UndoItem.idl */,
    … …  
    2080320804                                516BB7920CE91E6800512F79 /* JSTreeWalkerCustom.cpp */,
    2080420805                                4BAFD0E22192604D00C0AB64 /* JSTypedOMCSSStyleValueCustom.cpp */,
     20806                                F4E1965A21F2395000285078 /* JSUndoItemCustom.cpp */,
    2080520807                                BE6DF708171CA2C500DD52B8 /* JSVideoTrackCustom.cpp */,
    2080620808                                BE6DF70A171CA2C500DD52B8 /* JSVideoTrackListCustom.cpp */,
    … …  
    2080820810                                D3F3D3591A69A3B00059FC2B /* JSWebGL2RenderingContextCustom.cpp */,
    2080920811                                49EED14C1051971A00099FAB /* JSWebGLRenderingContextCustom.cpp */,
    20810                                 31A088C41E737B2C003B6609 /* JSWebGPURenderingContextCustom.cpp */,
    20811                                 31A088C51E737B2C003B6609 /* JSWebGPURenderPassAttachmentDescriptorCustom.cpp */,
    2081220812                                E18258AB0EF3CD7000933242 /* JSWorkerGlobalScopeCustom.cpp */,
    2081320813                                46F91BC91FCDD0FE001599C3 /* JSWorkerNavigatorCustom.cpp */,

  • trunk/Source/WebCore/bindings/js/JSUndoItemCustom.cpp

    r240314 r240315  
    2525
    2626#include "config.h"
    27 #include "UndoManager.h"
    28 
    29 #include "UndoItem.h"
    30 #include <wtf/IsoMallocInlines.h>
     27#include "JSUndoItem.h"
    3128
    3229namespace WebCore {
    3330
    34 WTF_MAKE_ISO_ALLOCATED_IMPL(UndoManager);
     31void JSUndoItem::visitAdditionalChildren(JSC::SlotVisitor& visitor)
     32{
     33    wrapped().undoHandler().visitJSFunction(visitor);
     34    wrapped().redoHandler().visitJSFunction(visitor);
     35}
    3536
    36 void UndoManager::addItem(Ref<UndoItem>&& item)
     37bool JSUndoItemOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> handle, void*, SlotVisitor& visitor, const char** reason)
    3738{
    38     UNUSED_PARAM(item);
    39     UNUSED_PARAM(m_document);
     39    if (UNLIKELY(reason))
     40        *reason = "Document is an opaque root.";
     41
     42    auto* documentForUndoItem = jsCast<JSUndoItem*>(handle.slot()->asCell())->wrapped().document();
     43    return documentForUndoItem && visitor.containsOpaqueRoot(documentForUndoItem);
    4044}
    4145

  • trunk/Source/WebCore/dom/Document.cpp

    r240237 r240315  
    25022502#endif
    25032503
     2504    m_undoManager->removeAllItems();
     2505
    25042506#if HAVE(ACCESSIBILITY)
    25052507    if (this != &topDocument()) {

  • trunk/Source/WebCore/page/UndoItem.cpp

    r240314 r240315  
    2525
    2626#include "config.h"
     27#include "UndoItem.h"
     28
    2729#include "UndoManager.h"
    28 
    29 #include "UndoItem.h"
    3030#include <wtf/IsoMallocInlines.h>
    3131
    3232namespace WebCore {
    3333
    34 WTF_MAKE_ISO_ALLOCATED_IMPL(UndoManager);
     34WTF_MAKE_ISO_ALLOCATED_IMPL(UndoItem);
    3535
    36 void UndoManager::addItem(Ref<UndoItem>&& item)
     36void UndoItem::setUndoManager(UndoManager* undoManager)
    3737{
    38     UNUSED_PARAM(item);
    39     UNUSED_PARAM(m_document);
     38    m_undoManager = makeWeakPtr(undoManager);
     39}
     40
     41void UndoItem::invalidate()
     42{
     43    if (auto* undoManager = m_undoManager.get()) {
     44        undoManager->removeItem(*this);
     45        m_undoManager = nullptr;
     46    }
     47}
     48
     49bool UndoItem::isValid() const
     50{
     51    return !!m_undoManager;
     52}
     53
     54Document* UndoItem::document() const
     55{
     56    if (!isValid())
     57        return nullptr;
     58
     59    return &m_undoManager->document();
    4060}
    4161

  • trunk/Source/WebCore/page/UndoItem.h

    r239864 r240315  
    2727
    2828#include "VoidCallback.h"
    29 #include <wtf/Function.h>
    30 #include <wtf/IsoMallocInlines.h>
     29#include <wtf/IsoMalloc.h>
    3130#include <wtf/RefCounted.h>
     31#include <wtf/WeakPtr.h>
    3232#include <wtf/text/WTFString.h>
    3333
    3434namespace WebCore {
    3535
     36class Document;
     37class UndoManager;
     38
    3639class UndoItem : public RefCounted<UndoItem> {
    37     WTF_MAKE_ISO_ALLOCATED_INLINE(UndoItem);
     40    WTF_MAKE_ISO_ALLOCATED(UndoItem);
    3841public:
    3942    struct Init {
    … …  
    4750        return adoptRef(*new UndoItem(WTFMove(init)));
    4851    }
     52
     53    bool isValid() const;
     54    void invalidate();
     55
     56    Document* document() const;
     57
     58    void setUndoManager(UndoManager*);
    4959
    5060    const String& label() const { return m_label; }
    … …  
    6373    Ref<VoidCallback> m_undoHandler;
    6474    Ref<VoidCallback> m_redoHandler;
     75    WeakPtr<UndoManager> m_undoManager;
    6576};
    6677

  • trunk/Source/WebCore/page/UndoItem.idl

    r239864 r240315  
    3535    EnabledAtRuntime=UndoManagerAPI,
    3636    ImplementationLacksVTable,
     37    JSCustomMarkFunction,
     38    CustomIsReachable,
    3739    Constructor(UndoItemInit initDict),
    3840] interface UndoItem {

  • trunk/Source/WebCore/page/UndoManager.cpp

    r239864 r240315  
    3434WTF_MAKE_ISO_ALLOCATED_IMPL(UndoManager);
    3535
     36UndoManager::UndoManager(Document& document)
     37    : m_document(document)
     38{
     39}
     40
     41UndoManager::~UndoManager() = default;
     42
    3643void UndoManager::addItem(Ref<UndoItem>&& item)
    3744{
    38     UNUSED_PARAM(item);
    3945    UNUSED_PARAM(m_document);
     46
     47    item->setUndoManager(this);
     48    m_items.add(WTFMove(item));
     49}
     50
     51void UndoManager::removeItem(UndoItem& item)
     52{
     53    if (auto foundItem = m_items.take(&item))
     54        foundItem->setUndoManager(nullptr);
     55}
     56
     57void UndoManager::removeAllItems()
     58{
     59    for (auto& item : m_items)
     60        item->setUndoManager(nullptr);
     61    m_items.clear();
    4062}
    4163

  • trunk/Source/WebCore/page/UndoManager.h

    r239864 r240315  
    2929#include <wtf/Ref.h>
    3030#include <wtf/RefCounted.h>
     31#include <wtf/RefPtr.h>
     32#include <wtf/WeakPtr.h>
    3133
    3234namespace WebCore {
    … …  
    3537class UndoItem;
    3638
    37 class UndoManager : public RefCounted<UndoManager> {
     39class UndoManager : public RefCounted<UndoManager>, public CanMakeWeakPtr<UndoManager> {
    3840    WTF_MAKE_ISO_ALLOCATED(UndoManager);
    3941public:
    … …  
    4345    }
    4446
     47    ~UndoManager();
     48
     49    void removeItem(UndoItem&);
     50    void removeAllItems();
    4551    void addItem(Ref<UndoItem>&&);
    4652    Document& document() { return m_document; }
    4753
    4854private:
    49     UndoManager(Document& document)
    50         : m_document(document)
    51     {
    52     }
     55    UndoManager(Document&);
    5356
    5457    Document& m_document;
     58    HashSet<RefPtr<UndoItem>> m_items;
    5559};
    5660
Note: See TracChangeset for help on using the changeset viewer.