[classlib][awt] Revision #465514 broke image decoders.

[Oleg Khaschansky <ol...@gmail.com>]

Hi,

Rev. 465514 introduced a lot of invalid modifications to the
GifDecoder, PngDecoder and JpegDecoder. There were a number of fields
modified or initialized from the native code only, but they were
redeclared as final, so the decoders doesn't work properly any more.

This revision has the following comment:

Cleanup code
* Add if/else braces
* Add missing annotations
* Add type variables
* Use foreach loops
* etc

I'd suggest to roll back this revision and redo the cleanup in the
more accurate way.

Thanks,
  Oleg

Re: [classlib][awt] Revision #465514 broke image decoders.

[Oleg Khaschansky <ol...@gmail.com>]

> It'd be better to have Hashtable<?,?> as a type in all 3 classes and
> Hashtable<Object,Object> as an initial value for this field.

I mean that it is not an error but it seems to me semantically more meaningful

On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> Nathan, could you, please tell why you changed the field properties in
> these classes to
> Hashtable<Object,Object> in two of them and to
> Hashtable<String,String> in one of them (GifDecoder)?
>
> Look at the declaration in the ImageConsumer class:
> void setProperties(Hashtable<?,?> props)
>
> It'd be better to have Hashtable<?,?> as a type in all 3 classes and
> Hashtable<Object,Object> as an initial value for this field.
>
> On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> > > > Where are the tests for these decoders? How did you determine that
> > > > they no longer worked?
> >
> > Unfortunately, these classes are not covered with the unit tests.
> >
> > I was running a simple test application that did something like this:
> > Toolkit.getDefaultToolkit().getImage("image.jpg");
> > and if failed with a NPE.
> >
> > > I removed the final modifiers
> > At the first glance it seems like the problem doesn't appear any more.
> >
> > > There were only 3-4 other fields that were finalized. Your email
> > > mentioned "a lot of invalid modifications"; what are the other issues,
> > > specifically?
> > Only final fields. No other issues. But 3-4 in 3 classes - it is alot for me :)
> >
> > On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> > > I removed the final modifiers; this only affected PngDecoder,
> > > GifDecoder and JpegDecoder. I missed the comments in the fields of
> > > JpegDecoder, that was my mistake.
> > >
> > > There were only 3-4 other fields that were finalized. Your email
> > > mentioned "a lot of invalid modifications"; what are the other issues,
> > > specifically?
> > >
> > > -Nathan
> > >
> > > On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> > > > Where are the tests for these decoders? How did you determine that
> > > > they no longer worked?
> > > >
> > > > I'll remove the final modifiers.
> > > >
> > > > -Nathan
> > > >
> > > > On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> > > > > Hi,
> > > > >
> > > > > Rev. 465514 introduced a lot of invalid modifications to the
> > > > > GifDecoder, PngDecoder and JpegDecoder. There were a number of fields
> > > > > modified or initialized from the native code only, but they were
> > > > > redeclared as final, so the decoders doesn't work properly any more.
> > > > >
> > > > > This revision has the following comment:
> > > > >
> > > > > Cleanup code
> > > > > * Add if/else braces
> > > > > * Add missing annotations
> > > > > * Add type variables
> > > > > * Use foreach loops
> > > > > * etc
> > > > >
> > > > > I'd suggest to roll back this revision and redo the cleanup in the
> > > > > more accurate way.
> > > > >
> > > > > Thanks,
> > > > >   Oleg
> > > > >
> > > >
> > >
> >
>

Re: [classlib][awt] Revision #465514 broke image decoders.

[Oleg Khaschansky <ol...@gmail.com>]

> Since the ImageConsumer accepts Hashtable<?,?> my thought is that the
> the GifDecoder should declare the field based on what it actually
> uses. If the other decoders only use String keys and String values,
> then I would suggest we change the field declaration to match.

Yes, it sounds reasonable.

