<Swing Dev> [9] Review request for 8078268: javax.swing.text.html.parser.Parser parseScript incorrectly optimized

[Alexey Ivanov]

Hi Mikhail,

Source code changes look good now.

Please also see my comments below.

On 11.05.2016 23:11, mikhail cherkasov wrote:
> Please see update version:
> http://cr.openjdk.java.net/~mcherkas/8078268/webrev.02
> >You don't need to create JEditorPane, you can instantiate 
> HTMLDocument directly with new HTMLDocument().
> without JEditorPane parser will be null in HTMLDocument, so it's the 
> easiest way to init HTMLDocument right

I see.

Probably using HTMLEditorKit.read(...) method would be better?

HTMLEditorKit htmlKit = new HTMLEditorKit();
Document doc = htmlKit.createDefaultDocument();
htmlKit.read(new FileReader(...), doc, 0);

This way the code looks cleaner, in my opinion. And it resembles the way 
a real application would use to parse the document. What do you think?

>
>> Hi Mikhail,
>>
>> Can I ask you to add one space between if, while and the opening 
>> parenthesis which is in according to Java Coding Style?
This also applies to the test code.
>>
>> In your test, you use none of JFrame methods, then why do you extend 
>> JFrame?
The updated version still extends JFrame but uses none of its methods.
>>
>> You don't need to create JEditorPane, you can instantiate 
>> HTMLDocument directly with new HTMLDocument().
>>
>> An exception thrown from Runnable in invokeLater won't stop the test, 
>> and the test could fail because of the timeout. I suggest storing 
>> IOException to a static volatile field in the catch block. If the 
>> exception is not null, the main thread breaks from waiting 'while' 
>> and rethrows the exception.
An exception thrown in invokeLater would still not fail the test.


Regards,
Alexey
>>
>> Regards,
>> Alexey
>>
>> On 11.05.2016 17:20, mikhail cherkasov wrote:
>>> Hi all,
>>>
>>> Please review a fix for
>>> https://bugs.openjdk.java.net/browse/JDK-8078268
>>> webrev:
>>> http://cr.openjdk.java.net/~mcherkas/8078268/webrev.01/
>>>
>>> The problem is in the following line:
>>> http://hg.openjdk.java.net/jdk9/client/jdk/file/597626072716/src/java.desktop/share/classes/javax/swing/text/html/parser/Parser.java#l2127 
>>>
>>> for each new char it creates a string with a whole text inside the 
>>> script tag, so it's very expensive operation.
>>> I added check for <!-- and -->  by reading chars from input stream 
>>> like it was done for end script tag.
>>>
>>> Thanks,
>>> Mikhail.
>>
>