472363 - mSPTimerLock in nsWebShellWindow isn't needed

Status: RESOLVED INACTIVE

(Core :: XUL, defect)

Description

[Nickolay_Ponomarev]

I'm not sure why it was added (http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/xpfe/appshell/src&command=DIFF_FRAMESET&file=nsWebShellWindow.cpp&rev2=1.275&rev1=1.274), but it shouldn't be needed now: timer callbacks run on main thread, so the only thing this lock guards is SetPersistenceTimer/FirePersistenceTimer reentrancy and a lock is a rather heavyweight mechanism for that.

Comment 1

[Nickolay_Ponomarev]

Attachment: patch

Comment 2

[Nickolay_Ponomarev]

Attachment: an example changing window state while FirePersistenceTimer is running

This calls window.restore() in response to a mutation event caused by nsXULWindow::SavePersistentAttributes() setting "sizemode" attribute.

window.restore() ends up in nsWebShellWindow::SetPersistenceTimer().

Comment 3

[Nickolay_Ponomarev]

The patch also causes changes made to window position/size/mode from such mutation listeners to be correctly persisted.

Comment 4

[Josh Matthews [:jdm]]

This patch is quite bitrotted.  I'm presuming it's something we still want, however.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 6

[Nickolay_Ponomarev]

This was a peculiarity I had noticed in the code back at the time, with (almost) no visible effect.

Since no-one got around to landing it in 14 years that passed, the chances of that happening in the future are almost zero, so I'm closing it to get it off everyone's radar.