Bug 243264 – Retain cycle after snapshotting a WKWebView displaying a PDF file

• Component: PDF
• Status: RESOLVED FIXED

[Ali Juma]

Created attachment 461260 [details]
ViewController reproducing the leak

When a third-party embedder (that is, any embedder without the com.apple.QuartzCore.global-capture entitlement) calls -[WKWebView takeSnapshotWithConfiguration:completionHandler:], this creates a retain cycle that isn't broken until the WKWebView is navigated away. If, instead, the WKWebView is released by the embedder in this state, the cycle remains forever, and the WKWebView is leaked.

This seems to be a combination of two bugs:
1) In -[WKWebView takeSnapshotWithConfiguration:completionHandler:], the completion handler constructed on this line: https://github.com/WebKit/WebKit/blob/a790b3beee524f688f96f721372dee80a6235e7f/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm#L1247 captures strongSelf without seeming to need it (it's not used in the body of that completion handler).
2) For third-party embedders (for whom +[WKPDFView web_requiresCustomSnapshotting] is YES), the callback from (1) ends up getting passed to PDFHostViewController in -[WKPDFView web_snapshotRectInContentViewCoordinates], and for reasons that aren't clear (hard to tell without source for PDFKit), the callback isn't released even after it is called.

The result is we have a cycle:
1) WKWebView retains WKPDFView (through _customContentView)
2) WKPDFView retains PDFHostViewController (through _hostViewController)
3) PDFHostViewController retains the callback
4) The callback retains WKWebView (because of the strongSelf mentioned earlier)

I've confirmed using Xcode's memory graph that the WKWebView remains alive even if the embedder releases it and removes it from the view hierarchy.

In addition to leaking memory, this is also web-observable if another page has an opener to the leaked WKWebView, since window.closed remains false even though the tab has actually been closed. This reproduces in multiple 3rd-party browsers on iOS (Chrome, Firefox, Brave, Edge).

The simplest fix would be to change https://github.com/WebKit/WebKit/blob/a790b3beee524f688f96f721372dee80a6235e7f/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm#L1247 so that strongSelf isn't captured in the completionHandler constructed at the end of that line.

An alternative would be to figure out why PDFHostViewController isn't releasing the callback, but that's outside the scope of WebKit.

I've attached a simple example of constructing a WKWebView and taking a snapshot, which includes a comment at the point where we can look a the memory graph to see the leak.

[Ali Juma]

Pull request: https://github.com/WebKit/WebKit/pull/2859

[EWS]

Committed 252966@main (b16d7efbd2b6): <https://commits.webkit.org/252966@main>

Reviewed commits have been landed. Closing PR #2859 and removing active labels.

[Radar WebKit Bug Importer]

<rdar://problem/97805012>

[Ali Juma]

I've also filed FB11080384 for having this fix cherry-picked to a release branch.