[webkit-changes] [WebKit/WebKit] 72e418: Document Leak occurs in LayoutTests/editing/mac/se...

[Nathan Solomon]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 72e418d5143c735ed4c0f2f89b0ee5c64c15a5e0
      https://github.com/WebKit/WebKit/commit/72e418d5143c735ed4c0f2f89b0ee5c64c15a5e0
  Author: Nathan Solomon <nathan_solomon at apple.com>
  Date:   2024-10-23 (Wed, 23 Oct 2024)

  Changed paths:
    A LayoutTests/editing/mac/selection/word-thai-does-not-leak-expected.txt
    A LayoutTests/editing/mac/selection/word-thai-does-not-leak.html
    M LayoutTests/editing/mac/selection/word-thai.html
    M LayoutTests/resources/document-leak-test.js
    M Source/WebCore/dom/BoundaryPoint.h
    M Source/WebCore/dom/SimpleRange.cpp
    M Source/WebCore/dom/SimpleRange.h
    M Source/WebCore/page/EventHandler.cpp
    M Source/WebCore/page/EventHandler.h
    M Source/WebCore/testing/Internals.cpp

  Log Message:
  -----------
  Document Leak occurs in LayoutTests/editing/mac/selection/word-thai.html
https://bugs.webkit.org/show_bug.cgi?id=280688
rdar://137058331

Reviewed by Ryan Reno.

The document leak occurs because the m_dragStartSelection member in
EventHandler, which is of type SimpleRange, holds Refs to Nodes.
This is a problem because the Document holds Refs to these nodes
because the document holds a strong reference to an Editor, which holds a
strong reference to a LocalFrame, which holds a strong reference to an
EventHandler.
This causes a reference cycle to occur between
the Document and the Nodes the EventHandler holds. Fix this reference
cycle by changing the type of m_dragStartSelection to a new type
that allows the member to hold a WeakReference to these nodes. Creating
this new type instead of modifying SimpleRange to hold WeakPtrs to nodes
prevents making extensive changes to areas of the codebase that can
still use a SimpleRange without causing a
reference cycle. Keep the change isolated to what code
relies on accessing m_dragStartSelection's Nodes to prevent adding
extensive amounts of null checking in other areas.

The test I ran to verify the fix was: run-webkit-tests --world-leaks LayoutTests/editing/mac/selection/word-thai.html
This test would previously fail when --world-leaks was passed in.

* Source/WebCore/dom/BoundaryPoint.h:
(WebCore::BoundaryPointWeak::BoundaryPointWeak):
* Source/WebCore/dom/SimpleRange.cpp:
(WebCore::SimpleRangeWeak::SimpleRangeWeak):
(WebCore::firstIntersectingNode):
(WebCore::nodePastLastIntersectingNode):
(WebCore::firstIntersectingNodeWithDeprecatedZeroOffsetStartQuirk):
(WebCore::commonInclusiveAncestor):
(WebCore::IntersectingNodeIterator::IntersectingNodeIterator):
* Source/WebCore/dom/SimpleRange.h:
(WebCore::SimpleRangeWeak::startContainer const):
(WebCore::SimpleRangeWeak::protectedStartContainer const):
(WebCore::SimpleRangeWeak::startOffset const):
(WebCore::SimpleRangeWeak::endContainer const):
(WebCore::SimpleRangeWeak::protectedEndContainer const):
(WebCore::SimpleRangeWeak::endOffset const):
(WebCore::SimpleRangeWeak::collapsed const):
(WebCore::SimpleRange::operator SimpleRangeWeak const):
* Source/WebCore/page/EventHandler.cpp:
(WebCore::EventHandler::updateSelectionForMouseDrag):
* Source/WebCore/page/EventHandler.h:
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::treeOrderBoundaryPoints):

Canonical link: https://commits.webkit.org/285629@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications