Port |Bug 636279 - _tabsRestoringCount goes negative if setBrowserState called at browser startup and last session had pinned tab(s)| to SeaMonkey

RESOLVED FIXED in seamonkey2.10

Status

SeaMonkey
Session Restore
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Misak Khachatryan, Assigned: Misak Khachatryan)

Tracking

SeaMonkey 2.9 Branch
seamonkey2.10

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

4.89 KB, patch
neil@parkwaycc.co.uk
: review+
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
Created attachment 602282 [details] [diff] [review]
patch

No so relevant to SeaMoneky, just good to be in sync. From Parent bug:

Someone reported to me that sessions being restored by my Session Manager add-on at start up where ignoring the max_concurrent_tabs preference which confused me since that's all handled internally by Firefox.

I added a ton of logging statements to nsSessionStore.js and tracked the problem down to calling setBrowserState start shortly after start up when the previous browser session had a pinned app tab in it.

What happens is that when the browser is startup up and there were previously pinned tabs, then the restoreWindow is called from onLoad.  This cycles down until it reaches restoreHistory where, because the selected browser is the current browser, restoreTab will be called which will increment _tabsRestoringCount to 1.  The problem here is that __resetTabRestoringStateisn't called for for that tab.

When setBrowserState is called, the _tabsRestoringCount is reset back to 0 by the call to _resetRestoringState.  It is then decremented to -1 by the call to _resetTabRestoringState from within the restoreWindow function (this is the bug).  The _tabsRestoringCount is only decremented once, no matter how many pinned tabs there were.  So at this point _tabsRestoringCount is -1.

What happens next, depends on how many tabs are being restored in the session, how many pinned tabs there were and what the max_concurrent_tabs preference is.  Some things will work correctly, other won't.  For example if max_concurrent_tabs is set to 0, then if there is more than one more tab restoring than there were pinned app tabs, all the tabs load simultaneously.  That's the extreme case.  In most cases, the number of concurrently loading tabs will be off by one, meaning restoreNextTab could be called too many times and/or too often.

The root of the problem though is that _tabsRestoringCount should not be going negative.
Attachment #602282 - Flags: review?(neil)

Updated

6 years ago
Attachment #602282 - Flags: review?(neil) → review+
(Assignee)

Comment 1

6 years ago
http://hg.mozilla.org/comm-central/rev/f0fdd72ac05e
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.10
Version: Trunk → SeaMonkey 2.9 Branch
You need to log in before you can comment on or make changes to this bug.