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 >
