RFR [15/java.xml] 8238183: SAX2StAXStreamWriter cannot deal with comments prior to the root element

[Joe Wang]

On 3/30/20 3:12 PM, naoto.sato at oracle.com wrote:
> Hi Joe,
>
> Much better and cleaner! One nit is that you could remove "docLocator 
> != null &&" as instanceof checks null.

Indeed, null check is removed:
http://cr.openjdk.java.net/~joehw/jdk15/8238183/webrev_03/

Thanks,
Joe

>
> Naoto
>
> On 3/30/20 2:56 PM, Joe Wang wrote:
>> Hi Naoto,
>>
>> Thanks for the review!
>>
>> I refactored the code around getting the XML version and encoding 
>> from the locator to get rid of the CCE. The impls with EventWriter 
>> and StreamWriter share a base class. All three classes are therefore 
>> refactored. No material change to the EventWriter impl. Two fields, 
>> xmlVersion and encoding, are added since I expect they will be needed 
>> later when we work on fixing the declaration. Note that, as of the 
>> current impl, encoding is not referenced in the StreamWriter impl, 
>> which is part of the issue in transforming the declaration 
>> (JDK-8241711).
>>
>> Here's webrev version 2:
>> http://cr.openjdk.java.net/~joehw/jdk15/8238183/webrev_02/
>>
>> -Joe
>>
>> On 3/30/20 11:23 AM, naoto.sato at oracle.com wrote:
>>> Hi Joe,
>>>
>>> Can writeStartDocument() be simplified by checking "docLocator 
>>> instanceof Locator2"? This way it won't need to catch CCE and issue 
>>> no-arg version.
>>>
>>> Otherwise looks good to me.
>>>
>>> Naoto
>>>
>>> On 3/30/20 11:02 AM, Joe Wang wrote:
>>>> Hi,
>>>>
>>>> Please review a fix for the StAXResult impl. The issue was that it 
>>>> output comment prior to the declaration, resulting in an invalid 
>>>> XML document. The patch focuses on fixing this issue, but it does 
>>>> not cover other issues the StAXResult impl may have (e.g. 
>>>> JDK-8241711).
>>>>
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8238183
>>>> webrev: http://cr.openjdk.java.net/~joehw/jdk15/8238183/webrev/
>>>>
>>>> Thanks,
>>>> Joe
>>>>
>>