On 10/25/06, Nathan Beyer <nb...@gmail.com> wrote:
> On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> > Nathan, could you, please tell why you changed the field properties in
> > these classes to
> > Hashtable<Object,Object> in two of them and to
> > Hashtable<String,String> in one of them (GifDecoder)?
> >
> > Look at the declaration in the ImageConsumer class:
> > void setProperties(Hashtable<?,?> props)
> >
> > It'd be better to have Hashtable<?,?> as a type in all 3 classes and
> > Hashtable<Object,Object> as an initial value for this field.
>
> A field type of Hashtable<?,?> would be painful, as you can't perform
> any sets when a type variable is ?. You'd have to cast it to
> Hashtable<Object,Object> before performing any sets.
>
> Since the ImageConsumer accepts Hashtable<?,?> my thought is that the
> the GifDecoder should declare the field based on what it actually
> uses. If the other decoders only use String keys and String values,
> then I would suggest we change the field declaration to match.
>
> -Nathan
>
> >
> > On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> > > > > Where are the tests for these decoders? How did you determine that
> > > > > they no longer worked?
> > >
> > > Unfortunately, these classes are not covered with the unit tests.
> > >
> > > I was running a simple test application that did something like this:
> > > Toolkit.getDefaultToolkit().getImage("image.jpg");
> > > and if failed with a NPE.
> > >
> > > > I removed the final modifiers
> > > At the first glance it seems like the problem doesn't appear any more.
> > >
> > > > There were only 3-4 other fields that were finalized. Your email
> > > > mentioned "a lot of invalid modifications"; what are the other issues,
> > > > specifically?
> > > Only final fields. No other issues. But 3-4 in 3 classes - it is alot for me :)
> > >
> > > On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> > > > I removed the final modifiers; this only affected PngDecoder,
> > > > GifDecoder and JpegDecoder. I missed the comments in the fields of
> > > > JpegDecoder, that was my mistake.
> > > >
> > > > There were only 3-4 other fields that were finalized. Your email
> > > > mentioned "a lot of invalid modifications"; what are the other issues,
> > > > specifically?
> > > >
> > > > -Nathan
> > > >
> > > > On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> > > > > Where are the tests for these decoders? How did you determine that
> > > > > they no longer worked?
> > > > >
> > > > > I'll remove the final modifiers.
> > > > >
> > > > > -Nathan
> > > > >
> > > > > On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> > > > > > Hi,
> > > > > >
> > > > > > Rev. 465514 introduced a lot of invalid modifications to the
> > > > > > GifDecoder, PngDecoder and JpegDecoder. There were a number of fields
> > > > > > modified or initialized from the native code only, but they were
> > > > > > redeclared as final, so the decoders doesn't work properly any more.
> > > > > >
> > > > > > This revision has the following comment:
> > > > > >
> > > > > > Cleanup code
> > > > > > * Add if/else braces
> > > > > > * Add missing annotations
> > > > > > * Add type variables
> > > > > > * Use foreach loops
> > > > > > * etc
> > > > > >
> > > > > > I'd suggest to roll back this revision and redo the cleanup in the
> > > > > > more accurate way.
> > > > > >
> > > > > > Thanks,
> > > > > >   Oleg
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [classlib][awt] Revision #465514 broke image decoders.

[Nathan Beyer <nb...@gmail.com>]

On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> Nathan, could you, please tell why you changed the field properties in
> these classes to
> Hashtable<Object,Object> in two of them and to
> Hashtable<String,String> in one of them (GifDecoder)?
>
> Look at the declaration in the ImageConsumer class:
> void setProperties(Hashtable<?,?> props)
>
> It'd be better to have Hashtable<?,?> as a type in all 3 classes and
> Hashtable<Object,Object> as an initial value for this field.

A field type of Hashtable<?,?> would be painful, as you can't perform
any sets when a type variable is ?. You'd have to cast it to
Hashtable<Object,Object> before performing any sets.

Since the ImageConsumer accepts Hashtable<?,?> my thought is that the
the GifDecoder should declare the field based on what it actually
uses. If the other decoders only use String keys and String values,
then I would suggest we change the field declaration to match.

-Nathan

