Follow-up from bug 617076 (the part 1 patch): Timothy pointed out that nsViewManager::ResizeView shouldn't be called with a root view. I added an assertion for that which revealed that it triggers all the time so I added "NS_FRAME_NO_MOVE_VIEW | NS_FRAME_NO_SIZE_VIEW" flags in PresShell::DoReflow to avoid that. (see patch) Timothy said this is wrong and we should instead fix whatever makes that happen.
Add "I think" to whatever I said in comment 0. :)
Ok so nsViewManager::ResizeView is called on the root view all the time, but do we ever change the size of the root view this way? That is the case we don't want to happen.
You mean like so? It occurs when opening Edit->Preferences on Linux: ###!!! ASSERTION: ResizeView is resizing the root view: 'view != mRootView', file view/src/nsViewManager.cpp, line 1349 #0 NS_DebugBreak_P #1 nsViewManager::ResizeView at view/src/nsViewManager.cpp:1349 #2 PresShell::DoReflow at layout/base/nsPresShell.cpp:7869 #3 PresShell::ResizeReflowIgnoreOverride at layout/base/nsPresShell.cpp:2886 #4 DocumentViewerImpl::SizeToContent at layout/base/nsDocumentViewer.cpp:3207 #5 nsGlobalWindow::SizeToContent at dom/base/nsGlobalWindow.cpp:5263 #6 nsGlobalWindow::SizeToContent at dom/base/nsGlobalWindow.cpp:5241 #7 NS_InvokeByIndex_P ...
Yeah, I was thinking something like that. So you're saying that assertion still fires?
Yes, with the STR and stack in comment 3.
Ok, I see. The main reason I wanted that assertion was to make sure that the visible area stored on the prescontext is always in sync with the root view size. So I think we should just make ResizeView handle resizing a root view properly. Mats, I think you had a patch for that in the old bug?
And adding an assert that the root view's size matches the prescontext's visible area would be good, is the end PresShell::FlushPendingNotifications a good place for that?
It seems we can only assert that when (aType >= Flush_Layout) though.
(In reply to comment #8) > It seems we can only assert that when (aType >= Flush_Layout) though. That makes sense because we only do the FlushDelayedResize(PR_TRUE) call if aType >= Flush_Layout (some of the time).
Comment on attachment 510800 [details] [diff] [review] fix v2 >@@ -4906,16 +4909,27 @@ PresShell::FlushPendingNotifications(moz >+ static_cast<nsView*>(rootView)->GetDimensions(viewSize); Let's keep nsView inside of /view I think. So use whatever methods available on nsIView to get what you need. Thanks Mats.
Attachment #510800 - Flags: review?(tnikkel) → review+
Comment on attachment 510800 [details] [diff] [review] fix v2 I actually think we should take this. If zooming is involved then we could see the same type of problem as bug 617076. And it should be safe, we execute the same code, but in the right view manager with the right app units per dev pixel value.
with the nit fixed
Attachment #511350 - Flags: approval2.0? → approval2.0+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Summary: Make nsViewManager::ResizeView assert it's not called with a root view → Use the right VM for the parent view in nsViewManager::ResizeView. Assert that the root view size is in sync with the pres context visible size.
Target Milestone: --- → mozilla2.0b12
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.