Closed Bug 611693 Opened 9 years ago Closed 9 years ago

Txul, Ts regression on XP after landing bug 610201

Categories

(Core :: Widget: Win32, defect)

x86
Windows XP
defect
Not set

Tracking

()

VERIFIED FIXED
Tracking Status
blocking2.0 --- final+

People

(Reporter: jimm, Assigned: jimm)

References

Details

Attachments

(1 file, 2 obsolete files)

blocking2.0: --- → ?
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.
(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.
Blocks: 610201
Attached patch fix (obsolete) — Splinter Review
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 on attachment 490391 [details] [diff] [review]
fix

simple patch, put the consant checks back in to avoid calls to GetNearestWidget.
Attachment #490391 - Flags: review?(vladimir)
Summary: Txul, Ts regression on XP after landing bug 610201 & bug 586228 → Txul, Ts regression on XP after landing bug 610201
Attached patch fix (obsolete) — Splinter Review
woops, we need a patch that actually builds.
Attachment #490391 - Attachment is obsolete: true
Attachment #490399 - Flags: review?(vladimir)
Attachment #490391 - Flags: review?(vladimir)
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.
Attachment #490399 - Flags: review+
Attached patch fixSplinter Review
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.
Attachment #490399 - Attachment is obsolete: true
Attachment #490440 - Flags: review?(vladimir)
http://hg.mozilla.org/mozilla-central/rev/05a80026d347
Status: NEW → RESOLVED
blocking2.0: ? → ---
Closed: 9 years ago
Resolution: --- → FIXED
blocking2.0: --- → final+
Talos Improvement! Ts decrease 4.25% on XP Firefox  
http://groups.google.com/group/mozilla.dev.tree-management/browse_thread/thread/12fd8eec1fc77c45#
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.