<Swing Dev> Review request #3: 6852592 (revalidate() must be smarter)

[Anthony Petrov]

On 7/29/2009 7:59 PM Alexander Potochkin wrote:
>> Hm, what kind of code might work wrong in that classes when the Window 
>> and the Applet become validate roots? I don't see anything suspicious 
>> there. In the mentioned snippets looking for a top-level component is 
>> only used to verify that the whole hierarchy is contained within a 
>> top-level. Since the search starts exactly from the previously found 
>> validate root, the code must work perfectly even if there's no 
>> validate roots between a component and the top-level (i.e. when 
>> there's no a RootPane at least, which is unlikely for a Swing 
>> top-level, ain't it?) But as I said, even in that fantastic case 
>> everything seems to work OK.
> 
> Here is the current code in RM:
> 
>    for(Component c = invalidComponent; c != null; c = c.getParent()) {
>    if ((c instanceof JComponent) && (((JComponent)c).isValidateRoot())) {
>         validateRoot = c;
>         break;
>         }
>     }
> 
> I'd like to have it rewritten like this:
> 
>    for(Container c = invalidComponent; c != null; c = c.getParent()) {
>    if (c.isValidateRoot()) {
>         validateRoot = c;
>         break;
>         }
>     }
> 
> The next statement is:
> 
>     /* There's no validateRoot to apply validate to, so we're done.
>      */
>     if (validateRoot == null) {
>         return;
>     }
> 
> 
> So the code is ready to the situation when there is no validate root
> 
> If you make Window a validate root,
> the for loop will always find it and the logic will be changed

Yes! And that seems to be completely correct: we HAVE TO call validate() 
on a validate root (or a top-level component - whichever we find) - no 
matter what is the component that represents the found validate root - 
an old-good Swing RootPane, or the top-level window. So, whichever we 
find - we just schedule the invocation of the validate() method on that 
component, and therefore make the hw/lw mixing-related code happy. :)


> This is something to keep in mind

So I think the current code (including the rewritten version as you 
suggest) perfectly fits to the idea of making top-level windows validate 
roots.


>> Unfortunately the API in AWT does not make any difference between 
>> these kinds of ancestor: the Component.getParent() is used for both. 
>> And unfortunately we can't change that now due to 
>> backward-compatibility issues (that's why it is very important to 
>> architect APIs carefully from the start.)
> 
> You mean that dialog.getParent() returns the dialog's owner?
> 
> Indeed... what a mess!

A sad legacy...


>> That said, the invalidate() method may easily jump to the owner of a 
>> dialog (or a window) while invalidating the hierarchy of the owned 
>> window - which is absolutely incorrect. To make sure this never 
>> happens, we need to stop invalidating on top-level components, hence 
>> the need to make the Window a validate root.
>>
>> Sounds reasonable?
> 
> Yep

Great! And thanks for reviewing the code! By the way, are you proposing 
to apply the suggested chunk of code to the RepaintManager with this 
fix, or is it going to be a separate CR?

--
best regards,
Anthony