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

Status: RESOLVED DUPLICATE of bug 303987

(Core :: Layout, defect, P3)

Description

[timeless]

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.

Comment 1

[Boris Zbarsky [:bzbarsky]]

So... I think what we may need is for the pref observer API should have
beginchange/endchange notifications....

Comment 2

[Boris Zbarsky [:bzbarsky]]

Oh, and we should not be reconstructing the frame tree for font changes.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Maybe we should reflow asynchronously off a reflow event?

Comment 4

[timeless]

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.

Comment 5

[Wayne Mery (:wsmwk)]

still applicable/valid?

Comment 6

[Boris Zbarsky [:bzbarsky]]

This was fixed in bug 303987.