Bug 212546 - REGRESSION (r260306): Compatibility issue leading to crash on macOS games
Summary: REGRESSION (r260306): Compatibility issue leading to crash on macOS games
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on: 210625
Blocks:
  Show dependency treegraph
 
Reported: 2020-05-29 14:38 PDT by David Kilzer (:ddkilzer)
Modified: 2021-07-21 13:39 PDT (History)
5 users (show)

See Also:


Attachments
Patch v1 (1.50 KB, patch)
2020-05-29 14:43 PDT, David Kilzer (:ddkilzer)
bfulgham: review+
Details | Formatted Diff | Diff
Patch for landing (1.49 KB, patch)
2020-05-29 15:29 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2020-05-29 14:38:36 PDT
Compatibility issue leading to crash on macOS games.

Changing instance variables to __weak in the _WebSafeForwarder class caused a couple of macOS games to crash just before starting play.

The fix is to remove the __weak attribute, which effectively changes them back to __unsafe_unretained.

Caused by:

    Bug 210625: [iOS WK1] -[_WebSafeForwarder asyncForwarder] uses non-static dispatch_once_t predicate
    <https://bugs.webkit.org/show_bug.cgi?id=210625>
    <https://trac.webkit.org/changeset/260306/webkit>

<rdar://problem/62624078>
Comment 1 David Kilzer (:ddkilzer) 2020-05-29 14:43:47 PDT
Created attachment 400620 [details]
Patch v1
Comment 2 David Kilzer (:ddkilzer) 2020-05-29 14:45:21 PDT
Comment on attachment 400620 [details]
Patch v1

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

> Source/WebKitLegacy/mac/WebView/WebView.mm:569
> +    // Do not not change _target and _defaultTarget to __weak. See <rdar://problem/62624078>.
> +    id _target;
> +    id _defaultTarget;

Should I proactively change these to __unsafe_unretained to future-proof a possible mistake during ARC conversion?
Comment 3 Brent Fulgham 2020-05-29 14:57:38 PDT
Comment on attachment 400620 [details]
Patch v1

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

r=me

>> Source/WebKitLegacy/mac/WebView/WebView.mm:569
>> +    id _defaultTarget;
> 
> Should I proactively change these to __unsafe_unretained to future-proof a possible mistake during ARC conversion?

Yes, that seems like a good idea.
Comment 4 David Kilzer (:ddkilzer) 2020-05-29 15:29:27 PDT
Created attachment 400627 [details]
Patch for landing
Comment 5 David Kilzer (:ddkilzer) 2020-05-29 15:30:17 PDT
Waiting on EWS before marking cq+.
Comment 6 David Kilzer (:ddkilzer) 2020-05-29 17:40:32 PDT
Comment on attachment 400627 [details]
Patch for landing

TestWebKitAPI.WKWebsiteDataStore.RemoveAndFetchData is failing on api-ios, but it is a WebKit2 test, so this patch can't possibly affect it.  Marking as cq+.
Comment 7 EWS 2020-05-29 17:42:49 PDT
Committed r262330: <https://trac.webkit.org/changeset/262330>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 400627 [details].