>
> On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> > > > Where are the tests for these decoders? How did you determine that
> > > > they no longer worked?
> >
> > Unfortunately, these classes are not covered with the unit tests.
> >
> > I was running a simple test application that did something like this:
> > Toolkit.getDefaultToolkit().getImage("image.jpg");
> > and if failed with a NPE.
> >
> > > I removed the final modifiers
> > At the first glance it seems like the problem doesn't appear any more.
> >
> > > There were only 3-4 other fields that were finalized. Your email
> > > mentioned "a lot of invalid modifications"; what are the other issues,
> > > specifically?
> > Only final fields. No other issues. But 3-4 in 3 classes - it is alot for me :)
> >
> > On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> > > I removed the final modifiers; this only affected PngDecoder,
> > > GifDecoder and JpegDecoder. I missed the comments in the fields of
> > > JpegDecoder, that was my mistake.
> > >
> > > There were only 3-4 other fields that were finalized. Your email
> > > mentioned "a lot of invalid modifications"; what are the other issues,
> > > specifically?
> > >
> > > -Nathan
> > >
> > > On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> > > > Where are the tests for these decoders? How did you determine that
> > > > they no longer worked?
> > > >
> > > > I'll remove the final modifiers.
> > > >
> > > > -Nathan
> > > >
> > > > On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> > > > > Hi,
> > > > >
> > > > > Rev. 465514 introduced a lot of invalid modifications to the
> > > > > GifDecoder, PngDecoder and JpegDecoder. There were a number of fields
> > > > > modified or initialized from the native code only, but they were
> > > > > redeclared as final, so the decoders doesn't work properly any more.
> > > > >
> > > > > This revision has the following comment:
> > > > >
> > > > > Cleanup code
> > > > > * Add if/else braces
> > > > > * Add missing annotations
> > > > > * Add type variables
> > > > > * Use foreach loops
> > > > > * etc
> > > > >
> > > > > I'd suggest to roll back this revision and redo the cleanup in the
> > > > > more accurate way.
> > > > >
> > > > > Thanks,
> > > > >   Oleg
> > > > >
> > > >
> > >
> >
>

Re: [classlib][awt] Revision #465514 broke image decoders.

[Oleg Khaschansky <ol...@gmail.com>]

Nathan, could you, please tell why you changed the field properties in
these classes to
Hashtable<Object,Object> in two of them and to
Hashtable<String,String> in one of them (GifDecoder)?

Look at the declaration in the ImageConsumer class:
void setProperties(Hashtable<?,?> props)

It'd be better to have Hashtable<?,?> as a type in all 3 classes and
Hashtable<Object,Object> as an initial value for this field.

On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> > > Where are the tests for these decoders? How did you determine that
> > > they no longer worked?
>
> Unfortunately, these classes are not covered with the unit tests.
>
> I was running a simple test application that did something like this:
> Toolkit.getDefaultToolkit().getImage("image.jpg");
> and if failed with a NPE.
>
> > I removed the final modifiers
> At the first glance it seems like the problem doesn't appear any more.
>
> > There were only 3-4 other fields that were finalized. Your email
> > mentioned "a lot of invalid modifications"; what are the other issues,
> > specifically?
> Only final fields. No other issues. But 3-4 in 3 classes - it is alot for me :)
>
> On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> > I removed the final modifiers; this only affected PngDecoder,
> > GifDecoder and JpegDecoder. I missed the comments in the fields of
> > JpegDecoder, that was my mistake.
> >
> > There were only 3-4 other fields that were finalized. Your email
> > mentioned "a lot of invalid modifications"; what are the other issues,
> > specifically?
> >
> > -Nathan
> >
> > On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> > > Where are the tests for these decoders? How did you determine that
> > > they no longer worked?
> > >
> > > I'll remove the final modifiers.
> > >
> > > -Nathan
> > >
> > > On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> > > > Hi,
> > > >
> > > > Rev. 465514 introduced a lot of invalid modifications to the
> > > > GifDecoder, PngDecoder and JpegDecoder. There were a number of fields
> > > > modified or initialized from the native code only, but they were
> > > > redeclared as final, so the decoders doesn't work properly any more.
> > > >
> > > > This revision has the following comment:
> > > >
> > > > Cleanup code
> > > > * Add if/else braces
> > > > * Add missing annotations
> > > > * Add type variables
> > > > * Use foreach loops
> > > > * etc
> > > >
> > > > I'd suggest to roll back this revision and redo the cleanup in the
> > > > more accurate way.
> > > >
> > > > Thanks,
> > > >   Oleg
> > > >
> > >
> >
>

