Closed Bug 314687 Opened 15 years ago Closed 15 years ago

[BeOS] Stop unhandled native message flood.

Categories

(Core Graveyard :: Widget: BeOS, defect)

x86
BeOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: sergei_d, Assigned: sergei_d)

References

Details

(Keywords: fixed1.8.1)

Attachments

(2 files)

There is old problem in BeOS port, with long history of partial solutions.
There are some native events, like resize, draw, mouse move and, maybe, wheel, which generate very intensive message sequences. Mozilla appeared quite frequently to unable to process those "just-in-time". As result, resize code was rewritten about 5 or more times, drawing code introduced cached drawing region amnd also was rewritten several times, so mouse code (which uses also special trick in nsAppShell). Also, as attempt to neutralize most ugly issues related to the fact, nsAppShell uses special events priorities, which has some side efects, like breaking real event order in some cases.
But actually, we gradual evolution of code now we have situation, when we are getting state of native widgets just-in-place - at Mozilla main GUI thread - for size, for draw, for wheel, and in future (i hope), with bug https://bugzilla.mozilla.org/show_bug.cgi?id=312755 - also for mouse.
Maybe, at some point, we can use it also for keyboard, if needed.

In this situation there is effective trick, described here:
http://www.livejournal.com/community/bezilla/139736.html?view=856792.

It means that we will implement set of "feedback-flags" in nsViewBeOS, to indicate that Mozilla got some  event related to given hook (all those flags will be set to true at constructor).
Then we will send events from hooks via toolkit, only if flag is true, and together with sending, we turn flag to false. It will stay false, until Mozilla gets message, and then turning the flag again to true.
It prevents pocking main thread with redundant messages, and, at same time, we get really actual native widget state, when message reaches its target, because, as i mentioned above, this trick will be used only in those places, when we are getting state at place.
Trick with Draw() works quite effectively, and I will prepare patch which'll possibly include changes for other mentioned events, to estimate if there is any serious  issue in such approach. 
Maybe it allows also to simplify in future code in nsAppShell/nsToolkit and let messages follow in natural order.
Attached patch patchSplinter Review
New handling for DoDraw() and FrameResized().
Similar changes for MouseMoved() and wheel-scrolling will be made in 
bugs:
https://bugzilla.mozilla.org/show_bug.cgi?id=312755
and
https://bugzilla.mozilla.org/show_bug.cgi?id=314792
respectively
Attachment #201652 - Flags: review?(thesuckiestemail)
Only thing which I see to discuss, if we should set flag in nsWindow::ONRESIZE: instead GetBounds() (as it allows to redice number of messages even more, but will require use of Lock/UnlockLooper() pair for safety.
Sergei, I had done a different version in my old code, which I've updated to the current nsWindow. It might be good to compare these. I made mine atomic, but it might not matter.
even in case of your approach which is quite similat, i prefer to clear flags not 
 in GetPaintRegion (which was OK in past), but in Validate(), as this is not method which really clears paint region, so changes state of pending painting
When we test or set flags in hooks, looper is locked anyway - as it is made such way in BeOS.
But when we operate with those flags from mozilla widget thread, seems reasonable to LockLooper for safety purpose. As this is atomic locking operation (according to BeBook) which actualy has very little overhead, when looper is already locked, and is used actually as simple if-test in such case in BeOS internals.

That's my thoughts about atomic operations in our situation.
Comment on attachment 201652 [details] [diff] [review]
patch

r=thesuckiestemail@yahoo.se

The difference between patches is that this one needs the window to be locked. While the other used atomic ops and compressed flags in one int, it had the flaw of having the flags only for the view though.
Attachment #201652 - Flags: review?(thesuckiestemail) → review+
Patch laned in trunk:
Checking in mozilla/widget/src/beos/nsWindow.cpp;
/cvsroot/mozilla/widget/src/beos/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 1.106; previous revision: 1.105
done
Checking in mozilla/widget/src/beos/nsWindow.h;
/cvsroot/mozilla/widget/src/beos/nsWindow.h,v  <--  nsWindow.h
new revision: 1.42; previous revision: 1.41
done 
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment on attachment 201652 [details] [diff] [review]
patch

Requesting approval for the MOZILLA_1_8_BRANCH.

This is a BeOS-only change and will not affect other platforms.
Attachment #201652 - Flags: approval1.8.1?
Add to the list of 2.0 bugs
Blocks: 311032
Comment on attachment 201652 [details] [diff] [review]
patch

a=drivers. Please go ahead and land this on the branch.
Attachment #201652 - Flags: approval1.8.1? → approval1.8.1+
cvs commit: Examining mozilla/widget/src/beos
Enter passphrase for key '/boot/home/.ssh/id_dsa':
Checking in mozilla/widget/src/beos/nsWindow.cpp;
/cvsroot/mozilla/widget/src/beos/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 1.91.4.12; previous revision: 1.91.4.11
done
Checking in mozilla/widget/src/beos/nsWindow.h;
/cvsroot/mozilla/widget/src/beos/nsWindow.h,v  <--  nsWindow.h
new revision: 1.35.4.7; previous revision: 1.35.4.6
done 
Keywords: fixed1.8.1
Sergei, could you commit this to the branch after bug 312660?
Never mind this comment. It was meant for another bug.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.