Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ iOS ] fast/text/international/system-language/navigator-language/navigator-language-fr.html is a flaky failure #536

Closed
wants to merge 2 commits into from
Closed

Conversation

cdumez
Copy link
Contributor

@cdumez cdumez commented May 6, 2022

067130f

[ iOS ] fast/text/international/system-language/navigator-language/navigator-language-fr.html is a flaky failure
https://bugs.webkit.org/show_bug.cgi?id=240104

Reviewed by NOBODY (OOPS!).

The test was flaky and navigator.language would sometimes return "fr" and sometimes "fr-FR".
The reason for this is that Cocoa ports used 2 separate mechanisms to override the system
language:
1. The override languages were used to set the AppleLanguages NSUserDefaults, causing APIs
   such as CFLocaleCopyPreferredLanguages() to return the overriden languages.
2. During process initialization we would also call WTF::overrideUserPreferredLanguages()
   which would add the override languages to an override Vector in WTF.

The test was setting the override language to "fr". When method 2 would succeed,
navigator.language would return "fr", from preferredLanguagesOverride().

However, Internals::resetToConsistentState() would reset WTF::preferredLanguagesOverride()
shortly after the test starts running. As a result, the override Vector in WTF would often
end up being empty and we would end up calling CFLocaleCopyPreferredLanguages().
However, CFLocaleCopyPreferredLanguages() return "fr-FR", which is equivalent but not
exactly the same.

To address the issue, I made 2 changes:
a. Use a single method for overriding languages for Cocoa ports. We are now using the
   AppleLanguages user default exclusively and not the WTF::preferredLanguagesOverride()
   Vector.
b. We stop resetting the override languages in Internals::resetToConsistentState(), since
   this runs after the test options have been processed and which may set the override
   languages. Once making fix a., we would end up with no override language for the test
   since resetting would reliably work. We don't need to reset languages manually anyway
   to avoid flakiness. The reason is that the override languages get set on the process
   pool configuration and we switch to a new process pool / WKWebView between tests when
   the previous & next tests have different test options (override languages being one of
   the options).

* Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm:
(WebKit::setAppleLanguagesPreference):
* Source/WTF/wtf/Language.cpp:
(WTF::overrideUserPreferredLanguages):
* Source/WTF/wtf/cf/LanguageCF.cpp:
(WTF::overrideUserPreferredLanguages):
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::resetToConsistentState):
* LayoutTests/platform/ios/TestExpectations:

73cafd8

Identifier::string() should return an AtomString
https://bugs.webkit.org/show_bug.cgi?id=240122

Reviewed by NOBODY (OOPS!).

Identifier::string() should return an AtomString instead of a String, since
it holds an AtomString internally.

Also add some overloads to jsString() to resolve ambiguity for some callers.

* Source/JavaScriptCore/API/JSContext.mm:
(-[JSContext dependencyIdentifiersForModuleJSScript:]):
* Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:
(JSC::processClauseList):
* Source/JavaScriptCore/dfg/DFGOperations.cpp:
(JSC::DFG::JSC_DEFINE_JIT_OPERATION):
* Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:
(Inspector::JSInjectedScriptHost::functionDetails):
* Source/JavaScriptCore/jsc.cpp:
(JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/Error.cpp:
(JSC::addErrorInfo):
* Source/JavaScriptCore/runtime/ErrorInstance.cpp:
(JSC::ErrorInstance::finishCreation):
* Source/JavaScriptCore/runtime/ErrorPrototype.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/FunctionExecutable.cpp:
(JSC::FunctionExecutable::toStringSlow):
* Source/JavaScriptCore/runtime/Identifier.h:
(JSC::Identifier::string const):
(JSC::Identifier::atomString const): Deleted.
* Source/JavaScriptCore/runtime/IdentifierInlines.h:
(JSC::identifierToJSValue):
(JSC::identifierToSafePublicJSValue):
* Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp:
(JSC::IntlDateTimeFormat::format const):
(JSC::IntlDateTimeFormat::formatRange):
* Source/JavaScriptCore/runtime/IntlDisplayNames.cpp:
(JSC::IntlDisplayNames::of const):
* Source/JavaScriptCore/runtime/IntlListFormat.cpp:
(JSC::IntlListFormat::format const):
* Source/JavaScriptCore/runtime/IntlRelativeTimeFormat.cpp:
(JSC::IntlRelativeTimeFormat::format const):
* Source/JavaScriptCore/runtime/JSFunction.cpp:
(JSC::JSFunction::reifyName):
* Source/JavaScriptCore/runtime/JSModuleLoader.cpp:
(JSC::JSModuleLoader::requestImportModule):
* Source/JavaScriptCore/runtime/JSString.h:
(JSC::jsString):
* Source/JavaScriptCore/runtime/JSStringInlines.h:
(JSC::repeatCharacter):
* Source/JavaScriptCore/runtime/StringPrototype.cpp:
(JSC::jsSpliceSubstrings):
(JSC::jsSpliceSubstringsWithSeparators):
(JSC::toLocaleCase):
(JSC::normalize):
* Source/JavaScriptCore/runtime/SymbolPrototype.cpp:
(JSC::JSC_DEFINE_CUSTOM_GETTER):
* Source/WTF/wtf/PrintStream.cpp:
(WTF::printInternal):
* Source/WTF/wtf/PrintStream.h:
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::processKeyframeLikeObject):
* Source/WebCore/inspector/WebInjectedScriptHost.cpp:
(WebCore::WebInjectedScriptHost::getInternalProperties):

cdumez added 2 commits May 5, 2022 19:14
https://bugs.webkit.org/show_bug.cgi?id=240122

