194572 - [BeOS]Attempt for better pending event handling: GetLastInputEventTime(PRUint32& aTime) and more

Status: RESOLVED WONTFIX

(Core Graveyard :: GFX: BeOS, defect)

Description

[Kevin McCluskey (gone)]

this was broken off from bug 165039

To improve interactivity during long page loads we need to implement
nsIWidget::GetLastInputEventTime.

Since it is currently not implemented, Mozilla gets the last user input event
time from the viewmanager. The viewmanager only reports times for mouse and
keyboard events which occur in windows associated with a viewmanager. Other
mouse and keyboard events such as clicking in a window title bar are not
reported so moving a window while loading a long page is sluggish. For embedded
applications, clicking on native menus and interacting with native dialog boxes
is also sluggish because these mouse and keyboard events times are not recorded
in the viewmanager.


NS_IMETHOD GetLastInputEventTime(PRUint32& aTime) = 0;

Description:

Get the last user input event time in milliseconds. If there are any pending
native toolkit input events it returns the current time. All input events are 
included (ie. it is *not* limited to events targeted at this nsIWidget instance.

@param aTime Last user input time in milliseconds. This value can be used to
compare durations but can not be used for determining wall clock time. The value
returned is platform dependent, but is compatible with the expression 
PR_IntervalToMicroseconds(PR_IntervalNow()).

Comment 1

[Sergei Dolgov]

investigating, though, i don't see any problem with windows move while loading
"heavy" pages - in BeOS multithreaded environment it's rather care of
Application  Server - and  TitleBar, borders, resize corner belong rather to
app_server department.

Comment 2

[Sergei Dolgov]

Maybe there is still some sense to implement it if it helps for wheel events on
page load (seems wheel scrolling is lagging or lost at all on heavy load), also
for new tab creation from links while large page is loading somewhere.

Comment 3

[Sergei Dolgov]

If i understand situation correctly, first approach to this may lok very simple:
NS_METHOD nsWindow::GetLastInputEventTime(PRUint32& aTime)
{
	aTime =  (mView ? PR_IntervalToMicroseconds(PR_IntervalNow()) - 
(PRUint32)idle_time():PR_IntervalToMicroseconds(PR_IntervalNow()))/1000;
	return NS_OK;
}

idle_time() is BeOS API call reporting time from last user activity (mouse,
keyboard).
But i still have question about milliseconds and microseconds problem.

Comment 4

[Paul]

Attachment: initial implementation, tested, seems to work

I don't like the fact that I must iterate through ALL windows, but, according
to the description of the method in nsIWidget.h, it wants the current time if
there are ANY pending events for the application.  (see comment)

Comment 5

[Sergei Dolgov]

Problem is that nor reading bug 165039, neither communication with patch authors
on #mozilla IRC didn't convince me, that we really need it, and more, that it
didn't decrease (heh) BeZilla performance.

In BeOS port we don't suffer from problems described in original bug,
also we don't have so extensive PendingEvents code in nsWindow as Win32 Mozilla has.

And i didn't notice any improvement with patch, nor with my old primitive,
neither with your new more advanced. Rather vice versa.


So i'm more than unsure that we need it now, before e.g. wide usage of native
widgets is here.

I'm wondering if we can perform some objective tests

Comment 6

[Sergei Dolgov]

In current situation i think that good idea is to check it in, when patch is
correct and full, but in ifdef-ed state (OFF) - with comments.

Inspite it cluters nsWindow.cpp a bit, we can restore it in future with ease, if
it appears that we really need something alike.

(And about general not-so-good user resposibility of BeZilla. I think, that with
current implementation of nsprub, event management and CallMethod proxy usage,
main reason is slow interaction with file-system (or low priority for file
events). Mozilla relies heavily on it, if i understand problem corectly - reading
chrome files (inspite XUL cache) in/after each Create() event, reading/writing
RDF files, filling caches - all those are blockers in situation when we trying
to reduce our multithreading with proxying and sequentization.
So it seems we should search for solution somewhere else)

Comment 7

[Paul]

I don't know if this is needed now or not either.  It can remain here, as a
patch, until other work is done on nsWindow.  But, I don't think we should check
it in, unless we will use it.

I did not notice a slow down because of it either (I think it is fairly quick),
and, to get it to actually return PRInterval_Now(), takes a LOT of work.  I.e.,
go to a site with a lot of links, and start openning them in different tabs with
the middle mouse button, and then try to scroll the current page, while the
others are loading.  If you put a debug statement in the if (pendingInputEvents)
test, you'll see, that it does actually become true occasionally.

Comment 8

[Sergei Dolgov]

Yeah, i also thought about scrolling page while loading smth in another tab.
It is jumpy sometimes, especially with wheel-scroll.
But i also noticed that ONWHEEL priority is lower than ONMOUSE in nsAppShell.cpp.
Maybe we also should wait and see about life of this bug in Linux, XLIB and MacOS.
(It seems nobody took care about it there yet, inspite analogous bugs were
submitted:
http://bugzilla.mozilla.org/show_bug.cgi?id=172499
http://bugzilla.mozilla.org/show_bug.cgi?id=172501
http://bugzilla.mozilla.org/show_bug.cgi?id=194623)
------------
Anyway, current patch contains strange addition:
@@ -207,6 +209,45 @@
 	{
 		mView->Window()->EndViewTransaction();
 		mView->UnlockLooper();
+	}
+	return NS_OK;
+}
+

Comment 9

[Paul]

Hmm, ONMOUSE and ONWHEEL are both PRIORITY_THIRD in my nsAppShell.cpp

And that "oddity" in the diff, well, look at the bottom of what was added.  It
is just where diff decided the differences started, that's all.

Comment 10

[Sergei Dolgov]

I may put review if you think it is reasonable,

but i'm wondering if

if ((!be_app->->MessageQueue()->IsEmpty()) &&
		    ((be_app->MessageQueue()->FindMessage((uint32)B_MOUSE_DOWN)
will work too instead looping through windows.

Or there are lot of other messages (non-user) in be_app, so we trigger Find() in
this case even more frequently on !IsEmpty() check than with iteration through
BWindows?
Or is is just improper place and improper messages in be_app message queue?

Sorry, if question is stupid, i don't imagine well this part of BeAPI.

Comment 11

[Sergei Dolgov]

Paul, as tree is open now, it seems, do you need review for this?

Comment 12

[Sergei Dolgov]

Comment on attachment 121151 [details] [diff] [review]
initial implementation, tested, seems to work

r = [e-mail]

Comment 13

[tqh]

Still an issue?

Comment 14

[Sergei Dolgov]

Who knows.
We've made two different implementation, but i'm unsure if those had any effect
on BeOS port.
Maybe nothing bad happens if we check this patch in.

Comment 15

[Sergei Dolgov]

Current patch with BeOS MessageQueue is doubtful.
Problem is rather about events, stalled between point, when it reached widget and point, where NS_MESSAGE is sent.

Once we process BView/BWindow mouse/keyboard hooks, BeOS message queue goes empty, but event may still "hang" unsent.

Comment 16

[Sergei Dolgov]

Wondering if i can add DispatchPendingEvents() using Win 32 as example

Comment 17

[Sergei Dolgov]

Attachment: patch

Adds DispatchPendingEvents and GetLastInputEventTime methods to nsWindow and
GetEventQueue to nsToolkit.

I'm unsure if we really need flushing paint in DispatchPendingEvents() - Win32 OS/2 do it though.

Comment 18

[Doug Shelton]

Attachment: patch apply rejects

fyysik, applied this patch over (i think) clean cvs; one hunk didn't apply cleanly but it looks like the code is already in place.

Comment 19

[Doug Shelton]

I just discovered the latest patch (version 2006-02-10) breaks the Firefox tab drag/drop feature.  Backed out the patch and recompiled, it works.  Put in the patch and drag drop fails.   When I click and drag, it appears that the click is working, drag is working but the "drop" never gets sent.  The tab stays where it was and the "dropmarker" arrow is permanently left on the screen.

Comment 20

[Doug Shelton]

Attachment: applies cleanly to current trunk. Fixes tab drag/drop problem.

Comment 21

[Sergei Dolgov]

Attachment: patch, moderate, no more eventQueue involved.

Applies cleanly on last trunk, placed here mostly for MouseMove issues

Comment 22

[Sergei Dolgov]

I didn;t see any benefit from GetLastInputEventTime()
I think we should abandon this.