Re: [classlib][awt] Revision #465514 broke image decoders.

["Geir Magnusson Jr." <ge...@pobox.com>]

Oleg Khaschansky wrote:
>> That's the start of a unit test, are you going to finish it? ;-)
> Well, I'll look into this. We need to put a couple of images for this
> kind of test somewhere...

How about a picture of you? :)

> 
>> But not a lot compared to the number of cleanup changes Nathan has been
>> busy making!
> Right, I am sorry. This cleanup is a great work. I suggested to roll
> back because I thought that there could be other places with similar
> issues that could have been missed. AWT has a lot of native code, so
> refactorings should be done with this in mind. The test coverage for
> AWT is quite poor, in comparison with the undelying functionality,
> it's quite easy to introduce a new bug in it.
> 
> On 10/24/06, Mark Hindess <ma...@googlemail.com> wrote:
>>
>> On 24 October 2006 at 18:25, "Oleg Khaschansky" 
>> <ol...@gmail.com> wrote:
>> > > > Where are the tests for these decoders? How did you determine that
>> > > > they no longer worked?
>> >
>> > Unfortunately, these classes are not covered with the unit tests.
>> >
>> > I was running a simple test application that did something like this:
>> > Toolkit.getDefaultToolkit().getImage("image.jpg");
>> > and if failed with a NPE.
>>
>> That's the start of a unit test, are you going to finish it? ;-)
>>
>> > > I removed the final modifiers
>> > At the first glance it seems like the problem doesn't appear any more.
>> >
>> > > There were only 3-4 other fields that were finalized. Your email
>> > > mentioned "a lot of invalid modifications"; what are the other 
>> issues,
>> > > specifically?
>> > Only final fields. No other issues. But 3-4 in 3 classes - it is 
>> alot for me
>> > :)
>>
>> But not a lot compared to the number of cleanup changes Nathan has been
>> busy making!
>>
>> Regards,
>>  Mark.
>>
>> > On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
>> > > I removed the final modifiers; this only affected PngDecoder,
>> > > GifDecoder and JpegDecoder. I missed the comments in the fields of
>> > > JpegDecoder, that was my mistake.
>> > >
>> > > There were only 3-4 other fields that were finalized. Your email
>> > > mentioned "a lot of invalid modifications"; what are the other 
>> issues,
>> > > specifically?
>> > >
>> > > -Nathan
>> > >
>> > > On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
>> > > > Where are the tests for these decoders? How did you determine that
>> > > > they no longer worked?
>> > > >
>> > > > I'll remove the final modifiers.
>> > > >
>> > > > -Nathan
>> > > >
>> > > > On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
>> > > > > Hi,
>> > > > >
>> > > > > Rev. 465514 introduced a lot of invalid modifications to the
>> > > > > GifDecoder, PngDecoder and JpegDecoder. There were a number of 
>> fields
>> > > > > modified or initialized from the native code only, but they were
>> > > > > redeclared as final, so the decoders doesn't work properly any 
>> more.
>> > > > >
>> > > > > This revision has the following comment:
>> > > > >
>> > > > > Cleanup code
>> > > > > * Add if/else braces
>> > > > > * Add missing annotations
>> > > > > * Add type variables
>> > > > > * Use foreach loops
>> > > > > * etc
>> > > > >
>> > > > > I'd suggest to roll back this revision and redo the cleanup in 
>> the
>> > > > > more accurate way.
>> > > > >
>> > > > > Thanks,
>> > > > >   Oleg
>> > > > >
>> > > >
>> > >
>> >
>>
>>
>>

Re: [classlib][awt] Revision #465514 broke image decoders.

[Oleg Khaschansky <ol...@gmail.com>]

Unit tests for the decoders are up there in [1]

[1] https://issues.apache.org/jira/browse/HARMONY-1954

