Bug 1391165 - part1: EditorBase::GetChildOffset() and EditorBase::GetNodeLocation() shouldn't use nsINode::IndexOf() as far as possible r?smaug
nsINode::IndexOf() is expensive especially when it's in a hot path, it's too
expensive. So, EditorBase::GetChildOffset() and EditorBase::GetNodeLocation()
should check child's siblings first. If some of them are nullptr, it means
that it's first child or last child of the parent.
Note that EditorBase::GetChildOffset() may return wrong index if it's called
while aChild is being removed from aParent or aParent isn't actual parent of
aChild. However, there are MOZ_ASSERTs to ensure value isn't -1. Therefore,
it's safe to assume that aParent is always the parent of aChild and it won't
be called from ContentRemoved() of mutation observers.
MozReview-Commit-ID: 8JdYWuZbHe5
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -3152,29 +3152,57 @@ EditorBase::JoinNodesImpl(nsINode* aNode
RefPtr<Selection> selection = GetSelection();
NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
selection->Collapse(aNodeToKeep, AssertedCast<int32_t>(firstNodeLength));
}
return err.StealNSResult();
}
+// static
int32_t
EditorBase::GetChildOffset(nsIDOMNode* aChild,
nsIDOMNode* aParent)
{
MOZ_ASSERT(aChild && aParent);
nsCOMPtr<nsINode> parent = do_QueryInterface(aParent);
nsCOMPtr<nsINode> child = do_QueryInterface(aChild);
MOZ_ASSERT(parent && child);
- int32_t idx = parent->IndexOf(child);
- MOZ_ASSERT(idx != -1);
- return idx;
+ return GetChildOffset(child, parent);
+}
+
+// static
+int32_t
+EditorBase::GetChildOffset(nsINode* aChild,
+ nsINode* aParent)
+{
+ MOZ_ASSERT(aChild);
+ MOZ_ASSERT(aParent);
+
+ // nsINode::IndexOf() is expensive. So, if we can return index without
+ // calling it, we should do that.
+
+ // If there is no previous siblings, it means that it's the first child.
+ if (aParent->GetFirstChild() == aChild) {
+ MOZ_ASSERT(aParent->IndexOf(aChild) == 0);
+ return 0;
+ }
+
+ // If there is no next siblings, it means that it's the last child.
+ if (aParent->GetLastChild() == aChild) {
+ int32_t lastChildIndex = static_cast<int32_t>(aParent->Length() - 1);
+ MOZ_ASSERT(aParent->IndexOf(aChild) == lastChildIndex);
+ return lastChildIndex;
+ }
+
+ int32_t index = aParent->IndexOf(aChild);
+ MOZ_ASSERT(index != -1);
+ return index;
}
// static
already_AddRefed<nsIDOMNode>
EditorBase::GetNodeLocation(nsIDOMNode* aChild,
int32_t* outOffset)
{
MOZ_ASSERT(aChild && outOffset);
@@ -3195,17 +3223,17 @@ nsINode*
EditorBase::GetNodeLocation(nsINode* aChild,
int32_t* aOffset)
{
MOZ_ASSERT(aChild);
MOZ_ASSERT(aOffset);
nsINode* parent = aChild->GetParentNode();
if (parent) {
- *aOffset = parent->IndexOf(aChild);
+ *aOffset = GetChildOffset(aChild, parent);
MOZ_ASSERT(*aOffset != -1);
} else {
*aOffset = -1;
}
return parent;
}
/**
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -653,19 +653,24 @@ public:
*/
nsresult JoinNodesImpl(nsINode* aNodeToKeep,
nsINode* aNodeToJoin,
nsINode* aParent);
/**
* Return the offset of aChild in aParent. Asserts fatally if parent or
* child is null, or parent is not child's parent.
+ * FYI: aChild must not be being removed from aParent. In such case, these
+ * methods may return wrong index if aChild doesn't have previous
+ * sibling or next sibling.
*/
static int32_t GetChildOffset(nsIDOMNode* aChild,
nsIDOMNode* aParent);
+ static int32_t GetChildOffset(nsINode* aChild,
+ nsINode* aParent);
/**
* Set outOffset to the offset of aChild in the parent.
* Returns the parent of aChild.
*/
static already_AddRefed<nsIDOMNode> GetNodeLocation(nsIDOMNode* aChild,
int32_t* outOffset);
static nsINode* GetNodeLocation(nsINode* aChild, int32_t* aOffset);