Make WAKScrollView delegate a weak property to prevent crashes where the delegate has been released. <rdar://problem/39469669>
Created attachment 338374 [details] Patch v1
ERROR: Source/WebCore/platform/ios/wak/WAKScrollView.mm:65: Should not have spaces around = in property synthesis. [whitespace/property] [4] Should I just delete the _delegate instance variable?
Attachment 338374 [details] did not pass style-queue: ERROR: Source/WebCore/platform/ios/wak/WAKScrollView.mm:65: Should not have spaces around = in property synthesis. [whitespace/property] [4] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to Build Bot from comment #3) > Attachment 338374 [details] did not pass style-queue: > > > ERROR: Source/WebCore/platform/ios/wak/WAKScrollView.mm:65: Should not have > spaces around = in property synthesis. [whitespace/property] [4] > Total errors found: 1 in 3 files > > > If any of these errors are false positives, please file a bug against > check-webkit-style. Ha! Even better reason to remove it (build failure on iOS Simulator): In file included from ./platform/ios/wak/WAKScrollView.mm:27: ./platform/ios/wak/WAKScrollView.h:41:8: error: existing instance variable '_delegate' for __weak property 'delegate' must be __weak id _delegate; ^ ./platform/ios/wak/WAKScrollView.h:60:32: note: property declared here @property (nonatomic, weak) id delegate; ^ In file included from /Volumes/Data/EWS/WebKit/WebKitBuild/Release-iphonesimulator/DerivedSources/WebCore/unified-sources/UnifiedSource45-mm.mm:1: ./platform/ios/wak/WAKScrollView.mm:65:13: note: property synthesized here @synthesize delegate = _delegate; ^ 1 error generated.
Created attachment 338378 [details] Patch v2
Comment on attachment 338378 [details] Patch v2 This patch is effectively a no-op. 'weak' just means non-retained, which is what the old code was doing.
Maybe you're right: https://developer.apple.com/library/content/releasenotes/ObjectiveC/RN-TransitioningToARC/Introduction/Introduction.html "A weak reference does not extend the lifetime of the object it points to, and automatically becomes nil when there are no strong references to the object." Have you tested this?
Created attachment 338887 [details] Source for testing objc-arc and objc-weak
(In reply to Simon Fraser (smfr) from comment #7) > Maybe you're right: > https://developer.apple.com/library/content/releasenotes/ObjectiveC/RN- > TransitioningToARC/Introduction/Introduction.html > "A weak reference does not extend the lifetime of the object it points to, > and automatically becomes nil when there are no strong references to the > object." > > Have you tested this? Using Attachment #338887 [details]: With ARC: $ clang -fobjc-arc -x objective-c -o weak-smfr-arc -framework Foundation weak-smfr.m $ ./weak-smfr-arc 2018-04-26 10:17:42.295 weak-smfr-arc[97821:14281815] Before release: 0x7f916d40cb20 2018-04-26 10:17:42.295 weak-smfr-arc[97821:14281815] After release: 0x0 With MRR: $ clang -x objective-c -o weak-smfr-mrr -framework Foundation weak-smfr.m $ ./weak-smfr-mrr 2018-04-26 10:20:25.478 weak-smfr-mrr[97834:14292493] Before release: 0x7f88f2e06b00 2018-04-26 10:20:25.479 weak-smfr-mrr[97834:14292493] After release: 0x7f88f2e06b00 With MRR + WEAK: $ clang -fobjc-weak -x objective-c -o weak-smfr-weak -framework Foundation weak-smfr.m $ ./weak-smfr-weak 2018-04-26 10:20:38.310 weak-smfr-weak[97838:14293436] Before release: 0x7ff2eef04bd0 2018-04-26 10:20:38.310 weak-smfr-weak[97838:14293436] After release: 0x0 So, yes, this feature works as it was designed. :)
Created attachment 338889 [details] Patch v3 (fixed ChangeLog typo from v2)
Comment on attachment 338889 [details] Patch v3 (fixed ChangeLog typo from v2) Clearing flags on attachment: 338889 Committed r231058: <https://trac.webkit.org/changeset/231058>
All reviewed patches have been landed. Closing bug.