<AWT Dev> RfR JDK-8055160

Mandy Chung mandy.chung at oracle.com
Thu Jun 11 01:39:45 UTC 2015


> On Jun 10, 2015, at 4:54 PM, Pete Brunet <peter.brunet at oracle.com> wrote:
> 
> Hi Mandy, That's fixed in the JDK-8078335 patch I submitted earlier in
> the day to build-dev as a RfR.  I tested that on Win and Mac.
> 
> http://cr.openjdk.java.net/~ptbrunet/JDK-8078335/webrev.00/

Looks good to me.

Mandy

> 
> Pete
> 
> On 6/10/15 6:08 PM, Mandy Chung wrote:
>> Just a quick check, jdk.accessibility is only linked in windows image at the moment.  It is a bug. Are you going to fix that in this changeset?   I think you have to verify this change in windows as well as other platforms.
>> 
>> Mandy
>> 
>> 
>>> On Jun 10, 2015, at 3:33 PM, Pete Brunet <peter.brunet at oracle.com> wrote:
>>> 
>>> Due to some other priorities it's been over 2 months since the last webrev.  An update is here:
>>> http://cr.openjdk.java.net/~ptbrunet/JDK-8055160/webrev.03
>>> 
>>> The changes from webrev.02 are:
>>> 
>>> 1) The test was changed to not use the service provider to test the activation of the service provider.  Instead a file is created when Toolkit.getDefaultToolkit activates providers and tested for existence when the test runs.
>>> 
>>> 2) The copyright header in the new jdk.accessibility files were fixed.
>>> 
>>> Pete
>>> 
>>> On 4/3/15 3:59 PM, Pete Brunet wrote:
>>>> Due to the recent push of JDK-8076182 (Open source Java Access Bridge) which exposed some files that were in closed the webrev needs a full re-review.  I've also made the changes requested by Mandy.
>>>> 
>>>> http://cr.openjdk.java.net/~ptbrunet/JDK-8055160/webrev.02/ 
>>>> 
>>>> Pete
>>>> 
>>>> On 3/23/15 4:41 PM, Mandy Chung wrote:
>>>>> 
>>>>> On 3/19/2015 6:03 PM, Pete Brunet wrote:
>>>>>> A new webrev is available at 
>>>>>> http://cr.openjdk.java.net/~ptbrunet/JDK-8055160/webrev.01/ 
>>>>>> 
>>>>> line 820-821: this comment is incorrect.  
>>>>> 
>>>>> line 831-838: what happens if ServiceConfigurationException thrown or any exception is thrown by the activate method?  This should wrap with AWTError as I mentioned in my previous review comment.  This was hidden with the test (see below).
>>>>> 
>>>>> line 891-901: this example may not be necessary as the service loader documentation should cover it.
>>>>> 
>>>>>> 
>>>>>> The changes to the tests are:
>>>>>> - added an unused provider
>>>>>> - added a test activating two providers
>>>>>> 
>>>>>> Mandy, Regarding the last bullet I'm not sure I resolved your comment, "For the test, since you support multiple providers, perhaps good to add one more test case to activate two providers and load two providers but only one is activated."  If not, please let me know.
>>>>> Almost.   For Foo, Bar providers, their activate method throwing RuntimeException actually stops loading the second provider.  The activate method could perhaps update some static field defined in the Load class if it's called (perhaps adding its name) so that you can tell whether the expected providers are activated.  UnusedProvider throwing RuntimeException is good since you don't expect it's activated.
>>>>> 
>>>>> Otherwise, looks good.
>>>>> 
>>>>> Mandy
> 



More information about the awt-dev mailing list