hi Brent,
Thank you very much for tackling this. The code overall looks good (aside from
the 3 code language sub tag case that we need to handle, as Naoto pointed out)
Here are my 2 small comments:
#1
63 CFRelease(languages); // was missing from original patch
I think we can drop the comment from line 63?
#2
131 void setOSNameAndVersion(java_props_t *sprops) {
132 /* Don't rely on JRSCopyOSName because there's no guarantee the value
will
133 * remain the same, or even if the JRS functions will continue to be
part of
134 * Mac OS X. So hardcode os_name, and fill in os_version if we can.
135 */
The comment from lines 132-135 should be dropped - we no longer use JSR - and
just say that we're hardcoding the OS name?
cheers
> On Jun 13, 2016, at 3:58 PM, Brent Christian <[email protected]>
> wrote:
>
> Hi,
>
> Please review this Mac-only fix:
>
> http://cr.openjdk.java.net/~bchristi/7131356/webrev.01/
> https://bugs.openjdk.java.net/browse/JDK-7131356
>
> Thanks go to Gerard Ziemski for the thorough investigation and detailed
> write-up in the bug report.
>
> The symptoms:
>
> On those OS X machines where the default system Java is not installed, any
> attempt to instantiate JVM from a local JDK (for example via JNI as shown in
> the bug) presents "No Java installed. Do you want to install Java?" dialog
> and refuses to run. Clearly, a valid local JDK should be able to run in this
> case.
>
> The problem:
>
> In java_props_macosx.c, there is code that dynamically looks up 4 methods in
> JavaRuntimeSupport.framework (JRS) and calls into them to find out localized
> OS version, default locale and the preferred language, but JRS checks for a
> system available Java and refuses to run if none found.
>
> Background:
>
> JavaRuntimeSupport.framework (JRS) is a framework implemented and provided by
> Apple for use by Java. It is a "black box" that wraps SPIs required by the
> Apple-provided implementation of JDK, exposing them to the JDK as APIs.
>
> Now that the JDK implementation ownership has changed from Apple to Oracle,
> we have the issue that we don't control the JRS implementation, but rely on
> Apple to support it for our JDK to continue to run. Depending on a private
> framework provided by a third party for our code to continue to run doesn't
> seem like a good long term bet (see also 8024281).
>
> Proposed solution:
>
> Apple suggested changing the JDK initialization order so any access to JRS
> happens only after the JLI_MemAlloc symbol is available. We believe a better
> solution is to re-implement the JSR APIs in question, as a move toward
> eventually dropping reliance on JSR altogether. The three main changes are:
>
> 1. In "getMacOSXLocale", re-implement "JRSCopyPrimaryLanguage" using
> "CFLocaleCopyPreferredLanguages", which gives us the preferred language as
> set in "System Preferences"->"Language & Text"->"Language" in the form of
> "xx" (ie. just the language code - ex. "en", "fr", etc.). The original Apple
> code then calls into "JRSCopyCanonicalLanguageForPrimaryLanguage" to map
> "language" into "language_REGION" (ex. "en"-->"en_US", "fr"-->"fr_FR", etc.),
> though this appears to be unnecessary. Following the call to
> setupMacOSXLocale() in ParseLocale()[1], mapLookup() is called to map the
> language to a default country, per this comment:
>
> * If the locale name (without .encoding@variant, if any) matches
> * any of the names in the locale_aliases list, map it to the
> * corresponding full locale name. Most of the entries in the
> * locale_aliases list are locales that include a language name but
> * no country name, and this facility is used to map each language
> * to a default country if that's possible.
>
> I tried out a few locales, for English (en_US, en_GB), German (de_DE, de_CH)
> and French (fr_FR, fr_CA). Language/country system properties behave the
> same before and after this change, as does the java.util.Locale test attached
> to the bug.
>
> 2. In "setupMacOSXLocale" we simply drop the call to
> "JRSSetDefaultLocalization" as it appears to be a NOP. According to Apple,
> that API sets up native bundle locale, so that any access to native Cocoa UI
> (like FileOpenChooser) uses localized strings. Testing shows that this does
> not seem to work even in Apple's own JDK (ie. JDK 6), so dropping the call to
> this SPI here does not result in a regression. An issue was filed to
> investigate further (8024279, a dup of 8019464) which has since been closed
> as, "Not an Issue".
>
> 3. In "setOSNameAndVersion", re-implement JRSCopyOSVersion using
> [NSProcessInfo operatingSystemVersion]. (Use of JRSCopyOSName was already
> removed by 7178922).
>
>
> Thanks,
> -Brent
>
> 1.
> http://hg.openjdk.java.net/jdk9/dev/jdk/file/79043a1c3547/src/java.base/unix/native/libjava/java_props_md.c
>
