Skip to content

Commit

Permalink
Use more smart pointers in editing node
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=261572

Reviewed by Darin Adler and Brent Fulgham.

* Source/WebCore/dom/DocumentMarker.h:
(WebCore::DocumentMarker::DocumentMarker):
* Source/WebCore/dom/DocumentMarkerController.cpp:
(WebCore::DocumentMarkerController::markersFor):
(WebCore::DocumentMarkerController::markersInRange):
(): Deleted.
* Source/WebCore/dom/DocumentMarkerController.h:
* Source/WebCore/editing/AlternativeTextController.cpp:
(WebCore::markersHaveIdenticalDescription):
(WebCore::AlternativeTextController::respondToChangedSelection):
(WebCore::AlternativeTextController::recordSpellcheckerResponseForModifiedCorrection):
(WebCore::AlternativeTextController::applyDictationAlternative):
* Source/WebCore/editing/ApplyStyleCommand.cpp:
(WebCore::ApplyStyleCommand::applyInlineStyleChange):
* Source/WebCore/editing/CompositeEditCommand.cpp:
(WebCore::copyMarkers):
(WebCore::CompositeEditCommand::moveParagraphs):
* Source/WebCore/editing/DeleteSelectionCommand.cpp:
(WebCore::DeleteSelectionCommand::initializePositionData):
(WebCore::DeleteSelectionCommand::handleGeneralDelete):
(WebCore::DeleteSelectionCommand::originalStringForAutocorrectionAtBeginningOfSelection):
* Source/WebCore/editing/EditCommand.h:
(WebCore::EditCommand::protectedDocument const):
* Source/WebCore/editing/Editing.cpp:
(WebCore::enclosingEmptyListItem):
* Source/WebCore/editing/Editor.cpp:
(WebCore::Editor::updateMarkersForWordsAffectedByEditing):
(WebCore::Editor::selectionStartHasMarkerFor const):
* Source/WebCore/editing/FrameSelection.cpp:
(WebCore::FrameSelection::updateFromAssociatedLiveRange):
* Source/WebCore/editing/IndentOutdentCommand.cpp:
(WebCore::IndentOutdentCommand::tryIndentingAsListItem):
(WebCore::IndentOutdentCommand::outdentParagraph):
* Source/WebCore/editing/InsertListCommand.cpp:
(WebCore::adjacentEnclosingList):
* Source/WebCore/editing/ModifySelectionListLevel.cpp:
(WebCore::getStartEndListChildren):
(WebCore::canIncreaseListLevel):
(WebCore::IncreaseSelectionListLevelCommand::doApply):
(WebCore::IncreaseSelectionListLevelCommand::canIncreaseSelectionListLevel):
(WebCore::canDecreaseListLevel):
(WebCore::DecreaseSelectionListLevelCommand::doApply):
(WebCore::DecreaseSelectionListLevelCommand::canDecreaseSelectionListLevel):
* Source/WebCore/editing/ReplaceNodeWithSpanCommand.cpp:
(WebCore::ReplaceNodeWithSpanCommand::doUnapply):
* Source/WebCore/editing/ReplaceSelectionCommand.cpp:
(WebCore::positionAvoidingPrecedingNodes):
(WebCore::ReplaceSelectionCommand::doApply):
* Source/WebCore/editing/TextIterator.cpp:
(WebCore::TextIterator::TextIterator):
(WebCore::TextIterator::init):
(WebCore::TextIterator::advance):
(WebCore::TextIterator::handleTextNode):
(WebCore::TextIterator::handleTextRun):
(WebCore::TextIterator::handleReplacedElement):
(WebCore::TextIterator::shouldRepresentNodeOffsetZero):
(WebCore::TextIterator::representNodeOffsetZero):
(WebCore::TextIterator::handleNonTextNode):
(WebCore::TextIterator::exitNode):
(WebCore::TextIterator::protectedCurrentNode const):
(WebCore::SimplifiedBackwardsTextIterator::handleReplacedElement):
(WebCore::SimplifiedBackwardsTextIterator::handleNonTextNode):
(WebCore::SimplifiedBackwardsTextIterator::exitNode):
* Source/WebCore/editing/TextIterator.h:
(WebCore::SimplifiedBackwardsTextIterator::node const):
(WebCore::SimplifiedBackwardsTextIterator::protectedNode const):
* Source/WebCore/editing/VisibleSelection.cpp:
(WebCore::VisibleSelection::adjustSelectionRespectingGranularity):
(WebCore::VisibleSelection::adjustSelectionToAvoidCrossingEditingBoundaries):
* Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:
(WebCore::replaceRichContentWithAttachments):
* Source/WebCore/editing/markup.cpp:
(WebCore::restoreAttachmentElementsInFragment):
* Source/WebCore/page/ios/FrameIOS.mm:
(WebCore::LocalFrame::interpretationsForCurrentRoot const):
* Source/WebCore/rendering/MarkedText.cpp:
(WebCore::MarkedText::collectForDocumentMarkers):
* Source/WebCore/rendering/RenderReplaced.cpp:
(WebCore::draggedContentContainsReplacedElement):
* Source/WebCore/rendering/RenderText.cpp:
(WebCore::RenderText::draggedContentRangesBetweenOffsets const):
* Source/WebCore/testing/Internals.cpp:
* Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:
(WebKit::TextCheckingControllerProxy::annotatedSubstringBetweenPositions):
* Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm:
(WebKit::WebPage::dictationAlternativesAtSelection):
* Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::getPlatformEditorState const):
* Source/WebKitLegacy/ios/WebCoreSupport/WebVisiblePosition.mm:
(-[WebVisiblePosition enclosingRangeWithDictationPhraseAlternatives:]):
(-[WebVisiblePosition enclosingRangeWithCorrectionIndicator]):
* Source/WebKitLegacy/mac/WebView/WebFrame.mm:
(-[WebFrame getDictationResultRanges:andMetadatas:]):

