<AWT Dev> [9] Review Request: 6815345 java.awt.Component.createImage(int width, int height) should remove behavioral optionality

[Semyon Sadetsky]

On 9/24/2015 7:42 PM, Sergey Bylokhov wrote:
> On 24.09.15 18:38, Semyon Sadetsky wrote:
>>
>>
>> On 9/24/2015 4:58 PM, Sergey Bylokhov wrote:
>>> The new version of the fix:
>>> http://cr.openjdk.java.net/~serb/6815345/webrev.05
>> You assumption can be broken with the next
>>
>> AWTAccessor.getComponentAccessor().setPeer(jbutton1,
>>                  new ButtonPeer() {
>>                      ...
>>                      @Override
>>                      public Image createImage(int width, int height) {
>>                          return new BufferedImage(width, height,
>> BufferedImage.TYPE_INT_RGB);
>>                      }
>>                      ...
>>                  });
>
> AWTAccessor is unspecified thing, it is a safer version of reflection 
> which can break lots of things if used incorrectly. This api is for 
> internal use only.
>
>>
>> Maybe it would be better to write about peer=null in the method spec?
>
> peer is a private field it cannot be mentioned in the specification.
>
> Or
>> not to be that direct and leave the initial phrase mentioning 
>> isHeadless()?
>
  I read this article: 
http://www.oracle.com/technetwork/articles/javase/headless-136834.html.
As I understand peer is not strictly disabled for the headless mode, 
because some native resources like fonts, printing and images can be 
used. In headless mode peer#getImage()/getVolatileImage() can be used to 
construct native images, so they must not return null. Maybe 
Component#getImage() implementation should simply check for isHeadless() 
and return null?

You specified isHeadless()=true that describes the whole environment. 
Why isHeadlessInstance()=true is not mentioned? What if the whole 
environment is not headless but this component's graphics is headless?

And more global question: Why should we disable the creating of 
component image buffer for the headless mode? It could be used for the 
same performance reason as in non-headless.
>
>>>
>>> The headless case is covered, heavyweight components cannot be created
>>> in such mode, so only lightweight buttons are checked.
>>>
>>> On 24.09.15 15:58, Semyon Sadetsky wrote:
>>>>
>>>>
>>>> On 9/24/2015 3:25 PM, Sergey Bylokhov wrote:
>>>>> On 24.09.15 11:36, Semyon Sadetsky wrote:
>>>>>> Hi Sergey,
>>>>>>
>>>>>>   isHeadless()=true must return null. If so please add the
>>>>>> corresponding
>>>>>> test case. It is not not obvious from the code.
>>>>>
>>>>> isHeadless()=true is headless mode where the frames are always not
>>>>> displayable, so everything is similar to the current test except that
>>>>> in headless the pack() will be throw an exception and second part of
>>>>> the test in this mode is unnecessary.
>>>> Then maybe simply do not call pack() for the headless test?
>>>> Okay, let me rephrase what I meant. Since isHeadless()=true case is
>>>> mentioned in those 3 specs so explicitly it must be guarantied that 
>>>> the
>>>> specified behavior works for the case as described.  I cannot trace 
>>>> the
>>>> result by reading the code of the createImage(), isHeadless() 
>>>> method is
>>>> not even called there.  So the test case should be added. Or you could
>>>> remove isHeadless() references from the specs. Or write something like
>>>> "the result is non-deterministic if isHeadless() is not false..."
>>>>>
>>>>>>
>>>>>> --Semyon
>>>>>>
>>>>>> On 9/23/2015 9:14 PM, Sergey Bylokhov wrote:
>>>>>>> Hello.
>>>>>>> Please review the fix for jdk9.
>>>>>>> The specification is updated as suggested in JDK-6186530 and
>>>>>>> JDK-6815345. The test is added to prove that we always return null
>>>>>>> when the component is not displayable.
>>>>>>>
>>>>>>> ccc request will be created after the technical review. One
>>>>>>> additional
>>>>>>> bug filed https://bugs.openjdk.java.net/browse/JDK-8137047
>>>>>>>
>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-6815345
>>>>>>> Webrev can be found at:
>>>>>>> http://cr.openjdk.java.net/~serb/6815345/webrev.04
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>