Bug 194529 - REGRESSION (r238955, r240494): Soft-linking optional Lookup.framework triggers release assertion when missing
Summary: REGRESSION (r238955, r240494): Soft-linking optional Lookup.framework trigger...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar, Regression
Depends on: 193815
Blocks:
  Show dependency tree / graph
 
Reported: 2019-02-11 21:08 PST by David Kilzer (:ddkilzer)
Modified: 2019-02-12 11:58 PST (History)
8 users (show)

See Also:

Attachments
Patch v1 (6.91 KB, patch)
2019-02-11 21:38 PST, David Kilzer (:ddkilzer) 		no flags Details | Formatted Diff | Diff
Patch v2 (7.62 KB, patch)
2019-02-11 22:19 PST, David Kilzer (:ddkilzer) 		no flags Details | Formatted Diff | Diff
Patch v3 (9.99 KB, patch)
2019-02-12 05:02 PST, David Kilzer (:ddkilzer) 		no flags Details | Formatted Diff | Diff
Patch v4 (11.22 KB, patch)
2019-02-12 09:15 PST, David Kilzer (:ddkilzer) 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.