Last Comment Bug 558639 - Port Bug 531519 [getBrowserState sometimes returns two entries for one window] to Seamonkey
: Port Bug 531519 [getBrowserState sometimes returns two entries for one window...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Session Restore (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1a1
Assigned To: Misak Khachatryan
:
:
Mentors:
Depends on: 531519
Blocks:
  Show dependency treegraph
 
Reported: 2010-04-11 03:03 PDT by Misak Khachatryan
Modified: 2010-08-26 09:07 PDT (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (1.60 KB, patch)
2010-04-11 03:03 PDT, Misak Khachatryan
misak.bugzilla: review+
misak.bugzilla: superreview+
Details | Diff | Splinter Review
patch v2 (982 bytes, patch)
2010-04-11 10:00 PDT, Misak Khachatryan
neil: review+
neil: superreview+
dao+bmo: feedback-
Details | Diff | Splinter Review

Description Misak Khachatryan 2010-04-11 03:03:11 PDT
Created attachment 438335 [details] [diff] [review]
patch

From parent bug:

http://hg.mozilla.org/mozilla-central/rev/4a7f5376270a exposed a constant
failure on all platforms:
TEST-UNEXPECTED-FAIL |
chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_526613.js
| number of open browser windows according to getBrowserState - Got 3, expected
2

This happens even when running browser_526613.js alone.

I think the second window (from _windows) and the third one (from
_statesToRestore) are the same, but I'm not sure.

I ran test on my Fedora 12, no failed test seen.
Comment 1 neil@parkwaycc.co.uk 2010-04-11 04:55:19 PDT
Comment on attachment 438335 [details] [diff] [review]
patch

>+      if (this._windows[ix]._restoring) // window data is still in _statesToRestore
>+        continue;
Does checking (this._windows[ix].__SS_restoreID in this._statesToRestore) work? (That would avoid adding the extra _restoring flag.)
Comment 2 Misak Khachatryan 2010-04-11 10:00:06 PDT
Created attachment 438368 [details] [diff] [review]
patch v2

Well, seems to work, but as our browser_526613.js didn't fail on my Fedora 12, I'm requesting feedback from  Dão Gottwald, maybe he can add useful comment here.
Comment 3 neil@parkwaycc.co.uk 2010-04-11 10:06:55 PDT
Comment on attachment 438368 [details] [diff] [review]
patch v2

Let's prefer to go with this version if possible.
Comment 4 Dão Gottwald [:dao] 2010-04-12 03:35:34 PDT
Comment on attachment 438368 [details] [diff] [review]
patch v2

Looks wrong to me. __SS_restoreID is set on dom windows, this._windows[ix] is not a dom window.
Comment 5 Misak Khachatryan 2010-04-12 07:16:06 PDT
Comment on attachment 438335 [details] [diff] [review]
patch

Thank You very much Dão, unobsoleting previous patch and setting r+ and sr+, as discussed with Neil on IRC.
Comment 6 Misak Khachatryan 2010-04-17 06:07:04 PDT
checking ping
Comment 7 Misak Khachatryan 2010-04-20 00:01:27 PDT
Pushed http://hg.mozilla.org/comm-central/rev/59177a9718ca
Comment 8 Misak Khachatryan 2010-08-26 07:50:05 PDT
Hmm, looks like i checked in wrong patch ...

Note You need to log in before you can comment on or make changes to this bug.