135022 2014-07-17 13:50:40 -0700 Crash in ServicesOverlayController::~ServicesOverlayController 2014-07-17 14:05:39 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Mac All RESOLVED FIXED InRadar P2 Normal --- 1 beidson beidson thorton oldest_to_newest 1023084 0 beidson 2014-07-17 13:50:40 -0700 Crash in ServicesOverlayController::~ServicesOverlayController No need to uninstall the PageOverlay, because in WebPage teardown the PageOverlay has already been destroyed. <rdar://problem/17622172> 1023085 1 235090 beidson 2014-07-17 13:52:32 -0700 Created attachment 235090 Patch v1 1023087 2 235090 thorton 2014-07-17 13:54:32 -0700 Comment on attachment 235090 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=235090&action=review > Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm:-86 > - m_webPage->uninstallPageOverlay(m_servicesOverlay, PageOverlay::FadeMode::DoNotFade); you should assert that your back pointer was nulled out (and let it be nulled out! fix the early return in willMoveToWebPage). 1023090 3 thorton 2014-07-17 14:00:16 -0700 (In reply to comment #2) > (From update of attachment 235090 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=235090&action=review > > > Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm:-86 > > - m_webPage->uninstallPageOverlay(m_servicesOverlay, PageOverlay::FadeMode::DoNotFade); > > you should assert that your back pointer was nulled out (and let it be nulled out! fix the early return in willMoveToWebPage). Nevermind! 1023092 4 beidson 2014-07-17 14:03:40 -0700 (In reply to comment #2) > (From update of attachment 235090 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=235090&action=review > > > Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm:-86 > > - m_webPage->uninstallPageOverlay(m_servicesOverlay, PageOverlay::FadeMode::DoNotFade); > > you should assert that your back pointer was nulled out (and let it be nulled out! fix the early return in willMoveToWebPage). Clarified on IRC - Tim misread the early return - It's correct. But he also was assuming that willMoveToWebPage was called with a null WebPage on WebPage teardown. It's not. Filed https://bugs.webkit.org/show_bug.cgi?id=135024 for that 1023093 5 beidson 2014-07-17 14:05:39 -0700 http://trac.webkit.org/changeset/171194 235090 2014-07-17 13:52:32 -0700 2014-07-17 13:54:31 -0700 Patch v1 patch text/plain 1500 beidson ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCA4MzAxMWE4Li45MmU5MWMzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQg QEAKKzIwMTQtMDctMTcgIEJyYWR5IEVpZHNvbiAgPGJlaWRzb25AYXBwbGUuY29tPgorCisgICAg ICAgIENyYXNoIGluIFNlcnZpY2VzT3ZlcmxheUNvbnRyb2xsZXI6On5TZXJ2aWNlc092ZXJsYXlD b250cm9sbGVyLgorICAgICAgICA8cmRhcjovL3Byb2JsZW0vMTc2MjIxNzI+IGFuZCBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTM1MDIyCisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBXZWJQcm9jZXNzL1dlYlBhZ2UvbWFj L1NlcnZpY2VzT3ZlcmxheUNvbnRyb2xsZXIubW06CisgICAgICAgIChXZWJLaXQ6OlNlcnZpY2Vz T3ZlcmxheUNvbnRyb2xsZXI6On5TZXJ2aWNlc092ZXJsYXlDb250cm9sbGVyKTogRG9u4oCZdCBu ZWVkIHRvIHVuaW5zdGFsbCB0aGUKKyAgICAgICAgICAgIFBhZ2VPdmVybGF5IGFzIGl0IGhhcyBh bHJlYWR5IGJlZW4gZGVzdHJveWVkIGJ5IHRoaXMgcG9pbnQgaW4gV2ViUGFnZTo6fldlYlBhZ2Uu CisKIDIwMTQtMDctMTcgIFNhbmdoeXVwIExlZSAgPHNoNTMubGVlQHNhbXN1bmcuY29tPgogCiAg ICAgICAgIFtFRkxdW1dLMl0gQWRkIGEgImZvY3VzLG5vdGZvdW5kIiBzaWduYWwuCmRpZmYgLS1n aXQgYS9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvbWFjL1NlcnZpY2VzT3Zlcmxh eUNvbnRyb2xsZXIubW0gYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvbWFjL1Nl cnZpY2VzT3ZlcmxheUNvbnRyb2xsZXIubW0KaW5kZXggYzJhMDdkMC4uOTNkNjY0YiAxMDA2NDQK LS0tIGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL21hYy9TZXJ2aWNlc092ZXJs YXlDb250cm9sbGVyLm1tCisrKyBiL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9t YWMvU2VydmljZXNPdmVybGF5Q29udHJvbGxlci5tbQpAQCAtODEsMTAgKzgxLDYgQEAgU2Vydmlj ZXNPdmVybGF5Q29udHJvbGxlcjo6U2VydmljZXNPdmVybGF5Q29udHJvbGxlcihXZWJQYWdlJiB3 ZWJQYWdlKQogCiBTZXJ2aWNlc092ZXJsYXlDb250cm9sbGVyOjp+U2VydmljZXNPdmVybGF5Q29u dHJvbGxlcigpCiB7Ci0gICAgaWYgKG1fc2VydmljZXNPdmVybGF5KSB7Ci0gICAgICAgIEFTU0VS VChtX3dlYlBhZ2UpOwotICAgICAgICBtX3dlYlBhZ2UtPnVuaW5zdGFsbFBhZ2VPdmVybGF5KG1f c2VydmljZXNPdmVybGF5LCBQYWdlT3ZlcmxheTo6RmFkZU1vZGU6OkRvTm90RmFkZSk7Ci0gICAg fQogfQogCiB2b2lkIFNlcnZpY2VzT3ZlcmxheUNvbnRyb2xsZXI6OnBhZ2VPdmVybGF5RGVzdHJv eWVkKFBhZ2VPdmVybGF5KikK