Closed
Bug 314687
Opened 19 years ago
Closed 19 years ago
[BeOS] Stop unhandled native message flood.
Categories
(Core Graveyard :: Widget: BeOS, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: sergei_d, Assigned: sergei_d)
References
Details
(Keywords: fixed1.8.1)
Attachments
(2 files)
5.06 KB,
patch
|
thesuckiestemail
:
review+
beltzner
:
approval1.8.1+
|
Details | Diff | Splinter Review |
5.95 KB,
patch
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•19 years ago
|
||
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)
Assignee | ||
Comment 2•19 years ago
|
||
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.
Assignee | ||
Comment 4•19 years ago
|
||
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
Assignee | ||
Comment 5•19 years ago
|
||
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+
Assignee | ||
Comment 7•19 years ago
|
||
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: 19 years ago
Resolution: --- → FIXED
Comment 8•18 years ago
|
||
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?
Comment 10•18 years ago
|
||
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+
Assignee | ||
Comment 11•18 years ago
|
||
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
Comment 12•18 years ago
|
||
Sergei, could you commit this to the branch after bug 312660?
Comment 13•18 years ago
|
||
Never mind this comment. It was meant for another bug.
Updated•10 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•