<i18n dev> RFR: 8152817: Locale data loading fails silently when running with a security manager

[Masayoshi Okutsu]

Webrev has been updated. I realized that some code which was for loading 
resource bundles in both java.base and jdk.localedata remained. The 
providers are no longer used for loading resource bundles in java.base. 
The code was removed.

http://cr.openjdk.java.net/~okutsu/9/8152817/webrev.01/

Thanks,
Masayoshi

On 3/31/2016 8:56 AM, Masayoshi Okutsu wrote:
> On 3/31/2016 12:54 AM, Alan Bateman wrote:
>> On 30/03/2016 16:48, Mandy Chung wrote:
>>>> On Mar 30, 2016, at 8:40 AM, Masayoshi Okutsu 
>>>> <masayoshi.okutsu at oracle.com> wrote:
>>>>
>>>> Hello,
>>>>
>>>> Please review the fix for JDK-8152817. The fix is to load locale 
>>>> data from its own module without calling 
>>>> ResourceBundleProviderSupport.loadResourceBundle.
>>>>
>>>> I changed the synopsis of the JBS issue because it's a general 
>>>> issue of loading locale data under a security manager, and the 
>>>> regression test uses DateFormatSymbols.
>>>>
>>>> Issue:
>>>> https://bugs.openjdk.java.net/browse/JDK-8152817
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~okutsu/9/8152817/webrev.00/
>>>
>>> The patch looks fine.  It makes sense to have its own copy of the 
>>> utility method to load a module-private class.
>> I agree. There's another usage in SupplementaryLocaleDataProvider and 
>> I wonder if we have an issue there too.
>
> Thank you for catching this.
>
>>
>>
>>>
>>> You can call Class::newInstance instead of Constructor::newInstance 
>>> to make the code much simpler. Class::newInstance doesn’t throw 
>>> InvocationTargetException and no need to have uncheckedThrow.
>>>
>> I was wondering about that too.
>
> My initial fix called Class::newInstance. But I thought 
> Constructor::newInstance would be better after reading the API doc. I 
> will change it back to call Class::newInstance.
>
> Thanks,
> Masayoshi
>