[Webkit-unassigned] [Bug 243264] New: Retain cycle after snapshotting a WKWebView displaying a PDF file

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=243264

            Bug ID: 243264
           Summary: Retain cycle after snapshotting a WKWebView displaying
                    a PDF file
           Product: WebKit
           Version: WebKit Nightly Build
          Hardware: Unspecified
                OS: Unspecified
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: PDF
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: ajuma at chromium.org
                CC: bdakin at apple.com, justincohen at google.com,
                    thorton at apple.com, wenson_hsieh at apple.com

Created attachment 461260

  --> https://bugs.webkit.org/attachment.cgi?id=461260&action=review

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.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220727/e7fcc6e2/attachment-0001.htm>