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:
http://cr.openjdk.java.net/~ptbrunet/JDK-8055160/webrev.03
Build changes look good.
/Magnus
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
