614089 - SSTabRestored should be fired for each tab

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect)

Description

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

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.

Comment 1

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.1

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.

Comment 2

[Dietrich Ayala (: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.

Comment 3

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(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.

Comment 4

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

http://hg.mozilla.org/mozilla-central/rev/6309e9c330c0

Comment 5

[u279076]

Is there a way I can verify this bug?

Comment 6

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(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.