Closed Bug 614089 Opened 9 years ago Closed 9 years ago

SSTabRestored should be fired for each tab

Categories

(Firefox :: Session Restore, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 4.0b8

People

(Reporter: zpao, Assigned: zpao)

References

Details

Attachments

(1 file)

Currently SSTabRestored is only fired if a tab successfully fills in form data when restoring. However SSTabRestoring is fired at the beginning for each tab. This is pretty inconsistent and so SSTabRestored is a pretty useless event.

Some sessionstore tests currently depend on the # of location changes to determine if state has been restored. With bug 599909 we'll be triggering more onLocationChanges (up to 3 per tab).

So I'm proposing we actually make use of this event the way it should be used. It should be fired when we're done restoring each tab. This means at the end of the load/form fill (if a load is going to happen) or right away in the case of tabs with only userTypedValue.
Attached patch Patch v0.1Splinter Review
No explicit tests, but I think the coverage from use in other tests is good enough (particularly the changes to browser_522545.js). A bunch of these tests were suddenly having issues with the change in bug 599909.

394759:
Instead of waiting for a single tab's SSTabRestored, we'll wait for one from each tab. This eliminates the potential race condition (since we'd be setting currentURI to about:blank explicitly with 599909)

522545:
As I mentioned in bug 599909 comment 14, we're now triggering more onLocationChanges than before so it's an unreliable way to determine restoring state. We also can't just multiply by 3 because state without an active tab won't trigger the same number of onLocationChanges. One of the tests was already special casing because that method wasn't completely reliable, but by using SSTabRestored, all is good.

590268:
Similar to 394759, it just makes it so that we're completely sure of our state before moving on.
Attachment #493069 - Flags: review?(dietrich)
Comment on attachment 493069 [details] [diff] [review]
Patch v0.1

this looks ok, r=me. not sure if we need to call this out for docs or not. if anyone was listening, they'll be getting more notifications now than before, and perhaps unexpectedly.
Attachment #493069 - Flags: review?(dietrich) → review+
(In reply to comment #2)
> Comment on attachment 493069 [details] [diff] [review]
> Patch v0.1
> 
> this looks ok, r=me. not sure if we need to call this out for docs or not. if
> anyone was listening, they'll be getting more notifications now than before,
> and perhaps unexpectedly.

Well, so devmo already said "An event of type SSTabRestored fires each time a tab has finished restoring." So we're really just making that a reality.
http://hg.mozilla.org/mozilla-central/rev/6309e9c330c0
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b8
Is there a way I can verify this bug?
(In reply to comment #5)
> Is there a way I can verify this bug?

You could write an extension but that seems a bit overkill. Otherwise not really.
Status: RESOLVED → VERIFIED
Depends on: 684777
You need to log in before you can comment on or make changes to this bug.