On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> > That's the start of a unit test, are you going to finish it? ;-)
> Well, I'll look into this. We need to put a couple of images for this
> kind of test somewhere...
>
> > But not a lot compared to the number of cleanup changes Nathan has been
> > busy making!
> Right, I am sorry. This cleanup is a great work. I suggested to roll
> back because I thought that there could be other places with similar
> issues that could have been missed. AWT has a lot of native code, so
> refactorings should be done with this in mind. The test coverage for
> AWT is quite poor, in comparison with the undelying functionality,
> it's quite easy to introduce a new bug in it.
>
> On 10/24/06, Mark Hindess <ma...@googlemail.com> wrote:
> >
> > On 24 October 2006 at 18:25, "Oleg Khaschansky" <ol...@gmail.com> wrote:
> > > > > Where are the tests for these decoders? How did you determine that
> > > > > they no longer worked?
> > >
> > > Unfortunately, these classes are not covered with the unit tests.
> > >
> > > I was running a simple test application that did something like this:
> > > Toolkit.getDefaultToolkit().getImage("image.jpg");
> > > and if failed with a NPE.
> >
> > That's the start of a unit test, are you going to finish it? ;-)
> >
> > > > I removed the final modifiers
> > > At the first glance it seems like the problem doesn't appear any more.
> > >
> > > > There were only 3-4 other fields that were finalized. Your email
> > > > mentioned "a lot of invalid modifications"; what are the other issues,
> > > > specifically?
> > > Only final fields. No other issues. But 3-4 in 3 classes - it is alot for me
> > > :)
> >
> > But not a lot compared to the number of cleanup changes Nathan has been
> > busy making!
> >
> > Regards,
> >  Mark.
> >
> > > On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> > > > I removed the final modifiers; this only affected PngDecoder,
> > > > GifDecoder and JpegDecoder. I missed the comments in the fields of
> > > > JpegDecoder, that was my mistake.
> > > >
> > > > There were only 3-4 other fields that were finalized. Your email
> > > > mentioned "a lot of invalid modifications"; what are the other issues,
> > > > specifically?
> > > >
> > > > -Nathan
> > > >
> > > > On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> > > > > Where are the tests for these decoders? How did you determine that
> > > > > they no longer worked?
> > > > >
> > > > > I'll remove the final modifiers.
> > > > >
> > > > > -Nathan
> > > > >
> > > > > On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> > > > > > Hi,
> > > > > >
> > > > > > Rev. 465514 introduced a lot of invalid modifications to the
> > > > > > GifDecoder, PngDecoder and JpegDecoder. There were a number of fields
> > > > > > modified or initialized from the native code only, but they were
> > > > > > redeclared as final, so the decoders doesn't work properly any more.
> > > > > >
> > > > > > This revision has the following comment:
> > > > > >
> > > > > > Cleanup code
> > > > > > * Add if/else braces
> > > > > > * Add missing annotations
> > > > > > * Add type variables
> > > > > > * Use foreach loops
> > > > > > * etc
> > > > > >
> > > > > > I'd suggest to roll back this revision and redo the cleanup in the
> > > > > > more accurate way.
> > > > > >
> > > > > > Thanks,
> > > > > >   Oleg
> > > > > >
> > > > >
> > > >
> > >
> >
> >
> >
>

Re: [classlib][awt] Revision #465514 broke image decoders.

[Oleg Khaschansky <ol...@gmail.com>]

> That's the start of a unit test, are you going to finish it? ;-)
Well, I'll look into this. We need to put a couple of images for this
kind of test somewhere...

> But not a lot compared to the number of cleanup changes Nathan has been
> busy making!
Right, I am sorry. This cleanup is a great work. I suggested to roll
back because I thought that there could be other places with similar
issues that could have been missed. AWT has a lot of native code, so
refactorings should be done with this in mind. The test coverage for
AWT is quite poor, in comparison with the undelying functionality,
it's quite easy to introduce a new bug in it.

