Changeset 219852 in webkit

Timestamp:

Jul 24, 2017 6:17:51 PM (7 years ago)

Author:

Simon Fraser

Message:

[iOS WK2] Crash under PageOverlayController::uninstallPageOverlay when doing multiple finds
​https://bugs.webkit.org/show_bug.cgi?id=174806
rdar://problem/33501664

Reviewed by Tim Horton.

Calling TextIndicator::createWithSelectionInFrame can trigger layout via VisibleSelection code,
which can re-enter FindController::updateFindIndicator which has by now removed m_findIndicatorOverlay
from pageOverlayController's map, but not deleted it. The second call to uninstallPageOverlay()
thus crashes at m_overlayGraphicsLayers.take(&overlay)->removeFromParent().

Fix by nulling out m_findIndicatorOverlay as soon as we uninstall it.

• WebProcess/WebPage/ios/FindControllerIOS.mm:

(WebKit::FindController::updateFindIndicator):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• WebProcess/WebPage/ios/FindControllerIOS.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2017-07-24  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] Crash under PageOverlayController::uninstallPageOverlay when doing multiple finds
+        https://bugs.webkit.org/show_bug.cgi?id=174806
+        rdar://problem/33501664
+
+        Reviewed by Tim Horton.
+        
+        Calling TextIndicator::createWithSelectionInFrame can trigger layout via VisibleSelection code,
+        which can re-enter FindController::updateFindIndicator which has by now removed m_findIndicatorOverlay
+        from pageOverlayController's map, but not deleted it. The second call to uninstallPageOverlay()
+        thus crashes at m_overlayGraphicsLayers.take(&overlay)->removeFromParent().
+        
+        Fix by nulling out m_findIndicatorOverlay as soon as we uninstall it.
+
+        * WebProcess/WebPage/ios/FindControllerIOS.mm:
+        (WebKit::FindController::updateFindIndicator):
+
 2017-07-24  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm

@@ -90 +90 @@
 bool FindController::updateFindIndicator(Frame& selectedFrame, bool isShowingOverlay, bool shouldAnimate)
 {
-    if (m_findIndicatorOverlay)
+    if (m_findIndicatorOverlay) {
         m_webPage->mainFrame()->pageOverlayController().uninstallPageOverlay(*m_findIndicatorOverlay, PageOverlay::FadeMode::DoNotFade);
+        m_findIndicatorOverlay = nullptr;
+        m_isShowingFindIndicator = false;
+    }
 
     RefPtr<TextIndicator> textIndicator = TextIndicator::createWithSelectionInFrame(selectedFrame, findTextIndicatorOptions, TextIndicatorPresentationTransition::None, FloatSize(totalHorizontalMargin, totalVerticalMargin));
-    if (!textIndicator) {
-        m_findIndicatorOverlay = nullptr;
-        m_isShowingFindIndicator = false;
+    if (!textIndicator)
         return false;
-    }
 
     m_findIndicatorOverlayClient = std::make_unique<FindIndicatorOverlayClientIOS>(selectedFrame, textIndicator.get());