209145 2020-03-16 11:47:58 -0700 [Cocoa] Crash under -[WKPreferenceObserver init] 2020-03-30 13:53:08 -0700 1 1 1 Unclassified WebKit WebKit Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=209765 InRadar P2 Normal --- 1 pvollan pvollan bfulgham darin webkit-bug-importer oldest_to_newest 1630394 0 pvollan 2020-03-16 11:47:58 -0700 A crash has been observed on line 206 in PreferenceObserver.mm: 204 for (auto domain : domains) { 205 auto userDefaults = adoptNS([[WKUserDefaults alloc] initWithSuiteName:domain]); -> 206 userDefaults.get()->m_observer = self; 1630395 1 pvollan 2020-03-16 11:48:15 -0700 rdar://problem/60470713 1630400 2 393667 pvollan 2020-03-16 11:57:32 -0700 Created attachment 393667 Patch 1630418 3 393667 darin 2020-03-16 12:45:01 -0700 Comment on attachment 393667 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393667&action=review > Source/WebKit/UIProcess/Cocoa/PreferenceObserver.mm:206 > - auto userDefaults = adoptNS([[WKUserDefaults alloc] initWithSuiteName:domain]); > + auto userDefaults = adoptNS([WKUserDefaults alloc]); > + if (![userDefaults initWithSuiteName:domain]) { This is the wrong way to write it. Leave the code as is and write it like this: auto userDefaults = adoptNS([[WKUserDefaults alloc] initWithSuiteName:domain]); if (!userDefaults) { The adoptNS function already handles everything correctly. 1630433 4 pvollan 2020-03-16 13:12:02 -0700 (In reply to Darin Adler from comment #3) > Comment on attachment 393667 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=393667&action=review > > > Source/WebKit/UIProcess/Cocoa/PreferenceObserver.mm:206 > > - auto userDefaults = adoptNS([[WKUserDefaults alloc] initWithSuiteName:domain]); > > + auto userDefaults = adoptNS([WKUserDefaults alloc]); > > + if (![userDefaults initWithSuiteName:domain]) { > > This is the wrong way to write it. Leave the code as is and write it like > this: > > auto userDefaults = adoptNS([[WKUserDefaults alloc] > initWithSuiteName:domain]); > if (!userDefaults) { > > The adoptNS function already handles everything correctly. Fixed. Thanks for reviewing! 1630436 5 pvollan 2020-03-16 13:13:07 -0700 Committed r258515: <https://trac.webkit.org/changeset/258515/webkit> 393667 2020-03-16 11:57:32 -0700 2020-03-16 12:45:01 -0700 Patch bug-209145-20200316115732.patch text/plain 1809 pvollan SW5kZXg6IFNvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJL aXQvQ2hhbmdlTG9nCShyZXZpc2lvbiAyNTg1MTEpCisrKyBTb3VyY2UvV2ViS2l0L0NoYW5nZUxv Zwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBACisyMDIwLTAzLTE2ICBQZXIgQXJuZSBW b2xsYW4gIDxwdm9sbGFuQGFwcGxlLmNvbT4KKworICAgICAgICBbQ29jb2FdIENyYXNoIHVuZGVy IC1bV0tQcmVmZXJlbmNlT2JzZXJ2ZXIgaW5pdF0KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTIwOTE0NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIEhhbmRsZSB0aGUgY2FzZSB3aGVuIGNhbGxpbmcgW05TVXNl ckRlZmF1bHRzIGluaXRXaXRoU3VpdGVOYW1lOl0gZGlkIG5vdCBzdWNjZWVkLgorCisgICAgICAg IE5vIG5ldyB0ZXN0cywgc2luY2UgSSBoYXZlIG5vdCBiZWVuIGFibGUgdG8gcmVwcm9kdWNlLgor CisgICAgICAgICogVUlQcm9jZXNzL0NvY29hL1ByZWZlcmVuY2VPYnNlcnZlci5tbToKKyAgICAg ICAgKC1bV0tQcmVmZXJlbmNlT2JzZXJ2ZXIgaW5pdF0pOgorCiAyMDIwLTAzLTE2ICBEYXZpZCBL aWx6ZXIgIDxkZGtpbHplckBhcHBsZS5jb20+CiAKICAgICAgICAgV2ViUGFnZTo6R2V0RGF0YVNl bGVjdGlvbkZvclBhc3RlYm9hcmQgc2hvdWxkIHZhbGlkYXRlIGl0cyBgc2l6ZWAgdmFyaWFibGUK SW5kZXg6IFNvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL0NvY29hL1ByZWZlcmVuY2VPYnNlcnZlci5t bQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9Db2NvYS9QcmVmZXJlbmNl T2JzZXJ2ZXIubW0JKHJldmlzaW9uIDI1ODM2MykKKysrIFNvdXJjZS9XZWJLaXQvVUlQcm9jZXNz L0NvY29hL1ByZWZlcmVuY2VPYnNlcnZlci5tbQkod29ya2luZyBjb3B5KQpAQCAtMjAyLDcgKzIw MiwxMSBAQCAtIChpbnN0YW5jZXR5cGUpaW5pdAogICAgIH07CiAKICAgICBmb3IgKGF1dG8gZG9t YWluIDogZG9tYWlucykgewotICAgICAgICBhdXRvIHVzZXJEZWZhdWx0cyA9IGFkb3B0TlMoW1tX S1VzZXJEZWZhdWx0cyBhbGxvY10gaW5pdFdpdGhTdWl0ZU5hbWU6ZG9tYWluXSk7CisgICAgICAg IGF1dG8gdXNlckRlZmF1bHRzID0gYWRvcHROUyhbV0tVc2VyRGVmYXVsdHMgYWxsb2NdKTsKKyAg ICAgICAgaWYgKCFbdXNlckRlZmF1bHRzIGluaXRXaXRoU3VpdGVOYW1lOmRvbWFpbl0pIHsKKyAg ICAgICAgICAgIFdURkxvZ0Fsd2F5cygiQ291bGQgbm90IGluaXQgdXNlciBkZWZhdWx0cyBpbnN0 YW5jZSBmb3IgZG9tYWluICVzIiwgU3RyaW5nKGRvbWFpbikudXRmOCgpLmRhdGEoKSk7CisgICAg ICAgICAgICBjb250aW51ZTsKKyAgICAgICAgfQogICAgICAgICB1c2VyRGVmYXVsdHMuZ2V0KCkt Pm1fb2JzZXJ2ZXIgPSBzZWxmOwogICAgICAgICAvLyBTdGFydCBvYnNlcnZpbmcgYSBkdW1teSBr ZXkgaW4gb3JkZXIgdG8gbWFrZSB0aGUgcHJlZmVyZW5jZSBkYWVtb24gYmVjb21lIGF3YXJlIG9m IG91ciBOU1VzZXJEZWZhdWx0cyBpbnN0YW5jZS4KICAgICAgICAgLy8gVGhpcyBpcyB0byBtYWtl IHN1cmUgd2UgcmVjZWl2ZSBLVk8gbm90aWZpY2F0aW9ucy4gV2UgY2Fubm90IHVzZSBub3JtYWwg S1ZPIHRlY2huaXF1ZXMgaGVyZSwgc2luY2Ugd2UgYXJlIGxvb2tpbmcK