Skip to content
Permalink
Browse files
[iOS] Web content process occasionally crashes under VisibleSelection…
…::adjustPositionForEnd

https://bugs.webkit.org/show_bug.cgi?id=225072
rdar://77159489

Reviewed by Darin Adler.

Source/WebCore:

Remove a helper method that was added in r276688, which we no longer need.

* dom/Position.h:
(WebCore::Position::isInTreeScope const): Deleted.

Source/WebKit:

The fix for this crash that I landed in r276688 was based on a misunderstanding that accessing `treeScope()`
would trigger a null pointer deref if `isInTreeScope()` is `false`. This is not the case, since the `treeScope`
of a `Node` is a separate notion from whether that `Node` is connected to said tree scope.

Since this crash is really due to dereferencing a null container node in the case where `selectionStart` or
`selectionEnd` are null, a better fix is to just check the nullity of the container node.

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::rangeForPointInRootViewCoordinates):


Canonical link: https://commits.webkit.org/237142@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@276742 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
whsieh committed Apr 29, 2021
1 parent d72a65b commit fabc0663780233e0d4ebe14f7fb4dfa1450dcf8d
Showing 4 changed files with 35 additions and 6 deletions.
@@ -1,3 +1,16 @@
2021-04-28 Wenson Hsieh <wenson_hsieh@apple.com>

[iOS] Web content process occasionally crashes under VisibleSelection::adjustPositionForEnd
https://bugs.webkit.org/show_bug.cgi?id=225072
rdar://77159489

Reviewed by Darin Adler.

Remove a helper method that was added in r276688, which we no longer need.

* dom/Position.h:
(WebCore::Position::isInTreeScope const): Deleted.

2021-04-28 Cameron McCormack <heycam@apple.com>

Don't create custom scrollbars for fullscreen placeholders.
@@ -124,8 +124,6 @@ class Position {
return container ? container->rootEditableElement() : nullptr;
}

bool isInTreeScope() const { return m_anchorNode && m_anchorNode->isInTreeScope(); }

// These should only be used for PositionIsOffsetInAnchor positions, unless
// the position is a legacy editing position.
void moveToPosition(Node* anchorNode, unsigned offset);
@@ -1,3 +1,21 @@
2021-04-28 Wenson Hsieh <wenson_hsieh@apple.com>

[iOS] Web content process occasionally crashes under VisibleSelection::adjustPositionForEnd
https://bugs.webkit.org/show_bug.cgi?id=225072
rdar://77159489

Reviewed by Darin Adler.

The fix for this crash that I landed in r276688 was based on a misunderstanding that accessing `treeScope()`
would trigger a null pointer deref if `isInTreeScope()` is `false`. This is not the case, since the `treeScope`
of a `Node` is a separate notion from whether that `Node` is connected to said tree scope.

Since this crash is really due to dereferencing a null container node in the case where `selectionStart` or
`selectionEnd` are null, a better fix is to just check the nullity of the container node.

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::rangeForPointInRootViewCoordinates):

2021-04-28 Basuke Suzuki <basuke.suzuki@sony.com>

Suppress warnings for %{private}s format specifier
@@ -1524,15 +1524,15 @@ static IntPoint constrainPoint(const IntPoint& point, const Frame& frame, const
if (baseIsStart) {
if (result <= selectionStart)
result = selectionStart.next();
else if (targetNode && targetNode->isInTreeScope() && selectionStart.deepEquivalent().isInTreeScope() && selectionStart.deepEquivalent().treeScope() != &targetNode->treeScope())
result = VisibleSelection::adjustPositionForEnd(result.deepEquivalent(), selectionStart.deepEquivalent().containerNode());
else if (auto containerNode = makeRefPtr(selectionStart.deepEquivalent().containerNode()); containerNode && targetNode && &containerNode->treeScope() != &targetNode->treeScope())
result = VisibleSelection::adjustPositionForEnd(result.deepEquivalent(), containerNode.get());

range = makeSimpleRange(selectionStart, result);
} else {
if (selectionEnd <= result)
result = selectionEnd.previous();
else if (targetNode && targetNode->isInTreeScope() && selectionEnd.deepEquivalent().isInTreeScope() && selectionEnd.deepEquivalent().treeScope() != &targetNode->treeScope())
result = VisibleSelection::adjustPositionForStart(result.deepEquivalent(), selectionEnd.deepEquivalent().containerNode());
else if (auto containerNode = makeRefPtr(selectionEnd.deepEquivalent().containerNode()); containerNode && targetNode && &containerNode->treeScope() != &targetNode->treeScope())
result = VisibleSelection::adjustPositionForStart(result.deepEquivalent(), containerNode.get());

range = makeSimpleRange(result, selectionEnd);
}

0 comments on commit fabc066

Please sign in to comment.