Bug 194529

Summary: REGRESSION (r238955, r240494): Soft-linking optional Lookup.framework triggers release assertion when missing
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: Web Template FrameworkAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, aestes, commit-queue, ddkilzer, eric.carlson, jer.noble, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar, Regression
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 193815    
Bug Blocks:    
Attachments:
Description Flags
Patch v1
none
Patch v2
none
Patch v3
none
Patch v4 none

Description David Kilzer (:ddkilzer) 2019-02-11 21:08:37 PST
The optional Lookup.framework soft-link hits a release assertion when it's missing.

This regressed r240494 when I changed to use the newer soft-linking macros, but the newer soft-linking macros originally broke in r238955 when optional frameworks starting hitting release assertions.

<rdar://problem/47924449>
Comment 1 David Kilzer (:ddkilzer) 2019-02-11 21:38:33 PST
Created attachment 361767 [details]
Patch v1
Comment 2 David Kilzer (:ddkilzer) 2019-02-11 22:19:34 PST
Created attachment 361771 [details]
Patch v2
Comment 3 David Kilzer (:ddkilzer) 2019-02-12 05:02:55 PST
Created attachment 361790 [details]
Patch v3

The charm.
Comment 4 Eric Carlson 2019-02-12 05:41:20 PST
(In reply to David Kilzer (:ddkilzer) from comment #3)
> Created attachment 361790 [details]
> Patch v3
> 
> The charm.

Looks like the iOS bots don't agree with this old adage.
Comment 5 David Kilzer (:ddkilzer) 2019-02-12 09:01:27 PST
(In reply to Eric Carlson from comment #4)
> (In reply to David Kilzer (:ddkilzer) from comment #3)
> > Created attachment 361790 [details]
> > Patch v3
> > 
> > The charm.
> 
> Looks like the iOS bots don't agree with this old adage.

Doh.  Should have rebuilt again locally after adding DataDetectorsCoreSoftLink.mm to the WebCore target.
Comment 6 David Kilzer (:ddkilzer) 2019-02-12 09:15:34 PST
Created attachment 361801 [details]
Patch v4

Fix iOS builds.
Comment 7 David Kilzer (:ddkilzer) 2019-02-12 09:23:56 PST
Comment on attachment 361801 [details]
Patch v4

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

> Source/WebCore/ChangeLog:21
> +        * platform/cocoa/DataDetectorsCoreSoftLink.mm:
> +        - Switch from using SOFT_LINK_PRIVATE_FRAMEWORK_OPTIONAL() to
> +          SOFT_LINK_PRIVATE_FRAMEWORK_FOR_SOURCE() when linking
> +          DataDetectorsCore.framework. None of the other macros assume
> +          this framework is optional, and it was likely made optional
> +          originally because the framework was new to iOS and thus
> +          didn't exist on older versions.

Also, the optional behavior for frameworks in these new-style soft-linking macros really depends on how they're called from the other Class, Pointer, Constant, Function, etc. soft-linking macros in the same source file.
Comment 8 WebKit Commit Bot 2019-02-12 11:58:25 PST
Comment on attachment 361801 [details]
Patch v4

Clearing flags on attachment: 361801

Committed r241309: <https://trac.webkit.org/changeset/241309>
Comment 9 WebKit Commit Bot 2019-02-12 11:58:27 PST
All reviewed patches have been landed.  Closing bug.