Closed
Bug 503892
Opened 15 years ago
Closed 15 years ago
Remove window proc hook from nsToolkit
Categories
(Core :: Widget: Win32, defect)
Core
Widget: Win32
Tracking
()
RESOLVED
FIXED
People
(Reporter: robarnold, Assigned: robarnold)
References
Details
Attachments
(2 files, 1 obsolete file)
5.84 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
961 bytes,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
It's not needed to track if the user is moving one of the gui thread's window (there's a new API in Windows 98 to do so) and the FavorPerformanceHint calls don't add much if any value.
Attachment #388237 -
Flags: review?(roc)
Why is the variable called haveMouseActivity when it actually means "moving or sizing a window"? Seems like you could just do if (HIWORD(GetQueueStatus(QS_INPUT))) return PR_TRUE; ... if (!GetGUIThreadInfo(...)) return PR_FALSE; return GUI_INMOVESIZE == (guiInfo.flags & GUI_INMOVESIZE);
Comment 2•15 years ago
|
||
Comment on attachment 388237 [details] [diff] [review] v1.0 >+ if (GetGUIThreadInfo(GetCurrentThreadId(), &guiInfo)) >+ haveMouseActivity = GUI_INMOVESIZE == (guiInfo.flags & GUI_INMOVESIZE); >+ return qstatus || haveMouseActivity; Is it worth writing this as return qstatus || (GetGUIThreadInfo(GetCurrentThreadId(), &guiInfo) && (guiInfo.flags & GUI_INMOVESIZE));
Assignee | ||
Comment 3•15 years ago
|
||
Cleaned up per review comment. The diff no longer depends on 503879 so this should apply cleanly to mozilla-central.
Attachment #388237 -
Attachment is obsolete: true
Attachment #388963 -
Flags: review?(roc)
Attachment #388237 -
Flags: review?(roc)
Attachment #388963 -
Flags: review?(roc) → review+
Assignee | ||
Comment 4•15 years ago
|
||
Pushed to mozilla-central: http://hg.mozilla.org/mozilla-central/rev/e890d6c5ab6d
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 5•15 years ago
|
||
This looks to have broken WinMo builds http://tinderbox.mozilla.org/showlog.cgi?log=Mobile/1248216747.1248219194.19016.gz
Assignee | ||
Comment 6•15 years ago
|
||
This should comment out the offending lines for windows CE.
Attachment #390056 -
Flags: review?(roc)
Comment 7•15 years ago
|
||
rob, this does fix the problem. However, it is the right solution? What do we lose?
Assignee | ||
Comment 8•15 years ago
|
||
Windows does post a fairly good stream of WM_MOVING/WM_SIZING/WM_WINDOWPOSCHANGING messages during the resize or move so I'm not sure that you're actually losing that much (if anything). It occurs to me now that the extra check might be redundant but I'll need to spend some time with Spy++ to confirm. You still keep the performance advantage of not having a window hook for all the windows on that thread (i.e. the entire application).
It won't compile without a semicolon after PR_FALSE. If Windows CE supports WM_ENTERSIZEMOVE and WM_EXITSIZEMOVE, we could listen for those to keep track of this ourselves.
Assignee | ||
Comment 10•15 years ago
|
||
(In reply to comment #9) > It won't compile without a semicolon after PR_FALSE. Right. Can fix that before checkin. > If Windows CE supports WM_ENTERSIZEMOVE and WM_EXITSIZEMOVE, we could listen > for those to keep track of this ourselves. Based on the discussion in bug 496788, I'm not sure if checking for these messages helps more than it hurts.
Comment 11•15 years ago
|
||
WM_ENTERSIZEMOVE and WM_EXITSIZEMOVE are not supported on WinMo.
Attachment #390056 -
Flags: review?(roc) → review+
Comment 12•15 years ago
|
||
rob, lets get something checked in to fix the burn. if you think there is further wince specific work that can be done, lets file a new bug.
Assignee | ||
Comment 13•15 years ago
|
||
(In reply to comment #12) > rob, lets get something checked in to fix the burn. if you think there is > further wince specific work that can be done, lets file a new bug. Can't think of any that's wince-specific. Fix pushed to mozilla-central: http://hg.mozilla.org/mozilla-central/rev/9f11f584896f
Comment 14•15 years ago
|
||
Could this be causing Bug 507222 ?
You need to log in
before you can comment on or make changes to this bug.
Description
•