Title: [218068] trunk/Source
- Revision
- 218068
- Author
- [email protected]
- Date
- 2017-06-11 09:03:41 -0700 (Sun, 11 Jun 2017)
Log Message
REGRESSION (r217870): Null deref under PageOverlayController::uninstallPageOverlay using find in page
https://bugs.webkit.org/show_bug.cgi?id=173196
<rdar://problem/32686871>
Reviewed by Simon Fraser.
Source/WebCore:
* page/TextIndicator.cpp:
(WebCore::initializeIndicator):
* page/TextIndicator.h:
The change in r217870 had an expected but not thought-through side-effect
that you cannot create a TextIndicator for far-off-screen elements. This
is a problem for find-in-page, which uses TextIndicator to paint the yellow
highlight before scrolling it into view.
Bring back TextIndicatorOptionDoNotClipToVisibleRect and revert to it for
find in page, since it needs different behavior than something like
drag and drop, which always operates on content near the visible viewport.
Source/WebKit2:
* WebProcess/WebPage/ios/FindControllerIOS.mm:
(WebKit::FindController::updateFindIndicator):
The reason this caused a crash instead of just a missing indicator
is that FindControllerIOS would early return after uninstalling
the old overlay, before installing a new one, and leave m_isShowingFindIndicator
set to true. Instead, reset it (and m_findIndicatorOverlay, which
we would never re-use but might as well be freed immediately).
This likely already crashed in other less reproducible cases where a
TextIndicator was not created for a find match, so clean it up.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (218067 => 218068)
--- trunk/Source/WebCore/ChangeLog 2017-06-11 10:12:14 UTC (rev 218067)
+++ trunk/Source/WebCore/ChangeLog 2017-06-11 16:03:41 UTC (rev 218068)
@@ -1,3 +1,23 @@
+2017-06-11 Tim Horton <[email protected]>
+
+ REGRESSION (r217870): Null deref under PageOverlayController::uninstallPageOverlay using find in page
+ https://bugs.webkit.org/show_bug.cgi?id=173196
+ <rdar://problem/32686871>
+
+ Reviewed by Simon Fraser.
+
+ * page/TextIndicator.cpp:
+ (WebCore::initializeIndicator):
+ * page/TextIndicator.h:
+ The change in r217870 had an expected but not thought-through side-effect
+ that you cannot create a TextIndicator for far-off-screen elements. This
+ is a problem for find-in-page, which uses TextIndicator to paint the yellow
+ highlight before scrolling it into view.
+
+ Bring back TextIndicatorOptionDoNotClipToVisibleRect and revert to it for
+ find in page, since it needs different behavior than something like
+ drag and drop, which always operates on content near the visible viewport.
+
2017-06-10 Dan Bernstein <[email protected]>
Reverted r218056 because it made the IDE reindex constantly.
Modified: trunk/Source/WebCore/page/TextIndicator.cpp (218067 => 218068)
--- trunk/Source/WebCore/page/TextIndicator.cpp 2017-06-11 10:12:14 UTC (rev 218067)
+++ trunk/Source/WebCore/page/TextIndicator.cpp 2017-06-11 16:03:41 UTC (rev 218068)
@@ -301,7 +301,11 @@
Vector<FloatRect> clippedTextRectsInDocumentCoordinates;
Vector<FloatRect> textRectsInRootViewCoordinates;
for (const FloatRect& textRect : textRects) {
- FloatRect clippedTextRect = intersection(textRect, contentsClipRect);
+ FloatRect clippedTextRect;
+ if (data.options & TextIndicatorOptionDoNotClipToVisibleRect)
+ clippedTextRect = textRect;
+ else
+ clippedTextRect = intersection(textRect, contentsClipRect);
if (clippedTextRect.isEmpty())
continue;
Modified: trunk/Source/WebCore/page/TextIndicator.h (218067 => 218068)
--- trunk/Source/WebCore/page/TextIndicator.h 2017-06-11 10:12:14 UTC (rev 218067)
+++ trunk/Source/WebCore/page/TextIndicator.h 2017-06-11 16:03:41 UTC (rev 218068)
@@ -81,17 +81,21 @@
// If this option is set, expand the clip rect outward so that slightly offscreen content will be included.
TextIndicatorOptionExpandClipBeyondVisibleRect = 1 << 7,
+ // By default, TextIndicator clips the indicated rects to the visible content rect.
+ // If this option is set, do not clip to the visible rect.
+ TextIndicatorOptionDoNotClipToVisibleRect = 1 << 8,
+
// Include an additional snapshot of everything in view, with the exception of nodes within the currently selected range.
- TextIndicatorOptionIncludeSnapshotOfAllVisibleContentWithoutSelection = 1 << 8,
+ TextIndicatorOptionIncludeSnapshotOfAllVisibleContentWithoutSelection = 1 << 9,
// By default, TextIndicator uses text rects to size the snapshot. Enabling this flag causes it to use the bounds of the
// selection rects that would enclose the given Range instead.
// Currently, this is only supported on iOS.
- TextIndicatorOptionUseSelectionRectForSizing = 1 << 9,
+ TextIndicatorOptionUseSelectionRectForSizing = 1 << 10,
// Compute a background color to use when rendering a platter around the content image, falling back to a default if the
// content's background is too complex to be captured by a single color.
- TextIndicatorOptionComputeEstimatedBackgroundColor = 1 << 10,
+ TextIndicatorOptionComputeEstimatedBackgroundColor = 1 << 11,
};
typedef uint16_t TextIndicatorOptions;
Modified: trunk/Source/WebKit2/ChangeLog (218067 => 218068)
--- trunk/Source/WebKit2/ChangeLog 2017-06-11 10:12:14 UTC (rev 218067)
+++ trunk/Source/WebKit2/ChangeLog 2017-06-11 16:03:41 UTC (rev 218068)
@@ -1,3 +1,22 @@
+2017-06-11 Tim Horton <[email protected]>
+
+ REGRESSION (r217870): Null deref under PageOverlayController::uninstallPageOverlay using find in page
+ https://bugs.webkit.org/show_bug.cgi?id=173196
+ <rdar://problem/32686871>
+
+ Reviewed by Simon Fraser.
+
+ * WebProcess/WebPage/ios/FindControllerIOS.mm:
+ (WebKit::FindController::updateFindIndicator):
+ The reason this caused a crash instead of just a missing indicator
+ is that FindControllerIOS would early return after uninstalling
+ the old overlay, before installing a new one, and leave m_isShowingFindIndicator
+ set to true. Instead, reset it (and m_findIndicatorOverlay, which
+ we would never re-use but might as well be freed immediately).
+
+ This likely already crashed in other less reproducible cases where a
+ TextIndicator was not created for a find match, so clean it up.
+
2017-06-10 Carlos Garcia Campos <[email protected]>
[GTK] Remove WKFullScreenClientGtk
Modified: trunk/Source/WebKit2/WebProcess/WebPage/ios/FindControllerIOS.mm (218067 => 218068)
--- trunk/Source/WebKit2/WebProcess/WebPage/ios/FindControllerIOS.mm 2017-06-11 10:12:14 UTC (rev 218067)
+++ trunk/Source/WebKit2/WebProcess/WebPage/ios/FindControllerIOS.mm 2017-06-11 16:03:41 UTC (rev 218068)
@@ -50,7 +50,7 @@
const int totalHorizontalMargin = 2;
const int totalVerticalMargin = 1;
-const TextIndicatorOptions findTextIndicatorOptions = TextIndicatorOptionTightlyFitContent | TextIndicatorOptionIncludeMarginIfRangeMatchesSelection | TextIndicatorOptionExpandClipBeyondVisibleRect;
+const TextIndicatorOptions findTextIndicatorOptions = TextIndicatorOptionTightlyFitContent | TextIndicatorOptionIncludeMarginIfRangeMatchesSelection | TextIndicatorOptionDoNotClipToVisibleRect;
static Color highlightColor()
{
@@ -93,8 +93,11 @@
m_webPage->mainFrame()->pageOverlayController().uninstallPageOverlay(*m_findIndicatorOverlay, PageOverlay::FadeMode::DoNotFade);
RefPtr<TextIndicator> textIndicator = TextIndicator::createWithSelectionInFrame(selectedFrame, findTextIndicatorOptions, TextIndicatorPresentationTransition::None, FloatSize(totalHorizontalMargin, totalVerticalMargin));
- if (!textIndicator)
+ if (!textIndicator) {
+ m_findIndicatorOverlay = nullptr;
+ m_isShowingFindIndicator = false;
return false;
+ }
m_findIndicatorOverlayClient = std::make_unique<FindIndicatorOverlayClientIOS>(selectedFrame, textIndicator.get());
m_findIndicatorOverlay = PageOverlay::create(*m_findIndicatorOverlayClient, PageOverlay::OverlayType::Document);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
