Remove window proc hook from nsToolkit

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: robarnold, Assigned: robarnold)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Assignee

Description

10 years ago
Posted 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));
Assignee

Comment 3

10 years ago
Posted 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)
Assignee

Updated

10 years ago
No longer depends on: 503879
Assignee

Comment 4

10 years ago
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/e890d6c5ab6d
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Assignee

Comment 6

10 years ago
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?
Assignee

Comment 8

10 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

10 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.
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.
Assignee

Comment 13

10 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

10 years ago
Could this be causing Bug 507222 ?
You need to log in before you can comment on or make changes to this bug.