Bug 94324
Summary: | meta: Remove unnecessary RefPtr<Node>s from Nodes | ||
---|---|---|---|
Product: | WebKit | Reporter: | Kentaro Hara <haraken> |
Component: | DOM | Assignee: | Kentaro Hara <haraken> |
Status: | RESOLVED WONTFIX | ||
Severity: | Normal | CC: | annevk, ap, darin, morrita, tkent |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Bug Depends on: | 94330, 94335, 94336, 94338, 94339, 94340 | ||
Bug Blocks: | 88834 |
Kentaro Hara
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.
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Anne van Kesteren
I'm pretty sure there's no longer interest in doing this (see bug 94340 for instance).