On 10/24/06, Mark Hindess <ma...@googlemail.com> wrote:
>
> On 24 October 2006 at 18:25, "Oleg Khaschansky" <ol...@gmail.com> wrote:
> > > > Where are the tests for these decoders? How did you determine that
> > > > they no longer worked?
> >
> > Unfortunately, these classes are not covered with the unit tests.
> >
> > I was running a simple test application that did something like this:
> > Toolkit.getDefaultToolkit().getImage("image.jpg");
> > and if failed with a NPE.
>
> That's the start of a unit test, are you going to finish it? ;-)
>
> > > I removed the final modifiers
> > At the first glance it seems like the problem doesn't appear any more.
> >
> > > There were only 3-4 other fields that were finalized. Your email
> > > mentioned "a lot of invalid modifications"; what are the other issues,
> > > specifically?
> > Only final fields. No other issues. But 3-4 in 3 classes - it is alot for me
> > :)
>
> But not a lot compared to the number of cleanup changes Nathan has been
> busy making!
>
> Regards,
>  Mark.
>
> > On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> > > I removed the final modifiers; this only affected PngDecoder,
> > > GifDecoder and JpegDecoder. I missed the comments in the fields of
> > > JpegDecoder, that was my mistake.
> > >
> > > There were only 3-4 other fields that were finalized. Your email
> > > mentioned "a lot of invalid modifications"; what are the other issues,
> > > specifically?
> > >
> > > -Nathan
> > >
> > > On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> > > > Where are the tests for these decoders? How did you determine that
> > > > they no longer worked?
> > > >
> > > > I'll remove the final modifiers.
> > > >
> > > > -Nathan
> > > >
> > > > On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> > > > > Hi,
> > > > >
> > > > > Rev. 465514 introduced a lot of invalid modifications to the
> > > > > GifDecoder, PngDecoder and JpegDecoder. There were a number of fields
> > > > > modified or initialized from the native code only, but they were
> > > > > redeclared as final, so the decoders doesn't work properly any more.
> > > > >
> > > > > This revision has the following comment:
> > > > >
> > > > > Cleanup code
> > > > > * Add if/else braces
> > > > > * Add missing annotations
> > > > > * Add type variables
> > > > > * Use foreach loops
> > > > > * etc
> > > > >
> > > > > I'd suggest to roll back this revision and redo the cleanup in the
> > > > > more accurate way.
> > > > >
> > > > > Thanks,
> > > > >   Oleg
> > > > >
> > > >
> > >
> >
>
>
>

Re: [classlib][awt] Revision #465514 broke image decoders.

[Mark Hindess <ma...@googlemail.com>]

On 24 October 2006 at 18:25, "Oleg Khaschansky" <ol...@gmail.com> wrote:
> > > Where are the tests for these decoders? How did you determine that
> > > they no longer worked?
> 
> Unfortunately, these classes are not covered with the unit tests.
> 
> I was running a simple test application that did something like this:
> Toolkit.getDefaultToolkit().getImage("image.jpg");
> and if failed with a NPE.

That's the start of a unit test, are you going to finish it? ;-)

> > I removed the final modifiers
> At the first glance it seems like the problem doesn't appear any more.
> 
> > There were only 3-4 other fields that were finalized. Your email
> > mentioned "a lot of invalid modifications"; what are the other issues,
> > specifically?
> Only final fields. No other issues. But 3-4 in 3 classes - it is alot for me 
> :)

But not a lot compared to the number of cleanup changes Nathan has been
busy making!

Regards,
 Mark.

> On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> > I removed the final modifiers; this only affected PngDecoder,
> > GifDecoder and JpegDecoder. I missed the comments in the fields of
> > JpegDecoder, that was my mistake.
> >
> > There were only 3-4 other fields that were finalized. Your email
> > mentioned "a lot of invalid modifications"; what are the other issues,
> > specifically?
> >
> > -Nathan
> >
> > On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> > > Where are the tests for these decoders? How did you determine that
> > > they no longer worked?
> > >
> > > I'll remove the final modifiers.
> > >
> > > -Nathan
> > >
> > > On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> > > > Hi,
> > > >
> > > > Rev. 465514 introduced a lot of invalid modifications to the
> > > > GifDecoder, PngDecoder and JpegDecoder. There were a number of fields
> > > > modified or initialized from the native code only, but they were
> > > > redeclared as final, so the decoders doesn't work properly any more.
> > > >
> > > > This revision has the following comment:
> > > >
> > > > Cleanup code
> > > > * Add if/else braces
> > > > * Add missing annotations
> > > > * Add type variables
> > > > * Use foreach loops
> > > > * etc
> > > >
> > > > I'd suggest to roll back this revision and redo the cleanup in the
> > > > more accurate way.
> > > >
> > > > Thanks,
> > > >   Oleg
> > > >
> > >
> >
> 

