Skip to content

Commit c40691a

Browse files
committed
[iOS] DOM nodes can be leaked when searching for text in Safari
https://bugs.webkit.org/show_bug.cgi?id=285450 rdar://133689631 Reviewed by Abrar Rahman Protyasha and Richard Robinson. The UIKit API for find-in-page on iOS expects clients to vend all found ranges to the system. The system then tells the client to highlight / scroll to the appropriate range. Since converting between the range representation for UIKit and `SimpleRange`s can be slow, a cache of found ranges is maintained. Under most API use (via the system search UI), the cache is populated when the user starts searching and is cleared when the user dismisses the find bar. However, when searches are performed using Safari's URL bar, the find APIs may be called directly, without the full system user flow that clears caches. Since the cache preserves `SimpleRange`s, it strongly holds on to DOM nodes. Fix by updating the cache to store `WeakSimpleRange`s. Additionally, clear the cache on memory pressure. * Source/WebKit/WebProcess/WebPage/WebFoundTextRangeController.cpp: (WebKit::WebFoundTextRangeController::findTextRangesForStringMatches): (WebKit::WebFoundTextRangeController::clearAllDecoratedFoundText): (WebKit::WebFoundTextRangeController::clearCachedRanges): (WebKit::WebFoundTextRangeController::simpleRangeFromFoundTextRange): * Source/WebKit/WebProcess/WebPage/WebFoundTextRangeController.h: * Source/WebKit/WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::releaseMemory): Canonical link: https://commits.webkit.org/288505@main
1 parent d9f35fa commit c40691a

File tree

3 files changed

+20
-5
lines changed

3 files changed

+20
-5
lines changed

Diff for: Source/WebKit/WebProcess/WebPage/WebFoundTextRangeController.cpp

+15-4
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ void WebFoundTextRangeController::findTextRangesForStringMatches(const String& s
9090
auto domData = WebFoundTextRange::DOMData { range.location, range.length };
9191
auto foundTextRange = WebFoundTextRange { domData, frameName.length() ? frameName : emptyAtom(), order };
9292

93-
m_cachedFoundRanges.add(foundTextRange, simpleRange);
93+
m_cachedFoundRanges.add(foundTextRange, simpleRange.makeWeakSimpleRange());
9494
foundTextRanges.append(foundTextRange);
9595
}
9696

@@ -223,7 +223,7 @@ void WebFoundTextRangeController::scrollTextRangeToVisible(const WebFoundTextRan
223223

224224
void WebFoundTextRangeController::clearAllDecoratedFoundText()
225225
{
226-
m_cachedFoundRanges.clear();
226+
clearCachedRanges();
227227
m_decoratedRanges.clear();
228228
m_webPage->corePage()->unmarkAllTextMatches();
229229

@@ -300,6 +300,11 @@ void WebFoundTextRangeController::redraw()
300300
);
301301
}
302302

303+
void WebFoundTextRangeController::clearCachedRanges()
304+
{
305+
m_cachedFoundRanges.clear();
306+
}
307+
303308
void WebFoundTextRangeController::willMoveToPage(WebCore::PageOverlay&, WebCore::Page* page)
304309
{
305310
if (page)
@@ -513,7 +518,7 @@ WebCore::Document* WebFoundTextRangeController::documentForFoundTextRange(const
513518

514519
std::optional<WebCore::SimpleRange> WebFoundTextRangeController::simpleRangeFromFoundTextRange(WebFoundTextRange range)
515520
{
516-
return m_cachedFoundRanges.ensure(range, [&] () -> std::optional<WebCore::SimpleRange> {
521+
auto foundRange = m_cachedFoundRanges.ensure(range, [&] -> std::optional<WebCore::WeakSimpleRange> {
517522
if (!std::holds_alternative<WebFoundTextRange::DOMData>(range.data))
518523
return std::nullopt;
519524

@@ -523,8 +528,14 @@ std::optional<WebCore::SimpleRange> WebFoundTextRangeController::simpleRangeFrom
523528

524529
Ref documentElement = *document->documentElement();
525530
auto domData = std::get<WebFoundTextRange::DOMData>(range.data);
526-
return resolveCharacterRange(makeRangeSelectingNodeContents(documentElement), { domData.location, domData.length }, WebCore::findIteratorOptions());
531+
auto simpleRange = resolveCharacterRange(makeRangeSelectingNodeContents(documentElement), { domData.location, domData.length }, WebCore::findIteratorOptions());
532+
return simpleRange.makeWeakSimpleRange();
527533
}).iterator->value;
534+
535+
if (!foundRange)
536+
return std::nullopt;
537+
538+
return makeSimpleRange(*foundRange);
528539
}
529540

530541
} // namespace WebKit

Diff for: Source/WebKit/WebProcess/WebPage/WebFoundTextRangeController.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ class WebFoundTextRangeController : private WebCore::PageOverlayClient {
7373

7474
void redraw();
7575

76+
void clearCachedRanges();
77+
7678
private:
7779
// PageOverlayClient.
7880
void willMoveToPage(WebCore::PageOverlay&, WebCore::Page*) override;
@@ -99,7 +101,7 @@ class WebFoundTextRangeController : private WebCore::PageOverlayClient {
99101

100102
WebFoundTextRange m_highlightedRange;
101103

102-
HashMap<WebFoundTextRange, std::optional<WebCore::SimpleRange>> m_cachedFoundRanges;
104+
HashMap<WebFoundTextRange, std::optional<WebCore::WeakSimpleRange>> m_cachedFoundRanges;
103105
HashMap<WebFoundTextRange, FindDecorationStyle> m_decoratedRanges;
104106

105107
RefPtr<WebCore::TextIndicator> m_textIndicator;

Diff for: Source/WebKit/WebProcess/WebPage/WebPage.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -5142,6 +5142,8 @@ void WebPage::releaseMemory(Critical)
51425142
if (m_remoteRenderingBackendProxy)
51435143
m_remoteRenderingBackendProxy->remoteResourceCacheProxy().releaseMemory();
51445144
#endif
5145+
5146+
m_foundTextRangeController->clearCachedRanges();
51455147
}
51465148

51475149
void WebPage::willDestroyDecodedDataForAllImages()

0 commit comments

Comments
 (0)