review of commit ea30719

Ian Springer ian.springer at redhat.com
Mon Dec 13 18:21:31 UTC 2010


On 12/13/2010 12:47 PM, Joseph Marques wrote:
>> commit ea307199f414e3f155c4ababaabb5fee769c6c9c
>> Author: Ian Springer<ian.springer at redhat.com>
>> Date:   Mon Dec 13 12:30:56 2010 -0500
>>
>>       2nd attempt at fixing subtab refresh when switching back to a subtab from another subtab on the same Resource or group, hopefully this time without breaking TableSection detail views (https://bugzilla.redhat.com/show_bug.cgi?id=661528); disable ack-all and delete-all buttons on alert history view if there are 0 alerts in the history
>>
> ...
>> modules/enterprise/gui/coregui/src/main/java/org/rhq/enterprise/gui/coregui/client/inventory/common/detai/AbstractTwoLevelTabSetView.java
>> @@ -240,11 +240,12 @@ public abstract class AbstractTwoLevelTabSetView<T, U extends Layout>   extends Lo
>>                if (subView instanceof BookmarkableView) {
>>                    // Handle any remaining view items (e.g. id of a selected item in a subtab that contains a Master-Details view).
>>                    ((BookmarkableView) subView).renderView(viewPath);
>> -            } else if (subView instanceof RefreshableView) {
>> -                if (subView.isDrawn()) {
>> -                    // Refresh the data on the subtab, so it's not stale.
>> -                    ((RefreshableView) subView).refresh();
>> -                }
>> +            }
>> +
>> +            if (subView instanceof RefreshableView&&   viewPath.isEnd()&&   subView.isDrawn()) {
>> +                // Refresh the data on the subtab, so it's not stale.
>> +                Log.debug("Refreshing data for [" + subView.getClass().getName() + "]...");
>> +                ((RefreshableView) subView).refresh();
>>                }
>>
>>                this.tabSet.markForRedraw();
> Now I think we're dealing with a race condition.  Since many
> RefreshableViews are also BookmarkableViews, both checks may catch.
> However, the renderView(ViewPath) method is not guaranteed to do all of
> its work synchronously, so we may be calling refresh() before some or
> all of the asynchronous work inside renderView(ViewPath) completes.

Ugh, yes, you're right. In fact, this is how the code used to be, and 
Jay changed it at some point to prevent that race condition. Since our 
renderView() API currently provides no viewRendered() callback 
mechanism, I think the easiest way to address this for now is for a view 
V that implements both BookmarkableView and RefreshableView (the case 
I'm trying to cover here), make V's renderView impl responsible for 
calling refresh() on itself when it's all done doing its own thing. What 
do you think?



More information about the rhq-devel mailing list