Last Comment Bug 424395 - Do we need to invalidate during DocumentViewerImpl::Destroy() ?
: Do we need to invalidate during DocumentViewerImpl::Destroy() ?
Status: RESOLVED FIXED
[inbound]
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla8
Assigned To: Mats Palmgren (:mats)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-03-21 11:29 PDT by Mats Palmgren (:mats)
Modified: 2011-08-04 17:32 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
stack (4.12 KB, text/plain)
2008-03-21 11:29 PDT, Mats Palmgren (:mats)
no flags Details
wip (931 bytes, patch)
2008-03-21 11:31 PDT, Mats Palmgren (:mats)
roc: review-
roc: superreview-
Details | Diff | Splinter Review
wip2 (607 bytes, patch)
2011-07-19 13:31 PDT, Mats Palmgren (:mats)
roc: review+
Details | Diff | Splinter Review

Description Mats Palmgren (:mats) 2008-03-21 11:29:50 PDT
Created attachment 311013 [details]
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 (:mats) 2008-03-21 11:31:55 PDT
Created attachment 311014 [details] [diff] [review]
wip
Comment 2 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-03-21 20:37:55 PDT
Comment on attachment 311014 [details] [diff] [review]
wip

I like this
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2008-03-21 21:39:46 PDT
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) 2008-03-21 22:38:48 PDT
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 (:mats) 2011-07-19 13:31:51 PDT
Created attachment 546883 [details] [diff] [review]
wip2

Like so?  It passed on Try and I tested it manually on Linux/WinXP/OSX and
I didn't see any problems.
Comment 7 Marco Bonardo [::mak] 2011-07-21 06:13:23 PDT
http://hg.mozilla.org/mozilla-central/rev/1e772f5fbc24
Comment 8 Timothy Nikkel (:tnikkel) 2011-07-22 10:23:19 PDT
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) 2011-07-31 20:22:13 PDT
If the view manager has no observer then it's not connected to a visible presentation, I guess.
Comment 10 Timothy Nikkel (:tnikkel) 2011-08-04 14:29:00 PDT
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) 2011-08-04 17:32:13 PDT
Wouldn't the child document going away already have triggered the invalidation?

Note You need to log in before you can comment on or make changes to this bug.