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

David Kilzer (:ddkilzer)
Reported 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>
Attachments
Patch v1 (6.91 KB, patch)
2019-02-11 21:38 PST, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
Patch v2 (7.62 KB, patch)
2019-02-11 22:19 PST, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
Patch v3 (9.99 KB, patch)
2019-02-12 05:02 PST, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
Patch v4 (11.22 KB, patch)
2019-02-12 09:15 PST, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1 2019-02-11 21:38:33 PST
Created attachment 361767 [details] Patch v1
David Kilzer (:ddkilzer)
Comment 2 2019-02-11 22:19:34 PST
Created attachment 361771 [details] Patch v2
David Kilzer (:ddkilzer)
Comment 3 2019-02-12 05:02:55 PST
Created attachment 361790 [details] Patch v3 The charm.
Eric Carlson
Comment 4 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.
David Kilzer (:ddkilzer)
Comment 5 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.
David Kilzer (:ddkilzer)
Comment 6 2019-02-12 09:15:34 PST
Created attachment 361801 [details] Patch v4 Fix iOS builds.
David Kilzer (:ddkilzer)
Comment 7 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.
WebKit Commit Bot
Comment 8 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>
WebKit Commit Bot
Comment 9 2019-02-12 11:58:27 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.