787148 - Mac: Title bar and window repaint at different time during focus/blur

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Benoit Girard (:BenWa)]

STR:
1) Focus Firefox window
2) Focus another non overlapping window
3) Title bar will be painted without focus, then next frame the window is painted without focus (lighter grey).
4) Focus Firefox window
5) Title bar will be painted with focus first, then the next frame the window is painted with focus (darker grey).

Comment 1

[Benoit Girard (:BenWa)]

I suspect this regression was also merged into aurora.

Comment 2

[Steven Michaud [:smichaud] (Retired)]

Benoit, do you know when this started happening?  And do you have any idea what patch caused or triggered it?

Comment 3

[Benoit Girard (:BenWa)]

No, but my guess is the fix from bug 770056 was incomplete.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Yes, I agree that's most likely.

Comment 5

[Josh Aas]

This is really bad, we need to get a fix into Firefox 17.

Comment 6

[Steven Michaud [:smichaud] (Retired)]

> my guess is the fix from bug 770056 was incomplete.

I've confirmed this -- this bug first appeared when the patch for bug 770056 was landed.

The actual cause will probably be a lot harder to figure out.

Though this bug is annoying, I don't think it should be considered a blocker -- the glitch lasts for no more than a fraction of a second.

Comment 7

[Steven Michaud [:smichaud] (Retired)]

Attachment: Test patch that doesn't use CUIDraw() in DrawUnifiedToolbar (*not* a fix)

I've found something that indicates it may be very difficult to fix this bug, or even practically impossible.

I've found that I can make this bug go away by partially reversing Markus' patch for bug 668195.  This suggests that, now that http://hg.mozilla.org/mozilla-central/rev/bd0a91621ea9 has landed, CUIDraw's results show up slightly earlier than the results of the methods it replaced.

(A tryserver build will follow in a few hours.)

Comment 8

[Steven Michaud [:smichaud] (Retired)]

Markus and/or Matt, can you see any way around the problem I described in comment #7?

Comment 9

[Steven Michaud [:smichaud] (Retired)]

Benoit, do you have any ideas?

Comment 10

[Steven Michaud [:smichaud] (Retired)]

Attachment: Possible fix

This patch also "works", and might actually be an acceptable fix.

So the "fast" drawing has nothing to do with CUIDraw -- it's (presumably) triggered by the call to CGContextRestoreGState().

The only downside of my patch that I can see is that we'd no longer be constraining where CUIDraw() can draw to inBoxRect.  But maybe that isn't necessary.

What do you think, Markus?

(Another tryserver build will follow in a few hours.)

Comment 11

[Matt Woodrow (:mattwoodrow)]

Attachment: Force synchronous repaint of layers when the page goes inactive

This fixes the bug for me.

The problem was that we were invalidating frames when the document changed to inactive but not forcing a synchronous paint.

So we'd composite the old content to the screen immediately, and then draw the updated colour on the next refresh driver tick and composite it again.

Comment 12

[Steven Michaud [:smichaud] (Retired)]

Here's a tryserver build made with my patch from comment #10:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-192fd9fb485a/try-macosx64/firefox-18.0a1.en-US.mac.dmg

Comment 13

[Markus Stange [:mstange]]

Comment on attachment 659893 [details] [diff] [review]
Possible fix

Really, this patch makes a difference? I have a hard time believing that. In any case, Matt's patch is the right thing to do here.

Comment 14

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #13)

Try it yourself and you'll see that it does (though I don't know exactly how).  Furthermore the fact that it does is quite interesting, and may come in handy later.

In any case, though, I agree that Matt's patch is (probably) a better fix.

Comment 15

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/96b753ef66df

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/96b753ef66df

Comment 17

[Lukas Blakk [:lsblakk] use ?needinfo]

We're merging in a little over a week, let's get this nominated for uplift while still on Aurora please.

Comment 18

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

Comment on attachment 659912 [details] [diff] [review]
Force synchronous repaint of layers when the page goes inactive

Review of attachment 659912 [details] [diff] [review]:
-----------------------------------------------------------------

Fix has been on trunk for a while with no problems.

Comment 19

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 659912 [details] [diff] [review]
Force synchronous repaint of layers when the page goes inactive

Been on trunk for a while, low risk, approving for Aurora.

Comment 20

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/ea144003b87f

Comment 21

[Virgil Dicu [:virgil] [QA]]

Verified with Mac OS 10.7 and 10.8 while focusing different windows (overlapping non-overlapping) and returning focus to Firefox.

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:17.0) Gecko/17.0 Firefox/17.0 Gecko/17.0

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/17.0 Firefox/17.0 Gecko/17.0