CRASH at WebKit: WebKit::WebFullScreenManagerProxy::saveScrollPosition

• Component: New Bugs
• Status: RESOLVED FIXED

[Jer Noble]

CRASH at WebKit: WebKit::WebFullScreenManagerProxy::saveScrollPosition

[Jer Noble]

<rdar://problem/42160666>

[Jer Noble]

Created attachment 345257 [details]
Patch

[Jon Lee]

Comment on attachment 345257 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=345257&action=review

> Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm:495
> +    auto* page = self._webView._page;

Can we clean up the self._webView and webView references?

> Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm:720
> +    if (auto* page = [self._webView _page])

not webView?

[Jer Noble]

Committed r233940: <https://trac.webkit.org/changeset/233940>

[Chris Dumez]

Comment on attachment 345257 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=345257&action=review

> Source/WebKit/ChangeLog:9
> +        Null-check all uses of _page and _manager in WKFullScreenWindowControllerIOS.

How can WKWebView._page become null? As far as I can tell, _page is initialized in the init function and never null out so I personally do not see how WKWebView._page can be null if the webView is still alive.
Also note that _page is a WebPageProxy and that it OWNS the WebFullScreenManagerProxy, so no page no WebFullScreenManagerProxy.

See rdar://problem/42462494 for related crash.

[Chris Dumez]

Comment on attachment 345257 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=345257&action=review

>> Source/WebKit/ChangeLog:9
>> +        Null-check all uses of _page and _manager in WKFullScreenWindowControllerIOS.
> 
> How can WKWebView._page become null? As far as I can tell, _page is initialized in the init function and never null out so I personally do not see how WKWebView._page can be null if the webView is still alive.
> Also note that _page is a WebPageProxy and that it OWNS the WebFullScreenManagerProxy, so no page no WebFullScreenManagerProxy.
> 
> See rdar://problem/42462494 for related crash.

WKWebView._page definitely cannot be null since WKWebView's dealloc function dereferences _page unconditionally to call close().
However, WKFullScreenWindowControllerIOS._webView can be null as it is weak and its gets nulled out in some cases. I think we should null check _webView, not _webView._page.