Re: Review Request: RT-37788

2014-07-03 Thread Danno Ferrin
Try #2, after running it through the the Xcode leak analyzer and unit testing (FYI keep your persistent domains lower case). WebRev: http://cr.openjdk.java.net/~shemnon/RT-37788/webrev.01/rt.patch JIRA: https://javafx-jira.kenai.com/browse/RT-37788 And let’s move further discussion to the JIRA i

Re: Review Request: RT-37788

2014-07-02 Thread Petr Pchelko
Hello, Danno. I’ve noticed that you are leaking userDefaults and expandedOptions objects. Adding autorelease to them would save you some memory. With best regards. Petr. > On Jul 3, 2014, at 1:43 AM, Danno Ferrin wrote: > > Yes, this has to be fixed in native code. 8u40 it is then. > > I ca

Re: Review Request: RT-37788

2014-07-02 Thread Danno Ferrin
Yes, this has to be fixed in native code. 8u40 it is then. I can make it cause a crash, but that starts with shooting myself in the foot, and not much can be done to fix it (by passing in bogus VM arguments). On Jul 2, 2014, at 3:40 PM, Stephen F Northover wrote: > Personally, I wouldn't ch

Re: Review Request: RT-37788

2014-07-02 Thread Chris Bensen
The review is for 8u40, but Danno would like to get it into 8u20 last time we chatted which is why he was asking I believe. Chris On Jul 2, 2014, at 2:40 PM, Stephen F Northover wrote: > Personally, I wouldn't change any native code at this point unless it was > fixing a crash. The review

Re: Review Request: RT-37788

2014-07-02 Thread Stephen F Northover
Personally, I wouldn't change any native code at this point unless it was fixing a crash. The review is for 8u40, correct? Steve On 2014-07-02, 5:38 PM, Chris Bensen wrote: I’m not sure about for 8u20. Seems fairly straight forward, and your Obj-C seems as good as any Obj-C. My only complaint

Re: Review Request: RT-37788

2014-07-02 Thread Chris Bensen
I’m not sure about for 8u20. Seems fairly straight forward, and your Obj-C seems as good as any Obj-C. My only complaint at the moment is the following: 358 if ([pathParts count] > 2) { 359 // for 3 or more steps, the domain is first.second.third and the keys are "/first/second/thir

Re: Review Request: RT-37788

2014-07-02 Thread Stephen F Northover
I'm glad your not an objective C. I suggest getting Felipe to look at this code too should we decide to go ahead with it. Steve On 2014-07-02, 5:15 PM, Danno Ferrin wrote: Chris, Kevin, Steve, Please review this fix for RT-37788. Since I am not an objective C any comments are welcome. Als

Review Request: RT-37788

2014-07-02 Thread Danno Ferrin
Chris, Kevin, Steve, Please review this fix for RT-37788. Since I am not an objective C any comments are welcome. Also, please consider if this is too much for an 8u20 fix (the diff is against the current 8u40 codebase). Webrev: http://cr.openjdk.java.net/~shemnon/RT-37788/webrev.00/ JIRA: ht