<AWT Dev> RfR JDK-8055160
Magnus Ihse Bursie
magnus.ihse.bursie at oracle.com
Fri Jun 12 13:02:18 UTC 2015
On 2015-06-11 00:33, Pete Brunet wrote:
> Due to some other priorities it's been over 2 months since the last
> webrev. An update is here:
Build changes look good.
> 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.
> 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.
>> 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
>>> 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.
More information about the build-dev