This bug deals with the remaining OMTC work from bug 676241.

The new patch in bug 873944 apparently doesn't trigger a composition during drawRect when none has been scheduled from Gecko. So it won't fix the bug which I'd hoped it would fix, namely that hovering the titlebar buttons in OMTC drawintitlebar mode currently doesn't repaint them in their hovered state.

This is quite the brute force approach and looks very hacky. Any better ideas? The ClientLayerManager.cpp hunk makes us call PrepareWindowEffects in the empty transaction case, too, not only in the normal transaction case. I think we want this anyway. The nsViewManager.cpp hunk makes us call BeginTransaction+EndEmptyTransaction from viewWillDraw even if no views have ForcedRepaint(). The rest causes ShadowLayerForwarder to trigger a recomposition during viewWillDraw even though the transaction mTxn is empty.

Comment on attachment 759732 [details] [diff] [review] part 1: call NotifyDirtyRegion during viewWillDraw I think this patch would be good to take anyway, regardless of the validity of the next patch.

Comment on attachment 759734 [details] [diff] [review] part 2: trigger recomposition from viewWillDraw even if no layers changed Review of attachment 759734 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/nsPresShell.cpp @@ +5520,5 @@ > // that will cause us to forget to update the real layer manager! > if (!(aFlags & PAINT_LAYERS)) { > + if (layerManager->HasShadowManager() && Compositor::GetBackend() != LAYERS_BASIC) { > + return; > + } I assume this indention change wasn't intentional? ::: view/src/nsViewManager.cpp @@ +634,5 @@ > } > + } else { > + LayerManager *manager = aWidget->GetLayerManager(); > + if (manager->HasShadowManager() && > + Compositor::GetBackend() != LAYERS_BASIC) { How about just taking the normal branch if this check is true? Either move it up, or put it inside ForcedRepaint(). If we have OMTC, then getting a paint event means we always want to force a repaint. If we ever get OMTC everywhere, then we could drop the forced repaint concept entirely. I'd feel better about that rather than having explicit Begin/EndTransaction calls inside view code.

(In reply to Matt Woodrow (:mattwoodrow) from comment #5) > Comment on attachment 759734 [details] [diff] [review] > part 2: trigger recomposition from viewWillDraw even if no layers changed > > Review of attachment 759734 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: layout/base/nsPresShell.cpp > @@ +5520,5 @@ > > // that will cause us to forget to update the real layer manager! > > if (!(aFlags & PAINT_LAYERS)) { > > + if (layerManager->HasShadowManager() && Compositor::GetBackend() != LAYERS_BASIC) { > > + return; > > + } > > I assume this indention change wasn't intentional? Huh, not sure how this happened. > ::: view/src/nsViewManager.cpp > @@ +634,5 @@ > > } > > + } else { > > + LayerManager *manager = aWidget->GetLayerManager(); > > + if (manager->HasShadowManager() && > > + Compositor::GetBackend() != LAYERS_BASIC) { > > How about just taking the normal branch if this check is true? > > Either move it up, or put it inside ForcedRepaint(). I've moved it up.

Comment on attachment 761925 [details] [diff] [review] part 2 Review of attachment 761925 [details] [diff] [review]: ----------------------------------------------------------------- ::: view/src/nsViewManager.cpp @@ +632,5 @@ > + LayerManager *manager = aWidget->GetLayerManager(); > + if (view && > + (view->ForcedRepaint() || > + (manager->HasShadowManager() && > + Compositor::GetBackend() != LAYERS_BASIC))) { I'm assuming this will land after bug 873944? In which case these two lines can just become !manager->NeedsWidgetInvalidation()

Yes, I'll wait for bug 873944 before landing this.

https://hg.mozilla.org/integration/mozilla-inbound/rev/381f39912071 https://hg.mozilla.org/integration/mozilla-inbound/rev/c21f4e5561bb

https://hg.mozilla.org/mozilla-central/rev/381f39912071 https://hg.mozilla.org/mozilla-central/rev/c21f4e5561bb