1226333 - Add tests for async window flushing

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

Bug 1171708 gave us asynchronous window flushing for window close. Some tests were written, but those tests failed intermittently. We should figure out why the tests fail intermittently and reland them.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Bug 1226333 - Add tests for async window flushing. r=billm.

Bug 1226333 - Add tests for async window flushing. r=billm

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8689689 [details]
MozReview Request: Bug 1226333 - Add tests for async window flushing. r=billm.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25689/diff/1-2/

Comment 3

[Mike Conley (:mconley) (:⚙️)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=34bcc9ee3db7

Comment 4

[Mike Conley (:mconley) (:⚙️)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a1e75e6e6f3d

Comment 5

[Mike Conley (:mconley) (:⚙️)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c67eb506ae0b

Comment 6

[Mike Conley (:mconley) (:⚙️)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ae2be89920d

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8689689 [details]
MozReview Request: Bug 1226333 - Add tests for async window flushing. r=billm.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25689/diff/2-3/

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Finally got these to not fail intermittently. The key was to set the state in the parent to send the browser to example.com in test_remove_uninteresting_window (see the interdiff: https://reviewboard.mozilla.org/r/25689/diff/2-3/).

billm, you already signed off on this, but it got backed out due to the failures. Now that I've fixed them, still feel okay signing off on this?

Comment 9

[Mike Conley (:mconley) (:⚙️)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb83cf3ba517

Comment 10

[Mike Conley (:mconley) (:⚙️)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=33aa7900312f

Comment 11

[Mike Conley (:mconley) (:⚙️)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0261deb3d8ef

Comment 12

[Mike Conley (:mconley) (:⚙️)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d5c28d708e2a

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Okay, I finally understand what's happening here.

My patch adds this pref, browser.sessionstore.debug.no_auto_updates, which tries to side-step potential races by having the parent ignore non-flush, non-final updates from the child.

The problem with that is that if a message comes up from the child and is ignored, *the child still thinks it sent it*, so further updates from the child (from a flush, for example) won't include the data that it thinks it had already sent.

I believe the solution is to move the browser.sessionstore.debug.no_auto_updates pref from SessionStore.jsm into content-sessionStore.js, where it will just prevent us from setting timeouts (and cancel pending timeouts) on the MessageQueue until re-enabled.

Comment 14

[Mike Conley (:mconley) (:⚙️)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4a6aab7b8f9d

Comment 15

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8689689 [details]
MozReview Request: Bug 1226333 - Add tests for async window flushing. r=billm.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25689/diff/3-4/

Comment 16

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8689689 [details]
MozReview Request: Bug 1226333 - Add tests for async window flushing. r=billm.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25689/diff/4-5/

Comment 17

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8689689 [details]
MozReview Request: Bug 1226333 - Add tests for async window flushing. r=billm.

https://reviewboard.mozilla.org/r/25689/#review24559

Thanks for tracking this down!

Comment 18

[Mike Conley (:mconley) (:⚙️)]

https://hg.mozilla.org/integration/fx-team/rev/1a8e8ab6ee3dacc69e865879a4eb6739904d57b1
Bug 1226333 - Add tests for async window flushing. r=billm.

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1a8e8ab6ee3d

Comment 20

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bugnotes

http://people.mozilla.org/~mconley2/bugnotes/bug-1226333.html