94324 2012-08-17 03:36:10 -0700 meta: Remove unnecessary RefPtr<Node>s from Nodes 2024-03-15 04:33:37 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED WONTFIX P2 Normal --- 94330 94335 94336 94338 94339 94340 88834 1 haraken haraken annevk ap darin morrita tkent oldest_to_newest 698149 0 haraken 2012-08-17 03:36:10 -0700 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. 2021292 1 annevk 2024-03-15 04:33:37 -0700 I'm pretty sure there's no longer interest in doing this (see bug 94340 for instance).