Re: [classlib][awt] Revision #465514 broke image decoders.

[Oleg Khaschansky <ol...@gmail.com>]

> > Where are the tests for these decoders? How did you determine that
> > they no longer worked?

Unfortunately, these classes are not covered with the unit tests.

I was running a simple test application that did something like this:
Toolkit.getDefaultToolkit().getImage("image.jpg");
and if failed with a NPE.

> I removed the final modifiers
At the first glance it seems like the problem doesn't appear any more.

> There were only 3-4 other fields that were finalized. Your email
> mentioned "a lot of invalid modifications"; what are the other issues,
> specifically?
Only final fields. No other issues. But 3-4 in 3 classes - it is alot for me :)

On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> I removed the final modifiers; this only affected PngDecoder,
> GifDecoder and JpegDecoder. I missed the comments in the fields of
> JpegDecoder, that was my mistake.
>
> There were only 3-4 other fields that were finalized. Your email
> mentioned "a lot of invalid modifications"; what are the other issues,
> specifically?
>
> -Nathan
>
> On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> > Where are the tests for these decoders? How did you determine that
> > they no longer worked?
> >
> > I'll remove the final modifiers.
> >
> > -Nathan
> >
> > On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> > > Hi,
> > >
> > > Rev. 465514 introduced a lot of invalid modifications to the
> > > GifDecoder, PngDecoder and JpegDecoder. There were a number of fields
> > > modified or initialized from the native code only, but they were
> > > redeclared as final, so the decoders doesn't work properly any more.
> > >
> > > This revision has the following comment:
> > >
> > > Cleanup code
> > > * Add if/else braces
> > > * Add missing annotations
> > > * Add type variables
> > > * Use foreach loops
> > > * etc
> > >
> > > I'd suggest to roll back this revision and redo the cleanup in the
> > > more accurate way.
> > >
> > > Thanks,
> > >   Oleg
> > >
> >
>

Re: [classlib][awt] Revision #465514 broke image decoders.

[Nathan Beyer <nb...@gmail.com>]

I removed the final modifiers; this only affected PngDecoder,
GifDecoder and JpegDecoder. I missed the comments in the fields of
JpegDecoder, that was my mistake.

There were only 3-4 other fields that were finalized. Your email
mentioned "a lot of invalid modifications"; what are the other issues,
specifically?

-Nathan

On 10/24/06, Nathan Beyer <nb...@gmail.com> wrote:
> Where are the tests for these decoders? How did you determine that
> they no longer worked?
>
> I'll remove the final modifiers.
>
> -Nathan
>
> On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> > Hi,
> >
> > Rev. 465514 introduced a lot of invalid modifications to the
> > GifDecoder, PngDecoder and JpegDecoder. There were a number of fields
> > modified or initialized from the native code only, but they were
> > redeclared as final, so the decoders doesn't work properly any more.
> >
> > This revision has the following comment:
> >
> > Cleanup code
> > * Add if/else braces
> > * Add missing annotations
> > * Add type variables
> > * Use foreach loops
> > * etc
> >
> > I'd suggest to roll back this revision and redo the cleanup in the
> > more accurate way.
> >
> > Thanks,
> >   Oleg
> >
>

Re: [classlib][awt] Revision #465514 broke image decoders.

[Nathan Beyer <nb...@gmail.com>]

Where are the tests for these decoders? How did you determine that
they no longer worked?

I'll remove the final modifiers.

-Nathan

On 10/24/06, Oleg Khaschansky <ol...@gmail.com> wrote:
> Hi,
>
> Rev. 465514 introduced a lot of invalid modifications to the
> GifDecoder, PngDecoder and JpegDecoder. There were a number of fields
> modified or initialized from the native code only, but they were
> redeclared as final, so the decoders doesn't work properly any more.
>
> This revision has the following comment:
>
> Cleanup code
> * Add if/else braces
> * Add missing annotations
> * Add type variables
> * Use foreach loops
> * etc
>
> I'd suggest to roll back this revision and redo the cleanup in the
> more accurate way.
>
> Thanks,
>   Oleg
>