1391117 - sessionstore browser-chrome tests are going to permaleak on Windows when Gecko 57 merges to Beta on 2017-09-20

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

[Tracking Requested - why for this release]: Permaleaking tests on the next merge day.

Try bisection confirms that this was caused by bug 1388145. I didn't bisect through all the patches, but the first one in the series looks suspicious. Can you please take a look, Felipe?

https://treeherder.mozilla.org/logviewer.html#?job_id=123440025&repo=try
https://treeherder.mozilla.org/logviewer.html#?job_id=123441865&repo=try

TEST-UNEXPECTED-FAIL | leakcheck | default process: 9552 bytes leaked (BackstagePass, CondVar, IdlePeriod, Mutex, SharedScriptableHelperForJSIID, ...)

Comment 1

[:Felipe Gomes (needinfo for replies!)]

oh boy

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Ugh, now these same leaks are hitting the browser/extensions/shield-recipe-client/test directory on OSX/Win :(

Comment 3

[:Felipe Gomes (needinfo for replies!)]

I believe the root cause for this is bug 1392648, but it still would be nice to prevent the code from running into this situation (what happens is that some initialization tasks are running twice due to the sessionstore-windows-restored notification being sent again during tests).

So let's add a simple patch to prevent that.

Comment 4

[:Felipe Gomes (needinfo for replies!)]

Attachment: Bug 1391117 - Prevent nsBrowserGlue's initialization tasks from running twice in case its notification is sent again.

Review commit: https://reviewboard.mozilla.org/r/173382/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/173382/

Comment 5

[:Felipe Gomes (needinfo for replies!)]

Hey Ryan, if you get a chance, could you do a try push with this patch and without bug 1392648 to verify if this would independently fix the issue?

Comment 6

[Mike de Boer [:mikedeboer]]

Comment on attachment 8901944 [details]
Bug 1391117 - Prevent nsBrowserGlue's initialization tasks from running twice in case its notification is sent again.

https://reviewboard.mozilla.org/r/173382/#review178972

::: commit-message-128a7:1
(Diff revision 1)
> +Bug 1391117 - Prevent nsBrowserGlue's initialization tasks from running twice in case its notification is sent again. r=mdeboer

Huuuuuge nit: my nick is 'mikedeboer'!!!1

;-P

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Yes, this fixes the leaks by itself for both directories.

Comment 8

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8901944 [details]
Bug 1391117 - Prevent nsBrowserGlue's initialization tasks from running twice in case its notification is sent again.

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

Comment 9

[:Felipe Gomes (needinfo for replies!)]

Awesome, thanks!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/931ed60cd313
Prevent nsBrowserGlue's initialization tasks from running twice in case its notification is sent again. r=mikedeboer

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/931ed60cd313