Closed
Bug 165039
Opened 22 years ago
Closed 22 years ago
improve user interactivity during long page loads - Interruptible parsing needs better user input detection
Categories
(Core :: DOM: UI Events & Focus Handling, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla1.2beta
People
(Reporter: kmcclusk, Assigned: kmcclusk)
References
Details
(Keywords: perf, Whiteboard: [adt3])
Attachments
(1 file, 6 obsolete files)
On WIN32, when the user moves the window or clicks on menu items in an embedded application the parser is not interrupted frequently enough. This can be seen when loading a large local. If you move the window during the page load it is sluggish and jumps to the new location instead of dragging smoothly. Dragging over menu items in an embedded application (use MfcEmbed as a test) is also sluggish. This is because the logic to determine when the user has interacted with Gecko is based on stamping the time that an event arrives within the ViewManager. The logic needs to be improved to include the detection of events outside the Gecko window.
Assignee | ||
Updated•22 years ago
|
Assignee | ||
Comment 1•22 years ago
|
||
Note: This patch must be hand-applied. It also requires the patch in bug 157144 be checked in first.
Assignee | ||
Comment 2•22 years ago
|
||
With attachment 97099 [details] [diff] [review] and other improvements we will be able to increase the content update interval when the user is interacting with the UI/page during a long page load.
Assignee | ||
Comment 3•22 years ago
|
||
Attachment #97099 -
Attachment is obsolete: true
Attachment #97125 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Keywords: perf
Summary: Interruptible parsing needs better user input detection → improve user interactivity during long page loads - Interruptible parsing needs better user input detection
Comment 4•22 years ago
|
||
With attachment 97865 [details] [diff] [review], I experienced very smooth UI responsiveness while large documents were loading.
Assignee | ||
Comment 6•22 years ago
|
||
The patch will only affect WIN32. See: PR_IMPLEMENT(PRBool) PL_IsNativeEventPending() For the patch to have an effect on Linux or Mac PL_IsNativeEventPending() would need to have additional code inserted to determine if there is a pending native event on that platform.
I see. do you have plan to add more code about this for linux ?
No longer blocks: 166758
Assignee | ||
Comment 8•22 years ago
|
||
I will file separate bugs for Linux and Mac.
Assignee | ||
Comment 9•22 years ago
|
||
Attachment #97865 -
Attachment is obsolete: true
Assignee | ||
Comment 10•22 years ago
|
||
Attachment #100593 -
Attachment is obsolete: true
Comment 11•22 years ago
|
||
Comment on attachment 100721 [details] [diff] [review] Same as last patch - synchronized with the trunk Just a nit: NS_ENSURE_TRUE(NS_SUCCEEDED(rv), NS_ERROR_FAILURE); can be replaced with NS_ENSURE_SUCCESS(rv, rv); r=rods
Attachment #100721 -
Flags: review+
Assignee | ||
Comment 12•22 years ago
|
||
Attachment #100721 -
Attachment is obsolete: true
Comment 13•22 years ago
|
||
Comment on attachment 100957 [details] [diff] [review] improved patch which moves logic for hooking for the window move notification from plevent.c into the widget module. Also changes PL_FavorPerformanceHint so that calls to it can be nested. r=rods
Comment 14•22 years ago
|
||
Comment on attachment 100957 [details] [diff] [review] improved patch which moves logic for hooking for the window move notification from plevent.c into the widget module. Also changes PL_FavorPerformanceHint so that calls to it can be nested. ==== Rather than make the content sink dependent on widget stuff, should this code be in the view manager's GetLastUserEventTime() method? + nsCOMPtr<nsIWidget> widget; + nsresult rv = vm->GetWidget(getter_AddRefs(widget)); + NS_ENSURE_TRUE(NS_SUCCEEDED(rv), NS_ERROR_FAILURE); + if (! NS_SUCCEEDED(widget->GetLastInputEventTime(eventTime))) { + // If we can't get the last input time from the widget + // then we will get it from the viewmanager. + rv = vm->GetLastUserEventTime(eventTime); + NS_ENSURE_TRUE(NS_SUCCEEDED(rv), NS_ERROR_FAILURE); + } + ==== Use |NS_FAILED()| instead of |!NS_SUCCEEDED()|: + if (! NS_SUCCEEDED(widget->GetLastInputEventTime(eventTime))) {
Assignee | ||
Comment 15•22 years ago
|
||
I was planning on getting rid of the view manager's GetLastUserEventTime() method after all of the platforms supported nsIWidget::GetLastInputEventTime.
Comment 16•22 years ago
|
||
Comment on attachment 100957 [details] [diff] [review] improved patch which moves logic for hooking for the window move notification from plevent.c into the widget module. Also changes PL_FavorPerformanceHint so that calls to it can be nested. sr=kin@netscape.com Btw, I noticed that nsViewManager::GetWidget() always returns NS_OK, even if it returns a null widget, so we might want to check for a null widget before dereferencing it: + nsresult rv = vm->GetWidget(getter_AddRefs(widget)); + NS_ENSURE_TRUE(NS_SUCCEEDED(rv), NS_ERROR_FAILURE); + if (! NS_SUCCEEDED(widget->GetLastInputEventTime(eventTime))) { Perhaps something like: + if (!widget || NS_FAILED(widget->GetLastInputEventTime(eventTime))) {
Attachment #100957 -
Flags: superreview+
Attachment #100957 -
Flags: review+
Assignee | ||
Comment 17•22 years ago
|
||
Attachment #100957 -
Attachment is obsolete: true
Assignee | ||
Comment 18•22 years ago
|
||
Fix checked in to the trunk.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 19•22 years ago
|
||
What are the Mac and Linux specific bugs for this?
Assignee | ||
Comment 20•22 years ago
|
||
bug 172499 for Linux bug 172501 for Mac
Updated•22 years ago
|
QA Contact: rakeshmishra → trix
Comment 22•21 years ago
|
||
Kevin McCluskey wrote:
> bug 172499 for Linux
> bug 172501 for Mac
Did you file bugs for the Xlib, BeOS and Photon ports, too ?
Assignee | ||
Comment 23•21 years ago
|
||
OS/2 - bug 194571 BeOS - bug 194572
Assignee | ||
Comment 24•21 years ago
|
||
XLIB bug 194623
Assignee | ||
Comment 25•21 years ago
|
||
How do I file bugs against Photon?
Assignee | ||
Comment 26•21 years ago
|
||
Do I file against OS:Neutrino?
Comment 27•21 years ago
|
||
Does this patch helps to, for example, create new tab/window faster on page load?
Assignee | ||
Comment 28•21 years ago
|
||
This only affects interacting with Mozilla while it is loading page. This is most commonly seen when loading a very large page, then opening a new tab. So if you are seeing slow performance loading a new tab while another page is still loading then it should help. If you are seeing slow performance just opening a new tab, then this will not help.
Comment 29•21 years ago
|
||
Only thing i'm wondering now is why in nsIWidget.h in GetLastInputEventTime() description you use MILLIseconds while it must be compatible with PR_IntervalToMicroseconds () - MICROseconds. I'm confused;
Updated•5 years ago
|
Component: Event Handling → User events and focus handling
You need to log in
before you can comment on or make changes to this bug.
Description
•