263569 - [FIX]Users of EndUpdateViewBatch(NS_VMREFRESH_DEFERRED) probably don't need it

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P3)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Followon from bug 262760. A number of callers to
EndUpdateViewBatch(NS_VMREFRESH_DEFERRED) probably don't really need that
semantics (which is to delay all painting until an Invalidate PLEvent is
processed). Mostly they probably just want NS_VMREFRESH_NO_SYNC which will paint
whenever the platform toolkit decides to paint.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 161620 [details] [diff] [review]
Fix

This fixes the callers that are really just trying to prevent reflow-triggered
or hand-triggered invalidates from being applied one at a time to use NO_SYNC. 
I added a comment to the nsCSSFrameConstructor code and left nsDocumentViewer
well enough alone (I can't tell why that code even exists, really, so...)

One thing I wonder about.  Do we need to worry about reflow changing widget
dimensions so the invalidates that got deferred won't invalidate the right
areas?	Or would the widget size change automatically invalidate whatever needs
invalidating?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

> Or would the widget size change automatically invalidate whatever needs
> invalidating?

The view size change should repaint the difference in areas.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> The view size change should repaint the difference in areas.

Yes, but the dirty area we have stored on the view will be relative to the old
widget rect, not the new one.  So when we go to actually invalidate, we will
invalidate the wrong area, no?

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I looked into this and I think we're OK. nsViewManager::UpdateWidgetArea
distributes invalidation over the views with widgets which cover a particular
area of the screen. When you move a view we invalidate the union of the old and
new areas via the parent view (which will in turn call UpdateWidgetArea to
distribute the invalidation over other views with widgets). I think this
guarantees that the correct part of the screen will eventually be updated,
although a proof would be tricky and require induction over the depth of the
view in the view tree.

This would be simpler and also more efficient if we accumulated invalid regions
per-view and then did the UpdateWidgetArea logic in ProcessPendingUpdates. Note
that if this is broken then it would already have been visible in the places
where we already used Begin/EndUpdateViewBatch.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Fixed.