Move computeUserPrefersSimplified to Language.cpp and add locking around it
Created attachment 446309 [details] Patch
Comment on attachment 446309 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446309&action=review > Source/WTF/wtf/Language.cpp:137 > + Vector<String>& override = preferredLanguagesOverride(); |override| is per-thread? It's surprising to see it move from inside the critical section to outside. > Source/WTF/wtf/Language.cpp:149 > - return isolatedCopy(languages); > + return languages; How confident are you that this is safe? > Source/WTF/wtf/Language.cpp:164 > + if (equalIgnoringASCIICase(language, "zh-tw")) > + return false; > + if (equalIgnoringASCIICase(language, "zh-cn")) > + return true; Wow I can't believe I wrote this. https://www.rfc-editor.org/rfc/rfc4647
(In reply to Myles C. Maxfield from comment #2) > |override| is per-thread? It's surprising to see it move from inside the > critical section to outside. It's not. This function (computeUserPreferredLanguages, which is where the local `override` variable is) is annotated with WTF_REQUIRES_LOCK(languagesLock). > > Source/WTF/wtf/Language.cpp:149 > > - return isolatedCopy(languages); > > + return languages; > > How confident are you that this is safe? What I've written is safe now, because userPreferredLanguages now does the isolatedCopy, and computeUserPrefersSimplifiedChinese doesn't let the reference escape. I really just wanted to avoid having computeUserPrefersSimplifiedChinese clone the array for no good reason. The thread safety annotations won't help if someone writes another function that calls computeUserPrefersSimplifiedChinese and does let it escape. If you have a suggestion for how to rearrange the code to protect against that I'd be happy to entertain it.
(One way would be to duplicate the contents of computeUserPreferredLanguages into the two functions using it.)
<rdar://problem/86507413>
Created attachment 461034 [details] Patch
mattwoodrow@apple.com does not have reviewer permissions according to https://raw.githubusercontent.com/WebKit/WebKit/main/metadata/contributors.json. Rejecting attachment 461034 [details] from commit queue.
Committed 252655@main (352ffc3fc81e): <https://commits.webkit.org/252655@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 461034 [details].