Bug 209145

Summary: [Cocoa] Crash under -[WKPreferenceObserver init]
Product: WebKit Reporter: Per Arne Vollan <pvollan>
Component: WebKit Misc.Assignee: Per Arne Vollan <pvollan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, darin, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=209765
Attachments:
Description Flags
Patch darin: review+

Description Per Arne Vollan 2020-03-16 11:47:58 PDT
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;
Comment 1 Per Arne Vollan 2020-03-16 11:48:15 PDT
rdar://problem/60470713
Comment 2 Per Arne Vollan 2020-03-16 11:57:32 PDT
Created attachment 393667 [details]
Patch
Comment 3 Darin Adler 2020-03-16 12:45:01 PDT
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.
Comment 4 Per Arne Vollan 2020-03-16 13:12:02 PDT
(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!
Comment 5 Per Arne Vollan 2020-03-16 13:13:07 PDT
Committed r258515: <https://trac.webkit.org/changeset/258515/webkit>