Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-13 13:25:37 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1408544 - part 2: RangeBoundaryBase shouldn't compute mRef when mOffset is specified r=catalinb

Browse Source 
RangeBoundaryBase shouldn't compute mRef when it's initialized with offset.
E.g., some users of the template class may initialize it with a container and
offset in it but it may not refer mRef or may refer after modifying offset.

On the other hand, RangeBoundaryBase::InvalidateOffset() is a special method.
It assumes that mRef is always initialized when it's called but can be
invalidate mOffset until retrieved actually.  This is necessary for
nsRange::mStart and nsRange::mEnd since the offset may be changed when
some nodes are inserted before the referring node.

So, now, InvalidateOffset() should be a protected method and make nsRange a
friend class of RangeBoundaryBase.  Then, when nsRange sets mStart and/or mEnd,
nsRange itself should guarantee that their mRefs are initialized.

MozReview-Commit-ID: Alr4YkDXIND

--HG--
extra : rebase_source : 7e6828374db7989ae91b9e485571ec553f7435af
This commit is contained in:
Masayuki Nakano 2017-11-02 21:25:14 +09:00
parent 8b6c211c36
commit 45ce515072
4 changed files with 94 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

131
dom/base/RangeBoundary.h
View File

@ -11,6 +11,8 @@
#include "nsIContent.h"
#include "mozilla/Maybe.h"
class nsRange;
namespace mozilla {
// This class will maintain a reference to the child immediately
@ -43,6 +45,10 @@ class RangeBoundaryBase
template<typename T, typename U>
friend class RangeBoundaryBase;
// nsRange needs to use InvalidOffset() which requires mRef initialized
// before it's called.
friend class ::nsRange;
friend void ImplCycleCollectionTraverse(nsCycleCollectionTraversalCallback&,
RangeBoundary&, const char*,
uint32_t);
@ -67,20 +73,9 @@ public:
, mRef(nullptr)
, mOffset(mozilla::Some(aOffset))
{
if (mParent && mParent->IsContainerNode()) {
// Find a reference node
if (aOffset == static_cast<int32_t>(aContainer->GetChildCount())) {
mRef = aContainer->GetLastChild();
} else if (aOffset != 0) {
mRef = mParent->GetChildAt(aOffset - 1);
}
NS_WARNING_ASSERTION(mRef || aOffset == 0,
"Constructing RangeBoundary with invalid value");
if (!mParent) {
mOffset.reset();
}
NS_WARNING_ASSERTION(!mRef || mRef->GetParentNode() == mParent,
"Constructing RangeBoundary with invalid value");
}
protected:
@ -118,6 +113,7 @@ public:
nsIContent*
Ref() const
{
EnsureRef();
return mRef;
}
@ -133,6 +129,7 @@ public:
if (!mParent || !mParent->IsContainerNode()) {
return nullptr;
}
EnsureRef();
if (!mRef) {
MOZ_ASSERT(Offset() == 0, "invalid RangeBoundary");
return mParent->GetFirstChild();
@ -152,6 +149,7 @@ public:
if (NS_WARN_IF(!mParent) || NS_WARN_IF(!mParent->IsContainerNode())) {
return nullptr;
}
EnsureRef();
if (NS_WARN_IF(!mRef->GetNextSibling())) {
// Already referring the end of the container.
return nullptr;
@ -170,6 +168,7 @@ public:
if (NS_WARN_IF(!mParent) || NS_WARN_IF(!mParent->IsContainerNode())) {
return nullptr;
}
EnsureRef();
if (NS_WARN_IF(!mRef)) {
// Already referring the start of the container.
return nullptr;
@ -183,57 +182,33 @@ public:
if (mOffset.isSome()) {
return mOffset.value();
}
if (!mParent) {
MOZ_ASSERT(!mRef);
return 0;
}
MOZ_ASSERT(mParent->IsContainerNode(),
"If the container cannot have children, mOffset.isSome() should be true");
MOZ_ASSERT(mRef);
MOZ_ASSERT(mRef->GetParentNode() == mParent);
mOffset = mozilla::Some(mParent->IndexOf(mRef) + 1);
return mOffset.value();
}
void
InvalidateOffset()
{
MOZ_ASSERT(mParent);
MOZ_ASSERT(mParent->IsContainerNode(), "Range is positioned on a text node!");
if (!mRef) {
MOZ_ASSERT(mOffset.isSome() && mOffset.value() == 0,
"Invalidating offset of invalid RangeBoundary?");
return;
if (!mRef->GetPreviousSibling()) {
mOffset = mozilla::Some(1);
return mOffset.value();
}
mOffset.reset();
if (!mRef->GetNextSibling()) {
mOffset = mozilla::Some(mParent->GetChildCount());
return mOffset.value();
}
// Use nsINode::IndexOf() as the last resort due to being expensive.
mOffset = mozilla::Some(mParent->IndexOf(mRef) + 1);
return mOffset.value();
}
void
Set(nsINode* aContainer, int32_t aOffset)
{
mParent = aContainer;
if (mParent && mParent->IsContainerNode()) {
// Find a reference node
if (aOffset == static_cast<int32_t>(aContainer->GetChildCount())) {
mRef = aContainer->GetLastChild();
} else if (aOffset == 0) {
mRef = nullptr;
} else {
mRef = mParent->GetChildAt(aOffset - 1);
MOZ_ASSERT(mRef);
}
NS_WARNING_ASSERTION(mRef || aOffset == 0,
"Setting RangeBoundary to invalid value");
} else {
mRef = nullptr;
}
mRef = nullptr;
mOffset = mozilla::Some(aOffset);
NS_WARNING_ASSERTION(!mRef || mRef->GetParentNode() == mParent,
"Setting RangeBoundary to invalid value");
}
/**
@ -250,6 +225,7 @@ public:
if (NS_WARN_IF(!mParent)) {
return;
}
EnsureRef();
if (!mRef) {
if (!mParent->IsContainerNode()) {
// In text node or something, just increment the offset.
@ -296,6 +272,7 @@ public:
if (NS_WARN_IF(!mParent)) {
return;
}
EnsureRef();
if (!mRef) {
if (NS_WARN_IF(mParent->IsContainerNode())) {
// Already referring the start of the container
@ -343,10 +320,13 @@ public:
return false;
}
if (Ref()) {
return Ref()->GetParentNode() == Container();
if (mRef && mRef->GetParentNode() != mParent) {
return false;
}
return Offset() <= Container()->Length();
if (mOffset.isSome() && mOffset.value() > mParent->Length()) {
return false;
}
return true;
}
bool
@ -400,9 +380,50 @@ public:
return !(*this == aOther);
}
protected:
/**
* InvalidOffset() is error prone method, unfortunately. If somebody
* needs to call this method, it needs to call EnsureRef() before changing
* the position of the referencing point.
*/
void
InvalidateOffset()
{
MOZ_ASSERT(mParent);
MOZ_ASSERT(mParent->IsContainerNode(),
"Range is positioned on a text node!");
MOZ_ASSERT(mRef || (mOffset.isSome() && mOffset.value() == 0),
"mRef should be computed before a call of InvalidateOffset()");
if (!mRef) {
return;
}
mOffset.reset();
}
void
EnsureRef() const
{
if (mRef) {
return;
}
if (!mParent) {
MOZ_ASSERT(!mOffset.isSome());
return;
}
MOZ_ASSERT(mOffset.isSome());
MOZ_ASSERT(mOffset.value() <= mParent->Length());
if (!mParent->IsContainerNode() ||
mOffset.value() == 0) {
return;
}
mRef = mParent->GetChildAt(mOffset.value() - 1);
MOZ_ASSERT(mRef);
}
private:
ParentType mParent;
RefType mRef;
mutable RefType mRef;
mutable mozilla::Maybe<uint32_t> mOffset;
};

8
dom/base/nsRange.cpp
View File

@ -1020,6 +1020,14 @@ nsRange::DoSetRange(const RawRangeBoundary& aStart,
// sure to not use it here to determine our "old" current ancestor.
mStart = aStart;
mEnd = aEnd;
// If RangeBoundary is initialized with a container node and offset in it,
// its mRef may not have been initialized yet. However, nsRange will need to
// adjust the offset when the node position is changed. In such case,
// RangeBoundary::mRef needs to be initialized for recomputing offset later.
mStart.EnsureRef();
mEnd.EnsureRef();
mIsPositioned = !!mStart.Container();
if (checkCommonAncestor) {
nsINode* oldCommonAncestor = mRegisteredCommonAncestor;

17
editor/libeditor/EditorBase.cpp
View File

@ -4043,15 +4043,12 @@ EditorBase::JoinNodeDeep(nsIContent& aLeftNode,
nsCOMPtr<nsIContent> rightNodeToJoin = &aRightNode;
nsCOMPtr<nsINode> parentNode = aRightNode.GetParentNode();
nsCOMPtr<nsINode> resultNode = nullptr;
int32_t resultOffset = -1;
EditorDOMPoint ret;
while (leftNodeToJoin && rightNodeToJoin && parentNode &&
AreNodesSameType(leftNodeToJoin, rightNodeToJoin)) {
uint32_t length = leftNodeToJoin->Length();
resultNode = rightNodeToJoin;
resultOffset = length;
ret.Set(rightNodeToJoin, length);
// Do the join
nsresult rv = JoinNodes(*leftNodeToJoin, *rightNodeToJoin);
@ -4061,7 +4058,7 @@ EditorBase::JoinNodeDeep(nsIContent& aLeftNode,
if (parentNode->GetAsText()) {
// We've joined all the way down to text nodes, we're done!
return EditorDOMPoint(resultNode, resultOffset);
return ret;
}
// Get new left and right nodes, and begin anew
@ -4078,22 +4075,22 @@ EditorBase::JoinNodeDeep(nsIContent& aLeftNode,
leftNodeToJoin = leftNodeToJoin->GetPreviousSibling();
}
if (!leftNodeToJoin) {
return EditorDOMPoint(resultNode, resultOffset);
return ret;
}
while (rightNodeToJoin && !IsEditable(rightNodeToJoin)) {
rightNodeToJoin = rightNodeToJoin->GetNextSibling();
}
if (!rightNodeToJoin) {
return EditorDOMPoint(resultNode, resultOffset);
return ret;
}
}
if (NS_WARN_IF(!resultNode)) {
if (NS_WARN_IF(!ret.IsSet())) {
return EditorDOMPoint();
}
return EditorDOMPoint(resultNode, resultOffset);
return ret;
}
void

7
editor/libeditor/HTMLEditRules.cpp
View File

@ -7364,8 +7364,7 @@ HTMLEditRules::JoinNodesSmart(nsIContent& aNodeLeft,
}
}
nsCOMPtr<nsINode> resultNode = &aNodeRight;
int32_t resultOffset = aNodeLeft.Length();
EditorDOMPoint ret(&aNodeRight, aNodeLeft.Length());
// Separate join rules for differing blocks
if (HTMLEditUtils::IsList(&aNodeLeft) || aNodeLeft.GetAsText()) {
@ -7374,7 +7373,7 @@ HTMLEditRules::JoinNodesSmart(nsIContent& aNodeLeft,
if (NS_WARN_IF(NS_FAILED(rv))) {
return EditorDOMPoint();
}
return EditorDOMPoint(resultNode, resultOffset);
return ret;
}
// Remember the last left child, and first right child
@ -7414,7 +7413,7 @@ HTMLEditRules::JoinNodesSmart(nsIContent& aNodeLeft,
}
return JoinNodesSmart(*lastLeft, *firstRight);
}
return EditorDOMPoint(resultNode, resultOffset);
return ret;
}
Element*