Closed Bug 503892 Opened 12 years ago Closed 12 years ago

Remove window proc hook from nsToolkit

Categories

(Core :: Widget: Win32, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: robarnold, Assigned: robarnold)

References

Details

Attachments

(2 files, 1 obsolete file)

Attached patch v1.0 (obsolete) — 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 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));
Blocks: 496788
Attached patch v1.1Splinter Review
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)
No longer depends on: 503879
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/e890d6c5ab6d
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Attached patch winmo/wince fixSplinter Review
This should comment out the offending lines for windows CE.
Attachment #390056 - Flags: review?(roc)
rob, this does fix the problem. However, it is the right solution?  What do we lose?
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.
(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.
WM_ENTERSIZEMOVE and WM_EXITSIZEMOVE are not supported on WinMo.
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.
(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
Could this be causing Bug 507222 ?
You need to log in before you can comment on or make changes to this bug.