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

[Alexander Potochkin]

Hello Anthony

> 
> On 07/28/2009 10:31 PM, Alexander Potochkin wrote:
>>>> As for the second question about making Window/Applet validate roots,
>>>> I just don't see any practical benefits of this
>>>>
>>>> Could you please give more details why it is worth doing?
>>>
>>> As to the Applet class, there's a valid justification: the user does 
>>> not have a direct access to the underlying embedded frame which 
>>> therefore might be left invalid forever. Please see [1] for the 
>>> detailed description.
>>
>> If you absolutely need at least one validate root in the hierarchy,
> 
> This is not required. The requirement is to keep the whole hierarchy 
> valid. We can achieve that either using validate roots or calling 
> validate() on the top-level component. Since applets do not have 
> "official" access to/knowledge about their top-level component (the 
> embedded frame), we have to workaround that making the Applet a validate 
> root.
> 
> 
>> and know how to correctly rewrite the messy code in RepaintManager and 
>> JViewport, I am fine with that
> 
> 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

This is something to keep in mind

> 
>>> Regarding the Window: Artem proposed that at [2]. I didn't come up 
>>> with any failing scenario, so I considered this change harmless and 
>>> applied it. IMO, it isn't technically needed, but is conceptually 
>>> correct. Perhaps Artem might comment on that?
> 
> And here comes the technical justification to make the Window a validate 
> root. A component in a hierarchy can have two kinds of ancestors: a 
> container and an owner. The first one is the physical container as seen 
> on the screen (say, a Panel that a Button lays on). The latter is a 
> component that is kind of responsible for and/or dependent on the owned 
> component (like when the owned component must block its owner in some 
> cases, for instance). A perfect example is an owned modal Dialog. Dialog 
> does not have a container ancestor, but may indeed have an owner.
> 
> 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!

> 
> 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

alexp

> 
> -- 
> best regards,
> Anthony