Closed Bug 732344 Opened 12 years ago Closed 12 years ago

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

Categories

(SeaMonkey :: Session Restore, defect)

SeaMonkey 2.9 Branch
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.10

People

(Reporter: misak.bugzilla, Assigned: misak.bugzilla)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
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)
Attachment #602282 - Flags: review?(neil) → review+
http://hg.mozilla.org/comm-central/rev/f0fdd72ac05e
Status: ASSIGNED → RESOLVED
Closed: 12 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.

Attachment

General

Created:
Updated:
Size: