Bug 221914 – Reduce explicit usage of [objC release] in WebKit even more

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Chris Dumez]

Reduce explicit usage of [objC release] in WebKit even more.

[Chris Dumez]

Created attachment 420379 [details]
Patch

[EWS Watchlist]

This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)

[Alex Christensen]

Comment on attachment 420379 [details]
Patch

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

> Source/WebKitLegacy/mac/History/WebHistory.mm:664
> +    _sharedHistory = retainPtr(history).leakRef();

I think it would be nicer to change the type of _sharedHistory to a function-scoped NeverDestroyed<RetainPtr>

> Source/WebKitLegacy/mac/Plugins/WebPluginDatabase.mm:195
> +    auto oldPaths = adoptNS(additionalWebPlugInPaths);

Ditto.  additionalWebPlugInPaths should be a RetainPtr.

[Chris Dumez]

(In reply to Alex Christensen from comment #3)
> Comment on attachment 420379 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=420379&action=review
> 
> > Source/WebKitLegacy/mac/History/WebHistory.mm:664
> > +    _sharedHistory = retainPtr(history).leakRef();
> 
> I think it would be nicer to change the type of _sharedHistory to a
> function-scoped NeverDestroyed<RetainPtr>

Same comment as below. _sharedHistory is a global static.

> 
> > Source/WebKitLegacy/mac/Plugins/WebPluginDatabase.mm:195
> > +    auto oldPaths = adoptNS(additionalWebPlugInPaths);
> 
> Ditto.  additionalWebPlugInPaths should be a RetainPtr.

additionalWebPlugInPaths is a global static. If I use a RetainPtr<> then the compiler complains about having an exit-time destructor. We usually use NeverDestroyed<> but it does not seem to fit the bill here since we want to update the pointer and release the previous value.

[Darin Adler]

(In reply to Chris Dumez from comment #4)
> additionalWebPlugInPaths is a global static. If I use a RetainPtr<> then the
> compiler complains about having an exit-time destructor. We usually use
> NeverDestroyed<> but it does not seem to fit the bill here since we want to
> update the pointer and release the previous value.

NeverDestroyed<RetainPtr<>> should work fine for that. Despite its name, assigning something to such a variable would update the pointer and release the previous value. That’s *assigning*, not *destroying*.

[Chris Dumez]

(In reply to Darin Adler from comment #5)
> (In reply to Chris Dumez from comment #4)
> > additionalWebPlugInPaths is a global static. If I use a RetainPtr<> then the
> > compiler complains about having an exit-time destructor. We usually use
> > NeverDestroyed<> but it does not seem to fit the bill here since we want to
> > update the pointer and release the previous value.
> 
> NeverDestroyed<RetainPtr<>> should work fine for that. Despite its name,
> assigning something to such a variable would update the pointer and release
> the previous value. That’s *assigning*, not *destroying*.

Hmm. I did not see an operator=() in NeverDestroyed<>.

[Darin Adler]

You have to write .get() on the left side of the "=" operator. Adding operator= to NeverDestroyed would eliminate the need to write .get().

[Chris Dumez]

Created attachment 420484 [details]
Patch

[Chris Dumez]

Created attachment 420487 [details]
Patch

[EWS]

Committed r272919: <https://commits.webkit.org/r272919>

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

[Radar WebKit Bug Importer]

<rdar://problem/74399599>