Note that I need to remove the import of java.io.PrintWriter in java.awt.Toolkit.java
On 6/10/15 5:33 PM, 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 > > 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 >> >
