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

[Semyon Sadetsky]

On 9/24/2015 11:52 PM, Sergey Bylokhov wrote:
> On 24.09.15 20:16, Semyon Sadetsky wrote:
>>
>> 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,
>
> The native peers are disabled in such configurations.
Native container based components are disabled. But other native 
resources can be used if available.
>> 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.
>
> The peers is just some internal utility classes, they can return 
> anything they want. But if we pass/return results of the call to the 
> peer, we should follow the public specification. In this case the 
> public spec states that we should return null in headless, so all 
> peers which are used in headless (like NullComponentPeer) should 
> return null. And in the same time all hw peers should return non-null 
> value in heedful mode.
All **Peer interfaces are public and external. Peer is not a hidden or 
implementation specific term. It isn't allowed to application developers 
for implementation, but it is used for porting client libs to other 
platforms. Java specification has wider vision than one particular 
implementation. And for porting the headless mode is very sensitive 
topic because specific platforms are headless often.
>
> Maybe
>> Component#getImage() implementation should simply check for isHeadless()
>> and return null?
>>
>> You specified isHeadless()=true that describes the whole environment.
>
>
> This is not me. Even before the fix the specification of these methods 
> mention that they always return null in the headless environment. This 
> fix change the first sentence about displayable components(according 
> to the current behavior) and also synchronize the spec between 
> createVolatileImage(also according current behavior) methods.
>
>
>> Why isHeadlessInstance()=true is not mentioned?What if the whole
>> environment is not headless but this component's graphics is headless?
>
> I guess this is because the first method was added first.
But what will happen if Unix DISPLAY variable is not set so 
isHeadlessInstance()=true?  Will peer be allowed? Should component 
buffering work smoothly?
Note this is a common network server scenario which runs Java app from 
the text console. I cannot get from the spec is it completely different 
headless mode or it is the same as the global 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.
>
> in the headless mode we cannot create most of the hw components, 
> actually non of top level window can be created. And for lw components 
> like in swing the one global buffer is used in the RepaintManager.
One global buffer means one global repaint off all components for any 
single component change. Why can't we be isomorphic here?
>
>>>
>>>>>
>>>>> 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
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>