Closed Bug 1365607 Opened 7 years ago Closed 3 years ago

[meta] Switch devtools to async/await from task.js/yield

Categories

(DevTools :: General, enhancement, P5)

enhancement

Tracking

(firefox91 fixed)

RESOLVED FIXED
91 Branch
Tracking Status
firefox91 --- fixed

People

(Reporter: tromey, Assigned: hexagonrecursion)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: meta)

Attachments

(5 files)

Like bug 1353542 did for other parts of the browser, it would be good
to switch devtools from Task to real async+await.
That bug has some scripts used for the conversion.
Also, Florian offered to help a bit in https://bugzilla.mozilla.org/show_bug.cgi?id=1353542#c8
DevTools bug triage (filter on CLIMBING SHOES).
Priority: -- → P3
it might be better doing this at the same time as bug 1414177.
Depends on: 1416959
Here's a tweaked version of Florian's conversion script that I was able to run on RDM.

To use:

<obj-dir>/dist/bin/xpcshell <script> <directory to convert>
Also try running:

To use:

<obj-dir>/dist/bin/xpcshell <script> <directory to convert> --replace-generators

as a second pass to replace non-task generators (which is usually correct, but may require more review).
Depends on: 1437828
Depends on: 1440321
Depends on: 1440322
Keywords: meta
Priority: P3 → P5
Summary: Switch devtools to async/await from Task.jsm/yield → [meta] Switch devtools to async/await from Task.jsm/yield
Depends on: 1440320
Depends on: 1448090
Depends on: 1448111
Quick summary of Task.jsm refactoring.

All devtools folders are now converted to async/await!

All? No, almost all. I ignored gcli, old webconsole tests and old debugger. Which are planned to be dropped.
And various tests that started failing when converted to async/await. They are all tracked in dependent bugs.
No longer depends on: 1444755
Note: As of bug 1446833 the remaining devtools tests added as generators are not being run (for instance, ./mach test devtools/client/memory/test/unit/test_action-toggle-inverted.js succeeds with only a single subtest).

I discovered this while investigating bug 1451333 and thought you might want to know :)
Thanks for the heads up.
I didn't particularly chased the "function*" occurences, we probably should as a second part of this meta bug.

Also, there is a couple of untracked usages of Task still:

* Docs still mention Task
  devtools/docs/contributing/javascript.md
  devtools/docs/tests/writing-tests.md
* scratchpad uses it (I may have blacklisted it while doing the big automated patches):
  devtools/client/scratchpad/test/browser_scratchpad_autocomplete.js:    .then(Task.async(runTests))
  devtools/client/scratchpad/test/browser_scratchpad_close_toolbox.js:    .then(Task.async(runTests))
  devtools/client/scratchpad/test/browser_scratchpad_disable_view_menu_items.js:    .then(Task.async(runTests))
  devtools/client/scratchpad/test/browser_scratchpad_inspect_primitives.js:    .then(Task.async(runTests))
* a couple of tests uses it in framework:
  devtools/client/framework/test/browser_toolbox_tool_remote_reopen.js
  devtools/client/framework/test/browser_target_support.js
  devtools/client/framework/test/browser_toolbox_tool_ready.js
* this test only mention Task in a comment:
  devtools/client/framework/toolbox-highlighter-utils.js
(In reply to Alexandre Poirot [:ochameau] from comment #8)
> Thanks for the heads up.
> I didn't particularly chased the "function*" occurences, we probably should
> as a second part of this meta bug.

You should also look for 'yield' if you weren't already.

Note that bug 1446833 only affected xpcshell tests, not mochitests.
Depends on: 1459567
Product: Firefox → DevTools
No longer blocks: dt-fission
The cleanup is almost finished.
There is only one last usage in production code:
  https://searchfox.org/mozilla-central/source/devtools/client/shared/redux/middleware/task.js#25
  This code should be easy to remove as no other code should be producing Task generators... They should all be async functions now.

Otherwise, it is only tests, mostly old debugger tests and a couple of comments still referring to Tasks:
  https://searchfox.org/mozilla-central/search?q=%5CsTask%5C.&case=true&regexp=true&path=devtools%2F

Finally, we should be able to remove the task module:
  https://searchfox.org/mozilla-central/source/devtools/shared/task.js
Summary: [meta] Switch devtools to async/await from Task.jsm/yield → [meta] Switch devtools to async/await from task.js/yield
Depends on: 1674971

Here is one more implementation of Task https://searchfox.org/mozilla-central/source/devtools/client/debugger/src/utils/task.js
I can not find any places where this code is used. Can this be removed?

(In reply to Alexandre Poirot [:ochameau] from comment #10)

https://searchfox.org/mozilla-central/source/devtools/client/shared/redux/
middleware/task.js#25
This code should be easy to remove as no other code should be producing
Task generators... They should all be async functions now.

Nope. There are still dispatch() calls and asynchronous redux actions that depend on this middleware. Example:
https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/devtools/client/memory/actions/filter.js#24

Thanks Andrey for looking into this!

(In reply to Andrey Bienkowski from comment #12)

Here is one more implementation of Task https://searchfox.org/mozilla-central/source/devtools/client/debugger/src/utils/task.js
I can not find any places where this code is used. Can this be removed?

I didn't know about this other one.
Same, I couldn't see any usage of it

(In reply to Andrey Bienkowski from comment #13)

(In reply to Alexandre Poirot [:ochameau] from comment #10)

https://searchfox.org/mozilla-central/source/devtools/client/shared/redux/
middleware/task.js#25
This code should be easy to remove as no other code should be producing
Task generators... They should all be async functions now.

Nope. There are still dispatch() calls and asynchronous redux actions that depend on this middleware. Example:
https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/devtools/client/memory/actions/filter.js#24

I thought I got rid of all the generators in some dependencies of this bug...
It looks like I was incomplete in my generator's refactoring.

Assignee: nobody → hexagonrecursion
Status: NEW → ASSIGNED

So long and thanks for all the fish!

Depends on D117175

Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d0c52e87ef5d
Convert to async/await in devtools/client/memory/actions/filter.js r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/292be5a99a63
Refactor devtools/client/memory/actions/filter.js r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/f6fafb901311
Stop using devtools/shared/task from the task middleware r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/f2e64b982c7c
Delete devtools/shared/task.js r=jdescottes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: