1894556 - Need to replace idleTasksFinishedPromise with idleTasksFinished.promise

Status: RESOLVED FIXED

(Firefox :: General, task, P2)

Description

[Steve P]

Steps to reproduce:

After moving gBrowserInit to a new file, this variable is no longer needed and can be replaced. Here are the occurrences to change them.
https://searchfox.org/mozilla-central/search?q=idleTasksFinishedPromise&path=&case=false&regexp=false

Comment 1

[:Gijs (he/him)]

Hi Steve, are you interested in working on this as well, now that bug 1880909 has successfully landed? :-)

Comment 2

[Steve P]

Yes I am! It seems like it should be pretty straightforward, just replace those occurrences? Is there anything else I need to keep in mind when I go through and make the changes?

Comment 3

[:Gijs (he/him)]

(In reply to Steve P from comment #2)

Yes I am! It seems like it should be pretty straightforward, just replace those occurrences? Is there anything else I need to keep in mind when I go through and make the changes?

I don't think so, though we'll want to push to try and check we haven't broken any tests - there are a few framework bits for both talos (performance) and mochitest (integration/unit) tests that rely on this API, it would seem. It should be fine as it's fairly mechanical, but that's part of why I wanted to split it off from the other change. :-)

Comment 4

[Steve P]

Attachment: Bug 1894556 - Need to replace idleTasksFinishedPromise with idleTasksFinished.promise. r?Gijs

Comment 5

[Steve P]

All instances are replaced. I did the same tests as before, with opening tabs and dragging them in and out of different windows, all seems to be working as expected. Are there any actual tests you'd like me to run?

Comment 6

[:Gijs (he/him)]

(In reply to Steve P from comment #5)

All instances are replaced. I did the same tests as before, with opening tabs and dragging them in and out of different windows, all seems to be working as expected. Are there any actual tests you'd like me to run?

I think this is fine - I added some tests to the treeherder job that your phabricator push created: https://treeherder.mozilla.org/jobs?repo=try&revision=c5ebac3ece5ec88828c197ffe2b010ec1d40d276

In a few hours they should have completed, but I don't expect any problems either way. Please do request review / take the patch out of WIP - it looks good to me. :-)

Comment 7

[Steve P]

Thanks Gijs! I've updated the patch from WIP, but it doesn't seem to be adding you as a reviewer.

Comment 8

[:Gijs (he/him)]

(In reply to Steve P from comment #7)

Thanks Gijs! I've updated the patch from WIP, but it doesn't seem to be adding you as a reviewer.

I've just reviewed it anyway. ;-)

Comment 9

[Steve P]

Awesome, thanks for all your help!

If this patch is complete, do you know of any bugs you might suggest for me to move on to from here? :)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9b24b8dd1c0d
Need to replace idleTasksFinishedPromise with idleTasksFinished.promise. r=perftest-reviewers,Gijs,sparky

Comment 11

[:Gijs (he/him)]

(In reply to Steve P from comment #9)

Awesome, thanks for all your help!

If this patch is complete, do you know of any bugs you might suggest for me to move on to from here? :)

How about bug 1882776 ? It'll be a slight step up in terms of difficulty, but I'm confident you're up to it. :-)

Comment 12

[Steve P]

I'll be happy to take a shot at it, thanks!

Comment 13

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/9b24b8dd1c0d