preference changes cause layout to reflow and reflow and reflow ...

RESOLVED DUPLICATE of bug 303987

Status

()

P3
major
RESOLVED DUPLICATE of bug 303987
16 years ago
4 years ago

People

(Reporter: timeless, Unassigned)

Tracking

(Depends on: 1 bug, {arch, perf})

Trunk
Future
x86
Windows 2000
arch, perf
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

16 years ago
Ok. so i did something stupid. i made a lot of maybe changes to the fonts pref
panel, or maybe i only looked at the different panes, i don't think i actually
changed the values, but how should i remember? i hadn't touched the prefence
dialog since i opened it a few sleep cycles ago. and now my gecko is stuck
reflowing a lot. because as soon as it finishes reflowing everything (for a pref
which didn't matter because the font wasn't in any of my pages anyway) it gets
to do it all over again for the next preference. -- this is stupid.

What's happening is that each font pref change is causing a stack that looks
like this:

PresShell::ReconstructFrames() line 5361 + 41 bytes
PresShell::SetPreferenceStyleRules(PresShell * const 0x06c1c570, int 1) line 2223
nsPresContext::PreferenceChanged(const char * 0x05428768) line 623
nsPresContext::PrefChangedCallback(const char * 0x05428768, void * 0x08276b10)
line 107
pref_DoCallback(const char * 0x05428768) line 1187 + 17 bytes
pref_HashPref(const char * 0x05428768, PrefValue {...}, PrefType PREF_STRING,
PrefAction PREF_SETUSER) line 1073 + 9 bytes
PREF_SetCharPref(const char * 0x05428768, const char * 0x0012e91c) line 551 + 17
bytes
nsPrefBranch::SetCharPref(nsPrefBranch * const 0x00f44f98, const char *
0x05428768, const char * 0x0012e91c) line 230 + 13 bytes
nsPrefBranch::SetComplexValue(nsPrefBranch * const 0x00f44f98, const char *
0x05428768, const nsID & {...}, nsISupports * 0x07d8ab58) line 474 + 59 bytes
nsPrefService::SetComplexValue(nsPrefService * const 0x00f44f38, const char *
0x05428768, const nsID & {...}, nsISupports * 0x07d8ab58) line 57 + 42 bytes
nsPref::SetUnicharPref(nsPref * const 0x00f44e60, const char * 0x05428768, const
unsigned short * 0x07d4e128) line 426 + 42 bytes
XPTC_InvokeByIndex(nsISupports * 0x00f44e60, unsigned int 37, unsigned int 2,
nsXPTCVariant * 0x0012edd8) line 106
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode
CALL_METHOD) line 2016 + 42 bytes
XPC_WN_CallMethod(JSContext * 0x06697968, JSObject * 0x06501b70, unsigned int 2,
long * 0x07e53dd4, long * 0x0012f07c) line 1283 + 14 bytes
js_Invoke(JSContext * 0x06697968, unsigned int 2, unsigned int 0) line 839 + 23
bytes
js_Interpret(JSContext * 0x06697968, long * 0x0012f998) line 2803 + 15 bytes
js_Invoke(JSContext * 0x06697968, unsigned int 2, unsigned int 2) line 856 + 13
bytes
js_InternalInvoke(JSContext * 0x06697968, JSObject * 0x0588e4f8, long 91142216,
unsigned int 0, unsigned int 2, long * 0x07a2c7d0, long * 0x0012fac4) line 931 +
20 bytes
JS_CallFunctionValue(JSContext * 0x06697968, JSObject * 0x0588e4f8, long
91142216, unsigned int 2, long * 0x07a2c7d0, long * 0x0012fac4) line 3431 + 31 bytes
nsJSContext::CallEventHandler(nsJSContext * const 0x06f6ed90, void * 0x0588e4f8,
void * 0x056eb848, unsigned int 2, void * 0x07a2c7d0, int * 0x0012fb5c, int 0)
line 1041 + 33 bytes
GlobalWindowImpl::RunTimeout(nsTimeoutImpl * 0x074b49c0) line 4625 + 84 bytes
GlobalWindowImpl::TimerCallback(nsITimer * 0x07c81608, void * 0x074b49c0) line 4976
nsTimerImpl::Fire() line 367 + 17 bytes
nsTimerManager::FireNextIdleTimer(nsTimerManager * const 0x014b3f90) line 595
nsAppShell::Run(nsAppShell * const 0x00f864d0) line 177
nsAppShellService::Run(nsAppShellService * const 0x00f8d728) line 472
main1(int 3, char * * 0x002844d0, nsISupports * 0x00276f08) line 1541 + 32 bytes
main(int 3, char * * 0x002844d0) line 1902 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e87903()

And there are *lots* of them.  There has to be a way for preferences to say
"warning: we're about to send you a batch of preference changes, you might not
want to do something complicated until we're done".

+	aPrefName	0x05428768 "font.name.sans-serif.th"
+	aPrefName	0x07958078 "font.name.cursive.th"
+	aPrefName	0x07016ed8 "font.name.fantasy.th"
+	aPrefName	0x0796fbc0 "font.name.monospace.th"
+	aPrefName	0x07b702f8 "font.name.cursive.he"
+	aPrefName	0x09d56bd8 "font.name.cursive.ar"
+	aPrefName	0x09d56c88 "font.name.fantasy.ar"
+	aPrefName	0x073b2328 "font.language.group"

This is the list of prefs that changed after i spent time investigating

note that at some point i got sick of doing it so i poked the code so that it
cmp , 1 <- instead of cmp, 0
and 
jne     <- instead of je

that meant that i actually left the loop in a relatively timely manner.

You can either batch the changes or be smart enough to ignore changes that don't
actually affect a given pres context. i don't care which is done, perhaps both
should be.
So... I think what we may need is for the pref observer API should have
beginchange/endchange notifications....
Oh, and we should not be reconstructing the frame tree for font changes.
Depends on: 168326
Maybe we should reflow asynchronously off a reflow event?
(Reporter)

Comment 4

16 years ago
i like that idea. we should probably revoke and reissue the event each time we
get a new prefchange notification because the js processing might sleep to allow
things to breathe which could allow the presshell to start reflowing before all
of the pref changes happen.

Updated

16 years ago
Priority: -- → P3
Target Milestone: --- → Future
Assignee: layout → nobody
QA Contact: ian → layout

Comment 5

4 years ago
still applicable/valid?
Flags: needinfo?(bzbarsky)
This was fixed in bug 303987.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(bzbarsky)
Resolution: --- → DUPLICATE
Duplicate of bug: 303987
You need to log in before you can comment on or make changes to this bug.