Canonical link: https://commits.webkit.org/268054@main
  • Loading branch information
cdumez committed Sep 16, 2023
1 parent 865af24 commit 9318f2f
Show file tree
Hide file tree
Showing 59 changed files with 593 additions and 555 deletions.
8 changes: 8 additions & 0 deletions Source/WTF/wtf/WeakPtr.h
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,14 @@ template<typename T, WeakPtrFactoryInitialization initializationMode = WeakPtrFa
initializeWeakPtrFactory();
}

CanMakeWeakPtr(const CanMakeWeakPtr&)
{
if (initializationMode == WeakPtrFactoryInitialization::Eager)
initializeWeakPtrFactory();
}

CanMakeWeakPtr& operator=(const CanMakeWeakPtr&) { return *this; }

void initializeWeakPtrFactory()
{
m_weakPtrFactory.initializeIfNeeded(static_cast<T&>(*this));
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/Modules/highlight/AppHighlightStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ static std::optional<SimpleRange> findRangeByIdentifyingStartAndEndPositions(con
if (!endContainer)
return std::nullopt;

auto start = makeContainerOffsetPosition(startContainer.get(), range.startOffset());
auto end = makeContainerOffsetPosition(endContainer.get(), range.endOffset());
auto start = makeContainerOffsetPosition(WTFMove(startContainer), range.startOffset());
auto end = makeContainerOffsetPosition(WTFMove(endContainer), range.endOffset());
if (start.isOrphan() || end.isOrphan())
return std::nullopt;

Expand Down
28 changes: 14 additions & 14 deletions Source/WebCore/accessibility/AXObjectCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,11 @@ void AccessibilityReplacedText::postTextStateChangeNotification(AXObjectCache* c
return;

VisiblePosition position = selection.start();
auto* node = highestEditableRoot(position.deepEquivalent(), HasEditableAXRole);
auto node = highestEditableRoot(position.deepEquivalent(), HasEditableAXRole);
if (m_replacedText.length())
cache->postTextReplacementNotification(node, AXTextEditTypeDelete, m_replacedText, type, text, position);
cache->postTextReplacementNotification(node.get(), AXTextEditTypeDelete, m_replacedText, type, text, position);
else
cache->postTextStateChangeNotification(node, type, text, position);
cache->postTextStateChangeNotification(node.get(), type, text, position);
}

bool AXObjectCache::gAccessibilityEnabled = false;
Expand Down Expand Up @@ -1923,14 +1923,14 @@ void AXObjectCache::postTextStateChangeNotification(Node* node, const AXTextStat

void AXObjectCache::postTextStateChangeNotification(const Position& position, const AXTextStateChangeIntent& intent, const VisibleSelection& selection)
{
Node* node = position.deprecatedNode();
auto node = position.protectedDeprecatedNode();
if (!node)
return;

stopCachingComputedObjectAttributes();

#if PLATFORM(COCOA) || USE(ATSPI)
AccessibilityObject* object = getOrCreate(node);
AccessibilityObject* object = getOrCreate(node.get());
if (object && object->accessibilityIsIgnored()) {
#if PLATFORM(COCOA)
if (position.atLastEditingPositionForNode()) {
Expand All @@ -1949,7 +1949,7 @@ void AXObjectCache::postTextStateChangeNotification(const Position& position, co

postTextStateChangeNotification(object, intent, selection);
#else
postTextStateChangeNotification(node, intent, selection);
postTextStateChangeNotification(node.get(), intent, selection);
#endif
}

Expand Down Expand Up @@ -3483,12 +3483,12 @@ CharacterOffset AXObjectCache::startCharacterOffsetOfParagraph(const CharacterOf
if (isRenderedAsNonInlineTableImageOrHR(&startNode))
return startOrEndCharacterOffsetForRange(rangeForNodeContents(startNode), true);

auto* startBlock = enclosingBlock(&startNode);
auto startBlock = enclosingBlock(&startNode);
int offset = characterOffset.startIndex + characterOffset.offset;
auto* highestRoot = highestEditableRoot(firstPositionInOrBeforeNode(&startNode));
auto highestRoot = highestEditableRoot(firstPositionInOrBeforeNode(&startNode));
Position::AnchorType type = Position::PositionIsOffsetInAnchor;

auto& node = *findStartOfParagraph(&startNode, highestRoot, startBlock, offset, type, boundaryCrossingRule);
auto& node = *findStartOfParagraph(&startNode, highestRoot.get(), startBlock.get(), offset, type, boundaryCrossingRule);

if (type == Position::PositionIsOffsetInAnchor)
return characterOffsetForNodeAndOffset(node, offset, TraverseOptionIncludeStart);
Expand All @@ -3501,16 +3501,16 @@ CharacterOffset AXObjectCache::endCharacterOffsetOfParagraph(const CharacterOffs
if (characterOffset.isNull())
return CharacterOffset();

Node* startNode = characterOffset.node;
if (isRenderedAsNonInlineTableImageOrHR(startNode))
RefPtr startNode = characterOffset.node;
if (isRenderedAsNonInlineTableImageOrHR(startNode.get()))
return startOrEndCharacterOffsetForRange(rangeForNodeContents(*startNode), false);

Node* stayInsideBlock = enclosingBlock(startNode);
auto stayInsideBlock = enclosingBlock(startNode.get());
int offset = characterOffset.startIndex + characterOffset.offset;
Node* highestRoot = highestEditableRoot(firstPositionInOrBeforeNode(startNode));
auto highestRoot = highestEditableRoot(firstPositionInOrBeforeNode(startNode.get()));
Position::AnchorType type = Position::PositionIsOffsetInAnchor;

auto& node = *findEndOfParagraph(startNode, highestRoot, stayInsideBlock, offset, type, boundaryCrossingRule);
auto& node = *findEndOfParagraph(startNode.get(), highestRoot.get(), stayInsideBlock.get(), offset, type, boundaryCrossingRule);
if (type == Position::PositionIsOffsetInAnchor) {
if (node.isTextNode()) {
CharacterOffset startOffset = startOrEndCharacterOffsetForRange(rangeForNodeContents(node), true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,7 @@ AccessibilityObjectAtspi::TextAttributes AccessibilityObjectAtspi::textAttribute
}

VisiblePosition offsetPosition = m_coreObject->visiblePositionForIndex(adjustInputOffset(*utf16Offset, m_hasListMarkerAtStart));
auto* childNode = offsetPosition.deepEquivalent().deprecatedNode();
auto childNode = offsetPosition.deepEquivalent().protectedDeprecatedNode();
if (!childNode)
return { WTFMove(defaultAttributes), -1, -1 };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2873,7 +2873,7 @@ - (NSString *)debugDescriptionForTextMarkerRange:(AXTextMarkerRangeRef)textMarke
- (void)showNodeForTextMarker:(AXTextMarkerRef)textMarker
{
auto visiblePosition = visiblePositionForTextMarker(self.axBackingObject->axObjectCache(), textMarker);
Node* node = visiblePosition.deepEquivalent().deprecatedNode();
auto node = visiblePosition.deepEquivalent().protectedDeprecatedNode();
if (!node)
return;
node->showNode();
Expand All @@ -2883,7 +2883,7 @@ - (void)showNodeForTextMarker:(AXTextMarkerRef)textMarker
- (void)showNodeTreeForTextMarker:(AXTextMarkerRef)textMarker
{
auto visiblePosition = visiblePositionForTextMarker(self.axBackingObject->axObjectCache(), textMarker);
Node* node = visiblePosition.deepEquivalent().deprecatedNode();
auto node = visiblePosition.deepEquivalent().protectedDeprecatedNode();
if (!node)
return;
node->showTreeForThis();
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/css/StyleSheetContents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ StyleSheetContents::StyleSheetContents(StyleRuleImport* ownerRule, const String&

StyleSheetContents::StyleSheetContents(const StyleSheetContents& o)
: RefCounted<StyleSheetContents>()
, CanMakeWeakPtr<StyleSheetContents>()
, m_ownerRule(nullptr)
, m_originalURL(o.m_originalURL)
, m_encodingFromCharsetRule(o.m_encodingFromCharsetRule)
Expand Down
10 changes: 10 additions & 0 deletions Source/WebCore/dom/AbstractRange.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,14 @@ std::optional<SimpleRange> makeSimpleRange(const RefPtr<AbstractRange>& range)
return makeSimpleRange(range.get());
}

Ref<Node> AbstractRange::protectedStartContainer() const
{
return startContainer();
}

Ref<Node> AbstractRange::protectedEndContainer() const
{
return endContainer();
}

}
3 changes: 3 additions & 0 deletions Source/WebCore/dom/AbstractRange.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#pragma once

#include <wtf/Ref.h>
#include <wtf/RefCounted.h>

namespace WebCore {
Expand All @@ -38,8 +39,10 @@ class AbstractRange : public RefCounted<AbstractRange> {
virtual ~AbstractRange() = default;

virtual Node& startContainer() const = 0;
Ref<Node> protectedStartContainer() const;
virtual unsigned startOffset() const = 0;
virtual Node& endContainer() const = 0;
Ref<Node> protectedEndContainer() const;
virtual unsigned endOffset() const = 0;
virtual bool collapsed() const = 0;

Expand Down
5 changes: 5 additions & 0 deletions Source/WebCore/dom/ContainerNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,11 @@ class ChildNodesLazySnapshot {
ChildNodesLazySnapshot* m_nextSnapshot;
};

inline RefPtr<ContainerNode> Node::protectedParentNode() const
{
return parentNode();
}

} // namespace WebCore

SPECIALIZE_TYPE_TRAITS_BEGIN(WebCore::ContainerNode)
Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/dom/DocumentMarker.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <variant>
#include <wtf/Forward.h>
#include <wtf/OptionSet.h>
#include <wtf/WeakPtr.h>
#include <wtf/text/WTFString.h>

#if PLATFORM(IOS_FAMILY)
Expand All @@ -35,7 +36,7 @@ namespace WebCore {

// A range of a node within a document that is "marked", such as the range of a misspelled word.
// It optionally includes a description that could be displayed in the user interface.
class DocumentMarker {
class DocumentMarker : public CanMakeWeakPtr<DocumentMarker> {
public:
enum MarkerType {
Spelling = 1 << 0,
Expand Down
12 changes: 6 additions & 6 deletions Source/WebCore/dom/DocumentMarkerController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -448,19 +448,19 @@ DocumentMarker* DocumentMarkerController::markerContainingPoint(const LayoutPoin
return nullptr;
}

Vector<RenderedDocumentMarker*> DocumentMarkerController::markersFor(Node& node, OptionSet<DocumentMarker::MarkerType> types)
Vector<WeakPtr<RenderedDocumentMarker>> DocumentMarkerController::markersFor(Node& node, OptionSet<DocumentMarker::MarkerType> types)
{
if (!possiblyHasMarkers(types))
return { };

Vector<RenderedDocumentMarker*> result;
Vector<WeakPtr<RenderedDocumentMarker>> result;
auto list = m_markers.get(&node);
if (!list)
return result;

for (auto& marker : *list) {
if (types.contains(marker.type()))
result.append(&marker);
result.append(marker);
}

return result;
Expand Down Expand Up @@ -504,12 +504,12 @@ void DocumentMarkerController::forEachOfTypes(OptionSet<DocumentMarker::MarkerTy
}
}

Vector<RenderedDocumentMarker*> DocumentMarkerController::markersInRange(const SimpleRange& range, OptionSet<DocumentMarker::MarkerType> types)
Vector<WeakPtr<RenderedDocumentMarker>> DocumentMarkerController::markersInRange(const SimpleRange& range, OptionSet<DocumentMarker::MarkerType> types)
{
// FIXME: Consider making forEach public and changing callers to use that function instead of this one.
Vector<RenderedDocumentMarker*> markers;
Vector<WeakPtr<RenderedDocumentMarker>> markers;
forEach(range, types, [&] (Node&, RenderedDocumentMarker& marker) {
markers.append(&marker);
markers.append(marker);
return false;
});
return markers;
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/dom/DocumentMarkerController.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ class DocumentMarkerController {

void dismissMarkers(OptionSet<DocumentMarker::MarkerType> = DocumentMarker::allMarkers());

WEBCORE_EXPORT Vector<RenderedDocumentMarker*> markersFor(Node&, OptionSet<DocumentMarker::MarkerType> = DocumentMarker::allMarkers());
WEBCORE_EXPORT Vector<RenderedDocumentMarker*> markersInRange(const SimpleRange&, OptionSet<DocumentMarker::MarkerType>);
WEBCORE_EXPORT Vector<WeakPtr<RenderedDocumentMarker>> markersFor(Node&, OptionSet<DocumentMarker::MarkerType> = DocumentMarker::allMarkers());
WEBCORE_EXPORT Vector<WeakPtr<RenderedDocumentMarker>> markersInRange(const SimpleRange&, OptionSet<DocumentMarker::MarkerType>);
WEBCORE_EXPORT Vector<SimpleRange> rangesForMarkersInRange(const SimpleRange&, OptionSet<DocumentMarker::MarkerType>);
void clearDescriptionOnMarkersIntersectingRange(const SimpleRange&, OptionSet<DocumentMarker::MarkerType>);

Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/dom/Node.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ class Node : public EventTarget {
virtual NodeType nodeType() const = 0;
virtual size_t approximateMemoryCost() const { return sizeof(*this); }
ContainerNode* parentNode() const;
inline RefPtr<ContainerNode> protectedParentNode() const; // Defined in ContainerNode.h.
static ptrdiff_t parentNodeMemoryOffset() { return OBJECT_OFFSETOF(Node, m_parentNode); }
inline Element* parentElement() const;
Node* previousSibling() const { return m_previous; }
Expand Down
Loading

0 comments on commit 9318f2f

Please sign in to comment.