187769 2018-07-18 10:49:50 -0700 CRASH at WebKit: WebKit::WebFullScreenManagerProxy::saveScrollPosition 2018-07-23 15:18:43 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 jer.noble jer.noble cdumez ddkilzer eric.carlson jonlee thorton webkit-bug-importer oldest_to_newest 1443047 0 jer.noble 2018-07-18 10:49:50 -0700 CRASH at WebKit: WebKit::WebFullScreenManagerProxy::saveScrollPosition 1443048 1 jer.noble 2018-07-18 10:50:13 -0700 <rdar://problem/42160666> 1443049 2 345257 jer.noble 2018-07-18 10:52:27 -0700 Created attachment 345257 Patch 1443141 3 345257 jonlee 2018-07-18 13:51:55 -0700 Comment on attachment 345257 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? 1443225 4 jer.noble 2018-07-18 16:30:13 -0700 Committed r233940: <https://trac.webkit.org/changeset/233940> 1444410 5 345257 cdumez 2018-07-23 14:19:26 -0700 Comment on attachment 345257 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. 1444440 6 345257 cdumez 2018-07-23 15:18:43 -0700 Comment on attachment 345257 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. 345257 2018-07-18 10:52:27 -0700 2018-07-18 15:56:26 -0700 Patch bug-187769-20180718105226.patch text/plain 6248 jer.noble U3VidmVyc2lvbiBSZXZpc2lvbjogMjMzODg3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDFkYWVmYzkwZmViYmVjODRm OTEwNWE5OTU0MzA3MmViMmVmNzE3OWQuLjQzN2Y0NjI3ZDc5YmM5NmQxNTgyNWRjOGRkN2RlNDEy ZWYxNmRiZTIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjAgQEAKKzIwMTgtMDctMTggIEplciBOb2Js ZSAgPGplci5ub2JsZUBhcHBsZS5jb20+CisKKyAgICAgICAgQ1JBU0ggYXQgV2ViS2l0OiBXZWJL aXQ6OldlYkZ1bGxTY3JlZW5NYW5hZ2VyUHJveHk6OnNhdmVTY3JvbGxQb3NpdGlvbgorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTg3NzY5CisgICAgICAg IDxyZGFyOi8vcHJvYmxlbS80MjE2MDY2Nj4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBOdWxsLWNoZWNrIGFsbCB1c2VzIG9mIF9wYWdlIGFuZCBfbWFu YWdlciBpbiBXS0Z1bGxTY3JlZW5XaW5kb3dDb250cm9sbGVySU9TLgorCisgICAgICAgICogVUlQ cm9jZXNzL2lvcy9mdWxsc2NyZWVuL1dLRnVsbFNjcmVlbldpbmRvd0NvbnRyb2xsZXJJT1MubW06 CisgICAgICAgIChXZWJLaXQ6OldLV2ViVmlld1N0YXRlOjphcHBseVRvKToKKyAgICAgICAgKFdl YktpdDo6V0tXZWJWaWV3U3RhdGU6OnN0b3JlKToKKyAgICAgICAgKC1bV0tGdWxsU2NyZWVuV2lu ZG93Q29udHJvbGxlciBlbnRlckZ1bGxTY3JlZW5dKToKKyAgICAgICAgKC1bV0tGdWxsU2NyZWVu V2luZG93Q29udHJvbGxlciBiZWdhbkV4aXRGdWxsU2NyZWVuV2l0aEluaXRpYWxGcmFtZTpmaW5h bEZyYW1lOl0pOgorICAgICAgICAoLVtXS0Z1bGxTY3JlZW5XaW5kb3dDb250cm9sbGVyIF9jb21w bGV0ZWRFeGl0RnVsbFNjcmVlbl0pOgorCiAyMDE4LTA3LTEyICBKZXIgTm9ibGUgIDxqZXIubm9i bGVAYXBwbGUuY29tPgogCiAgICAgICAgIFBpUCBmcm9tIEVsZW1lbnQgRnVsbHNjcmVlbiBzaG91 bGQgbWF0Y2ggQVZLaXQncyBiZWhhdmlvcgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9VSVBy b2Nlc3MvaW9zL2Z1bGxzY3JlZW4vV0tGdWxsU2NyZWVuV2luZG93Q29udHJvbGxlcklPUy5tbSBi L1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL2lvcy9mdWxsc2NyZWVuL1dLRnVsbFNjcmVlbldpbmRv d0NvbnRyb2xsZXJJT1MubW0KaW5kZXggNTFkMGE1YWE4ZGUxOWY1OTVkNWUwZGE3YmExMWQxYmQw YzBkZTAyZS4uODhiZTQxNGIyZmI1YjBiN2IyNGY5M2U1M2VjMmU5NTk4YTU4YWExNSAxMDA2NDQK LS0tIGEvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvaW9zL2Z1bGxzY3JlZW4vV0tGdWxsU2NyZWVu V2luZG93Q29udHJvbGxlcklPUy5tbQorKysgYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9pb3Mv ZnVsbHNjcmVlbi9XS0Z1bGxTY3JlZW5XaW5kb3dDb250cm9sbGVySU9TLm1tCkBAIC0xMTUsOCAr MTE1LDEwIEBAIHN0cnVjdCBXS1dlYlZpZXdTdGF0ZSB7CiAgICAgICAgIFtbd2ViVmlldyBzY3Jv bGxWaWV3XSBzZXRDb250ZW50SW5zZXQ6X3NhdmVkRWRnZUluc2V0XTsKICAgICAgICAgW1t3ZWJW aWV3IHNjcm9sbFZpZXddIHNldENvbnRlbnRPZmZzZXQ6X3NhdmVkQ29udGVudE9mZnNldF07CiAg ICAgICAgIFtbd2ViVmlldyBzY3JvbGxWaWV3XSBzZXRTY3JvbGxJbmRpY2F0b3JJbnNldHM6X3Nh dmVkU2Nyb2xsSW5kaWNhdG9ySW5zZXRzXTsKLSAgICAgICAgW3dlYlZpZXcgX3BhZ2VdLT5zZXRU b3BDb250ZW50SW5zZXQoX3NhdmVkVG9wQ29udGVudEluc2V0KTsKLSAgICAgICAgW3dlYlZpZXcg X3BhZ2VdLT5zZXRGb3JjZUFsd2F5c1VzZXJTY2FsYWJsZShfc2F2ZWRGb3JjZUFsd2F5c1VzZXJT Y2FsYWJsZSk7CisgICAgICAgIGlmIChhdXRvKiBwYWdlID0gd2ViVmlldy5fcGFnZSkgeworICAg ICAgICAgICAgcGFnZS0+c2V0VG9wQ29udGVudEluc2V0KF9zYXZlZFRvcENvbnRlbnRJbnNldCk7 CisgICAgICAgICAgICBwYWdlLT5zZXRGb3JjZUFsd2F5c1VzZXJTY2FsYWJsZShfc2F2ZWRGb3Jj ZUFsd2F5c1VzZXJTY2FsYWJsZSk7CisgICAgICAgIH0KICAgICAgICAgW3dlYlZpZXcgX3NldFZp ZXdTY2FsZTpfc2F2ZWRWaWV3U2NhbGVdOwogICAgICAgICBbW3dlYlZpZXcgc2Nyb2xsVmlld10g c2V0Wm9vbVNjYWxlOl9zYXZlZFpvb21TY2FsZV07CiAgICAgICAgIHdlYlZpZXcuc2Nyb2xsVmll dy5taW5pbXVtWm9vbVNjYWxlID0gX3NhdmVkTWluaW11bVpvb21TY2FsZTsKQEAgLTEzMSw4ICsx MzMsMTAgQEAgc3RydWN0IFdLV2ViVmlld1N0YXRlIHsKICAgICAgICAgX3NhdmVkRWRnZUluc2V0 ID0gW1t3ZWJWaWV3IHNjcm9sbFZpZXddIGNvbnRlbnRJbnNldF07CiAgICAgICAgIF9zYXZlZENv bnRlbnRPZmZzZXQgPSBbW3dlYlZpZXcgc2Nyb2xsVmlld10gY29udGVudE9mZnNldF07CiAgICAg ICAgIF9zYXZlZFNjcm9sbEluZGljYXRvckluc2V0cyA9IFtbd2ViVmlldyBzY3JvbGxWaWV3XSBz Y3JvbGxJbmRpY2F0b3JJbnNldHNdOwotICAgICAgICBfc2F2ZWRUb3BDb250ZW50SW5zZXQgPSBb d2ViVmlldyBfcGFnZV0tPnRvcENvbnRlbnRJbnNldCgpOwotICAgICAgICBfc2F2ZWRGb3JjZUFs d2F5c1VzZXJTY2FsYWJsZSA9IFt3ZWJWaWV3IF9wYWdlXS0+Zm9yY2VBbHdheXNVc2VyU2NhbGFi bGUoKTsKKyAgICAgICAgaWYgKGF1dG8qIHBhZ2UgPSB3ZWJWaWV3Ll9wYWdlKSB7CisgICAgICAg ICAgICBfc2F2ZWRUb3BDb250ZW50SW5zZXQgPSBwYWdlLT50b3BDb250ZW50SW5zZXQoKTsKKyAg ICAgICAgICAgIF9zYXZlZEZvcmNlQWx3YXlzVXNlclNjYWxhYmxlID0gcGFnZS0+Zm9yY2VBbHdh eXNVc2VyU2NhbGFibGUoKTsKKyAgICAgICAgfQogICAgICAgICBfc2F2ZWRWaWV3U2NhbGUgPSBb d2ViVmlldyBfdmlld1NjYWxlXTsKICAgICAgICAgX3NhdmVkWm9vbVNjYWxlID0gW1t3ZWJWaWV3 IHNjcm9sbFZpZXddIHpvb21TY2FsZV07CiAgICAgICAgIF9zYXZlZE1pbmltdW1ab29tU2NhbGUg PSB3ZWJWaWV3LnNjcm9sbFZpZXcubWluaW11bVpvb21TY2FsZTsKQEAgLTQ4OCw2ICs0OTIsMTEg QEAgLSAodm9pZCllbnRlckZ1bGxTY3JlZW4KICAgICBpZiAoW3NlbGYgaXNGdWxsU2NyZWVuXSkK ICAgICAgICAgcmV0dXJuOwogCisgICAgYXV0byogcGFnZSA9IHNlbGYuX3dlYlZpZXcuX3BhZ2U7 CisgICAgYXV0byogbWFuYWdlciA9IHNlbGYuX21hbmFnZXI7CisgICAgaWYgKCFwYWdlIHx8ICFt YW5hZ2VyKQorICAgICAgICByZXR1cm47CisKICAgICBbc2VsZiBfaW52YWxpZGF0ZUVWT3JnYW5p emF0aW9uTmFtZV07CiAKICAgICBfZnVsbFNjcmVlblN0YXRlID0gV2FpdGluZ1RvRW50ZXJGdWxs U2NyZWVuOwpAQCAtNTM0LDkgKzU0Myw5IEBAIC0gKHZvaWQpZW50ZXJGdWxsU2NyZWVuCiAgICAg W19pbnRlcmFjdGl2ZVBpbmNoRGlzbWlzc0dlc3R1cmVSZWNvZ25pemVyIHNldENhbmNlbHNUb3Vj aGVzSW5WaWV3Ok5PXTsKICAgICBbX2Z1bGxzY3JlZW5WaWV3Q29udHJvbGxlci5nZXQoKS52aWV3 IGFkZEdlc3R1cmVSZWNvZ25pemVyOl9pbnRlcmFjdGl2ZVBpbmNoRGlzbWlzc0dlc3R1cmVSZWNv Z25pemVyLmdldCgpXTsKIAotICAgIFtzZWxmIF9tYW5hZ2VyXS0+c2F2ZVNjcm9sbFBvc2l0aW9u KCk7CisgICAgbWFuYWdlci0+c2F2ZVNjcm9sbFBvc2l0aW9uKCk7CiAKLSAgICBbd2ViVmlldyBf cGFnZV0tPnNldFN1cHByZXNzVmlzaWJpbGl0eVVwZGF0ZXModHJ1ZSk7CisgICAgcGFnZS0+c2V0 U3VwcHJlc3NWaXNpYmlsaXR5VXBkYXRlcyh0cnVlKTsKIAogICAgIF92aWV3U3RhdGUuc3RvcmUo d2ViVmlldy5nZXQoKSk7CiAKQEAgLTU0Nyw3ICs1NTYsOCBAQCAtICh2b2lkKWVudGVyRnVsbFNj cmVlbgogICAgIFdLU25hcHNob3RDb25maWd1cmF0aW9uKiBjb25maWcgPSBuaWw7CiAgICAgW3dl YlZpZXcgdGFrZVNuYXBzaG90V2l0aENvbmZpZ3VyYXRpb246Y29uZmlnIGNvbXBsZXRpb25IYW5k bGVyOl4oVUlJbWFnZSAqIHNuYXBzaG90SW1hZ2UsIE5TRXJyb3IgKiBlcnJvcikgewogICAgICAg ICBSZXRhaW5QdHI8V0tXZWJWaWV3PiB3ZWJWaWV3ID0gc2VsZi5fd2ViVmlldzsKLSAgICAgICAg aWYgKCFbd2ViVmlldyBfcGFnZV0pCisgICAgICAgIGF1dG8qIHBhZ2UgPSBbc2VsZi5fd2ViVmll dyBfcGFnZV07CisgICAgICAgIGlmICghcGFnZSkKICAgICAgICAgICAgIHJldHVybjsKIAogICAg ICAgICBbQ0FUcmFuc2FjdGlvbiBiZWdpbl07CkBAIC01NjUsMTQgKzU3NSwxNSBAQCAtICh2b2lk KWVudGVyRnVsbFNjcmVlbgogICAgICAgICBbd2ViVmlldyBzZXROZWVkc0xheW91dF07CiAgICAg ICAgIFt3ZWJWaWV3IGxheW91dElmTmVlZGVkXTsKICAgICAgICAgCi0gICAgICAgIFtzZWxmIF9t YW5hZ2VyXS0+c2V0QW5pbWF0aW5nRnVsbFNjcmVlbih0cnVlKTsKKyAgICAgICAgaWYgKGF1dG8q IG1hbmFnZXIgPSBzZWxmLl9tYW5hZ2VyKQorICAgICAgICAgICAgbWFuYWdlci0+c2V0QW5pbWF0 aW5nRnVsbFNjcmVlbih0cnVlKTsKIAogICAgICAgICBWaWV3cG9ydEFyZ3VtZW50cyBhcmd1bWVu dHMgeyBWaWV3cG9ydEFyZ3VtZW50czo6Q1NTRGV2aWNlQWRhcHRhdGlvbiB9OwogICAgICAgICBh cmd1bWVudHMuem9vbSA9IDE7CiAgICAgICAgIGFyZ3VtZW50cy5taW5ab29tID0gMTsKICAgICAg ICAgYXJndW1lbnRzLm1heFpvb20gPSAxOwogICAgICAgICBhcmd1bWVudHMudXNlclpvb20gPSAx OwotICAgICAgICBbd2ViVmlldyBfcGFnZV0tPnNldE92ZXJyaWRlVmlld3BvcnRBcmd1bWVudHMo YXJndW1lbnRzKTsKKyAgICAgICAgcGFnZS0+c2V0T3ZlcnJpZGVWaWV3cG9ydEFyZ3VtZW50cyhh cmd1bWVudHMpOwogCiAgICAgICAgIF9yZXBhaW50Q2FsbGJhY2sgPSBWb2lkQ2FsbGJhY2s6OmNy ZWF0ZShbcHJvdGVjdGVkU2VsZiA9IHJldGFpblB0cihzZWxmKSwgc2VsZl0oV2ViS2l0OjpDYWxs YmFja0Jhc2U6OkVycm9yKSB7CiAgICAgICAgICAgICBfcmVwYWludENhbGxiYWNrID0gbnVsbHB0 cjsKQEAgLTU4NCw3ICs1OTUsNyBAQCAtICh2b2lkKWVudGVyRnVsbFNjcmVlbgogICAgICAgICAg ICAgQVNTRVJUX05PVF9SRUFDSEVEKCk7CiAgICAgICAgICAgICBbc2VsZiBfZXhpdEZ1bGxzY3Jl ZW5JbW1lZGlhdGVseV07CiAgICAgICAgIH0pOwotICAgICAgICBbd2ViVmlldyBfcGFnZV0tPmZv cmNlUmVwYWludChfcmVwYWludENhbGxiYWNrLmNvcHlSZWYoKSk7CisgICAgICAgIHBhZ2UtPmZv cmNlUmVwYWludChfcmVwYWludENhbGxiYWNrLmNvcHlSZWYoKSk7CiAKICAgICAgICAgW0NBVHJh bnNhY3Rpb24gY29tbWl0XTsKICAgICB9XTsKQEAgLTY3NCw3ICs2ODUsOCBAQCAtICh2b2lkKWJl Z2FuRXhpdEZ1bGxTY3JlZW5XaXRoSW5pdGlhbEZyYW1lOihDR1JlY3QpaW5pdGlhbEZyYW1lIGZp bmFsRnJhbWU6KENHUgogICAgIF9maW5hbEZyYW1lLnNpemUgPSBzaXplRXhwYW5kZWRUb1NpemUo X2ZpbmFsRnJhbWUuc2l6ZSwgQ0dTaXplTWFrZSgxLCAxKSk7CiAgICAgX2ZpbmFsRnJhbWUgPSBz YWZlSW5saW5lUmVjdChfZmluYWxGcmFtZSwgW19yb290Vmlld0NvbnRyb2xsZXIgdmlld10uZnJh bWUuc2l6ZSk7CiAKLSAgICBbc2VsZi5fd2ViVmlldyBfcGFnZV0tPnNldFN1cHByZXNzVmlzaWJp bGl0eVVwZGF0ZXModHJ1ZSk7CisgICAgaWYgKGF1dG8qIHBhZ2UgPSBbc2VsZi5fd2ViVmlldyBf cGFnZV0pCisgICAgICAgIHBhZ2UtPnNldFN1cHByZXNzVmlzaWJpbGl0eVVwZGF0ZXModHJ1ZSk7 CiAKICAgICBbX2Z1bGxzY3JlZW5WaWV3Q29udHJvbGxlciBzZXRQcmVmZXJzU3RhdHVzQmFySGlk ZGVuOk5PXTsKIApAQCAtNzA1LDcgKzcxNyw4IEBAIC0gKHZvaWQpX2NvbXBsZXRlZEV4aXRGdWxs U2NyZWVuCiAgICAgW3dlYlZpZXcgYmVjb21lRmlyc3RSZXNwb25kZXJdOwogCiAgICAgX3ZpZXdT dGF0ZS5hcHBseVRvKHdlYlZpZXcuZ2V0KCkpOwotICAgIFt3ZWJWaWV3IF9wYWdlXS0+c2V0T3Zl cnJpZGVWaWV3cG9ydEFyZ3VtZW50cyhzdGQ6Om51bGxvcHQpOworICAgIGlmIChhdXRvKiBwYWdl ID0gW3NlbGYuX3dlYlZpZXcgX3BhZ2VdKQorICAgICAgICBwYWdlLT5zZXRPdmVycmlkZVZpZXdw b3J0QXJndW1lbnRzKHN0ZDo6bnVsbG9wdCk7CiAKICAgICBbd2ViVmlldyBzZXROZWVkc0xheW91 dF07CiAgICAgW3dlYlZpZXcgbGF5b3V0SWZOZWVkZWRdOwo=