Reviewed by NOBODY (OOPS!).

Identifier::string() should return an AtomString instead of a String, since
it holds an AtomString internally.

Also add some overloads to jsString() to resolve ambiguity for some callers.

* Source/JavaScriptCore/API/JSContext.mm:
(-[JSContext dependencyIdentifiersForModuleJSScript:]):
* Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:
(JSC::processClauseList):
* Source/JavaScriptCore/dfg/DFGOperations.cpp:
(JSC::DFG::JSC_DEFINE_JIT_OPERATION):
* Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:
(Inspector::JSInjectedScriptHost::functionDetails):
* Source/JavaScriptCore/jsc.cpp:
(JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/Error.cpp:
(JSC::addErrorInfo):
* Source/JavaScriptCore/runtime/ErrorInstance.cpp:
(JSC::ErrorInstance::finishCreation):
* Source/JavaScriptCore/runtime/ErrorPrototype.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/FunctionExecutable.cpp:
(JSC::FunctionExecutable::toStringSlow):
* Source/JavaScriptCore/runtime/Identifier.h:
(JSC::Identifier::string const):
(JSC::Identifier::atomString const): Deleted.
* Source/JavaScriptCore/runtime/IdentifierInlines.h:
(JSC::identifierToJSValue):
(JSC::identifierToSafePublicJSValue):
* Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp:
(JSC::IntlDateTimeFormat::format const):
(JSC::IntlDateTimeFormat::formatRange):
* Source/JavaScriptCore/runtime/IntlDisplayNames.cpp:
(JSC::IntlDisplayNames::of const):
* Source/JavaScriptCore/runtime/IntlListFormat.cpp:
(JSC::IntlListFormat::format const):
* Source/JavaScriptCore/runtime/IntlRelativeTimeFormat.cpp:
(JSC::IntlRelativeTimeFormat::format const):
* Source/JavaScriptCore/runtime/JSFunction.cpp:
(JSC::JSFunction::reifyName):
* Source/JavaScriptCore/runtime/JSModuleLoader.cpp:
(JSC::JSModuleLoader::requestImportModule):
* Source/JavaScriptCore/runtime/JSString.h:
(JSC::jsString):
* Source/JavaScriptCore/runtime/JSStringInlines.h:
(JSC::repeatCharacter):
* Source/JavaScriptCore/runtime/StringPrototype.cpp:
(JSC::jsSpliceSubstrings):
(JSC::jsSpliceSubstringsWithSeparators):
(JSC::toLocaleCase):
(JSC::normalize):
* Source/JavaScriptCore/runtime/SymbolPrototype.cpp:
(JSC::JSC_DEFINE_CUSTOM_GETTER):
* Source/WTF/wtf/PrintStream.cpp:
(WTF::printInternal):
* Source/WTF/wtf/PrintStream.h:
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::processKeyframeLikeObject):
* Source/WebCore/inspector/WebInjectedScriptHost.cpp:
(WebCore::WebInjectedScriptHost::getInternalProperties):
…vigator-language-fr.html is a flaky failure

https://bugs.webkit.org/show_bug.cgi?id=240104

Reviewed by NOBODY (OOPS!).

The test was flaky and navigator.language would sometimes return "fr" and sometimes "fr-FR".
The reason for this is that Cocoa ports used 2 separate mechanisms to override the system
language:
1. The override languages were used to set the AppleLanguages NSUserDefaults, causing APIs
   such as CFLocaleCopyPreferredLanguages() to return the overriden languages.
2. During process initialization we would also call WTF::overrideUserPreferredLanguages()
   which would add the override languages to an override Vector in WTF.

The test was setting the override language to "fr". When method 2 would succeed,
navigator.language would return "fr", from preferredLanguagesOverride().

However, Internals::resetToConsistentState() would reset WTF::preferredLanguagesOverride()
shortly after the test starts running. As a result, the override Vector in WTF would often
end up being empty and we would end up calling CFLocaleCopyPreferredLanguages().
However, CFLocaleCopyPreferredLanguages() return "fr-FR", which is equivalent but not
exactly the same.

To address the issue, I made 2 changes:
a. Use a single method for overriding languages for Cocoa ports. We are now using the
   AppleLanguages user default exclusively and not the WTF::preferredLanguagesOverride()
   Vector.
b. We stop resetting the override languages in Internals::resetToConsistentState(), since
   this runs after the test options have been processed and which may set the override
   languages. Once making fix a., we would end up with no override language for the test
   since resetting would reliably work. We don't need to reset languages manually anyway
   to avoid flakiness. The reason is that the override languages get set on the process
   pool configuration and we switch to a new process pool / WKWebView between tests when
   the previous & next tests have different test options (override languages being one of
   the options).

* Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm:
(WebKit::setAppleLanguagesPreference):
* Source/WTF/wtf/Language.cpp:
(WTF::overrideUserPreferredLanguages):
* Source/WTF/wtf/cf/LanguageCF.cpp:
(WTF::overrideUserPreferredLanguages):
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::resetToConsistentState):
* LayoutTests/platform/ios/TestExpectations:
@cdumez cdumez self-assigned this May 6, 2022
@cdumez cdumez added New Bugs Unclassified bugs are placed in this component until the correct component can be determined. WebKit Nightly Build labels May 6, 2022
@cdumez cdumez closed this May 6, 2022
@cdumez cdumez deleted the 240104_navigator-language-fr-flaky branch May 6, 2022 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Bugs Unclassified bugs are placed in this component until the correct component can be determined.
Projects
None yet
1 participant