Bug 94324 - meta: Remove unnecessary RefPtr<Node>s from Nodes
Summary: meta: Remove unnecessary RefPtr<Node>s from Nodes
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on: 94330 94335 94336 94338 94339 94340
Blocks: 88834
  Show dependency treegraph
 
Reported: 2012-08-17 03:36 PDT by Kentaro Hara
Modified: 2024-03-15 04:33 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2012-08-17 03:36:10 PDT
Working context: I am removing reference cycles of RefPtr<Node>s.

To avoid reference cycles, we want to remove unnecessary RefPtr<Node>s from Nodes.

One example is DateInputType::m_pickerElement:

  class DateInputType {
    RefPtr<CalendarPickerElement> m_pickerElement;
  };

Given that m_pickerElement is guaranteed to exist in the shadow DOM tree of a DateInputType object, m_pickerElement is guaranteed to be kept alive while the DateInputType object is alive. Note that TreeShared.h guarantees that "If Node X has 1~ reference count, then all Nodes under X are kept alive". Consequently, m_pickerElement does not need to be a RefPtr<Node>; it can be a raw pointer.

Actually it is harmless to use RefPtr<Node>s for Nodes like m_pickerElement. However, there are some reasons why we want to replace these unnecessary RefPtr<Node>s with raw pointers:

[1] We must make sure that there are no reference cycles of RefPtr<Node>s. To guarantee that, we want to remove unnecessary RefPtr<Node>s. Think this way:

- If RefPtr<Node> is guaranteed to point to the subtree of 'this' Node, it does not need to be a RefPtr<Node>. It can be a raw pointer.

- If RefPtr<Node> is not guaranteed to point to the subtree of 'this' Node, it can potentially cause reference cycles. To avoid the reference cycles, a special trick is sometimes needed. For example, Document::m_focusedNode, Document::m_hoverNode, ...etc are implemented as RefPtr<Node>s but are not guaranteed to exist in the subtree of 'this' Node. So Document implements a special trick for these RefPtr<Node>s to avoid reference cycles (http://code.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/dom/Document.cpp&type=cs&l=698). (Note: actually the trick has a subtle bug and it can cause a reference cycle. But the details are out of the scope of the current discussion.)

Currently, since a bunch of RefPtr<Node>s are used without the trick, we cannot distinguish "safe" RefPtr<Node>s from "unsafe" RefPtr<Node>s. Here "unsafe" means that the RefPtr<Node> can cause reference cycles. To resolve the confusing situation, I want to adopt the following rules:

(a) If a RefPtr<Node> can be safely replaced with a raw pointer (i.e. if the RefPtr<Node> is guaranteed to point to the subtree), the RefPtr<Node> should be replaced with a raw pointer.

(b) If a RefPtr<Node> needs to be a RefPtr<Node> and it can cause a reference cycle, we need to implement a trick to avoid reference cycles.

(c) If a RefPtr<Node> needs to be a RefPtr<Node> and it cannot cause a reference cycle, a comment is needed to describe that.

[2] RefPtr<Node>s inside one DOM tree prevent us from implementing a new DOM lifetime semantics (bug 88834).

Practically speaking, RefPtr<Node>s that point to a shadow DOM tree can be safely replaced with raw pointers, because the shadow DOM tree is guaranteed to exist in the subtree of 'this' Node.
Comment 1 Anne van Kesteren 2024-03-15 04:33:37 PDT
I'm pretty sure there's no longer interest in doing this (see bug 94340 for instance).