611693 - Txul, Ts regression on XP after landing bug 610201

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

[Jim Mathies [:jimm]]

http://groups.google.com/group/mozilla.dev.tree-management/browse_thread/thread/69753b11f69952a5#

http://hg.mozilla.org/mozilla-central/rev/546cc8df6cae
http://hg.mozilla.org/mozilla-central/rev/59bbc730aee4

This makes no sense, neither of these patches has code that executes on XP.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Are we doing OS detection at a different point now than before? Is the detection itself expensive? Seems like we should back out either one or both of these to see whether the regression will go away.

Comment 2

[Jim Mathies [:jimm]]

(In reply to comment #1)
> Are we doing OS detection at a different point now than before? Is the
> detection itself expensive? Seems like we should back out either one or both of
> these to see whether the regression will go away.

No not really. In one patch XP drops out here:

http://hg.mozilla.org/mozilla-central/rev/546cc8df6cae#l2.28

In the other xp never executes the code because the fx button is disabled on xp by default.

I did have a bit of cleanup related to wm_activate in one patch:

http://hg.mozilla.org/mozilla-central/rev/59bbc730aee4#l2.96

AFAICT this shouldn't have affected things since I don't see references to acceptActivation in our code base. To test I've fired off two try builds, one as a base line and one with this code inserted back in. 

If that doesn't show anything I'll try backing each patch out and send those to try. If I don't have this straightened out by monday I'll back them both out.

Comment 3

[Jim Mathies [:jimm]]

Attachment: fix

Ok, I think I've tracked this down. Looks like GetNearestWidget has overhead I didn't anticipate. I removed the previous flag check in bug 610201.

Comment 4

[Jim Mathies [:jimm]]

Comment on attachment 490391 [details] [diff] [review]
fix

simple patch, put the consant checks back in to avoid calls to GetNearestWidget.

Comment 5

[Jim Mathies [:jimm]]

Attachment: fix

woops, we need a patch that actually builds.

Comment 6

[Jim Mathies [:jimm]]

Comment on attachment 490399 [details] [diff] [review]
fix

hmm, this didn't fix the problem but this:

http://hg.mozilla.org/try/rev/e8935557e87b

did. I need to investigate this further.

Comment 7

[Jim Mathies [:jimm]]

Attachment: fix

Ok, sorry for the review spam vlad. This is the right fix. UpdateNativeThemeInfo was getting called after UpdateTitlebarInfo in nsWindow, which reset this flag. Since WM_GETTITLEBARINFOEX isn't supported on xp, UpdateTitlebarInfo never succeeded, so the temp window was getting created multiple times on startup.

Comment 8

[Jim Mathies [:jimm]]

http://hg.mozilla.org/mozilla-central/rev/05a80026d347

Comment 9

[Jim Mathies [:jimm]]

Talos Improvement! Ts decrease 4.25% on XP Firefox  
http://groups.google.com/group/mozilla.dev.tree-management/browse_thread/thread/12fd8eec1fc77c45#