424395 - Do we need to invalidate during DocumentViewerImpl::Destroy() ?

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Mats Palmgren (inactive)]

Attachment: stack

See attached stack, it seems a bit unnecessary (and fragile) to do that.

One way of fixing it would be to call SetEnableRendering() in
DocumentViewerImpl::Show()/Hide().

Comment 1

[Mats Palmgren (inactive)]

Attachment: wip

Comment 2

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

Comment on attachment 311014 [details] [diff] [review]
wip

I like this

Comment 3

[Boris Zbarsky [:bzbarsky]]

Doesn't that disable rendering for the whole viewmanager tree, though?  That doesn't seem like the right thing to be doing, does it?

Comment 4

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

Comment on attachment 311014 [details] [diff] [review]
wip

of course, you're right :-(.

The safest thing to do here would be to just have nsViewManager::UpdateView drop invalidation on the floor if the view observer has been disconnected.

Comment 5

[Mats Palmgren (inactive)]

Attachment: wip2

Like so?  It passed on Try and I tested it manually on Linux/WinXP/OSX and
I didn't see any problems.

Comment 6

[Mats Palmgren (inactive)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/1e772f5fbc24

Comment 7

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

http://hg.mozilla.org/mozilla-central/rev/1e772f5fbc24

Comment 8

[Timothy Nikkel (:tnikkel)]

Why do we want to do this? If we have a change to the view tree that affects the rendering of the window why shouldn't we invalidate?

Comment 9

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

If the view manager has no observer then it's not connected to a visible presentation, I guess.

Comment 10

[Timothy Nikkel (:tnikkel)]

That document doesn't care about the invalidate, but a parent document might need to repaint if a child document goes away.

Comment 11

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

Wouldn't the child document going away already have triggered the invalidation?