Closed Bug 1694906 Opened 11 months ago Closed 11 months ago

Stop using TargetFactory::forTab and only use descriptors to open toolboxes

Categories

(DevTools :: General, task, P3)

task

Tracking

(Fission Milestone:M7, firefox88 fixed)

RESOLVED FIXED
88 Branch
Fission Milestone M7
Tracking Status
firefox88 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

(Whiteboard: dt-fission-m3-mvp)

Attachments

(5 files)

As discussed on the DevTools team channel, as we want to make this method less used, let's rename it.

Repurposing, I end up deleting all call sites to forTab in the next patch, not worth the extra review.

Summary: Rename TargetFactory::forTab to _legacyForTab and add a deprecation comment → Stop using TargetFactory::forTab and only use descriptors to open toolboxes
Blocks: 1695046
Blocks: 1695929
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/59da0caf4b8e
[devtools] Use descriptors instead of targets in all toolbox APIs r=ochameau,nchevobbe

Backed out for causing browser-chrome failures in browser_ext_addon_debugging_netmonitor.

Backout link: https://hg.mozilla.org/integration/autoland/rev/919a1dc56de51e1e215e983af4045886f365c192

Push with failures

Failure log

Flags: needinfo?(jdescottes)

Depends on D107052

As explained in the code comments added here, we now have to wait for toolbox open to be sure to have a valid target

Depends on D107099

With the previous patch, we are waiting for toolbox open before creating contexts.
This makes some tests a bit racy if they are only waiting for toolbox open. They should also wait for the context to be ready.
Firing an event from the devtools_page seems to work fine for this but don't hesitate to suggest a better approach.

Flags: needinfo?(jdescottes)
Blocks: 1644397

zombie says these patches modify a test that he will re-test and re-enable for Fission in bug 1691575.

Blocks: 1691575
Fission Milestone: --- → M7
Attachment #9206702 - Attachment description: Bug 1694906 - [devtools] Wait for devtools_page load in browser_ext_devtools_network.js tests → Bug 1694906 - [devtools] Fix race conditions in various devtools webextensions tests
Attachment #9206701 - Attachment description: Bug 1694906 - [devtools] Wait for toolbox open to create DevTools WebExtension contexts → Bug 1694906 - [devtools] Wait for toolbox-ready instead of toolbox-created in ext-devtools.js
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3b5269fe0113
[devtools] Remove remaining call sites for showToolbox in extension tests r=rpl
https://hg.mozilla.org/integration/autoland/rev/284e000deded
[devtools] Use descriptors instead of targets in all toolbox APIs r=ochameau,nchevobbe,rpl
https://hg.mozilla.org/integration/autoland/rev/02428006de51
[devtools] Add new option to createDescriptorForTab to support devtools webextensions r=rpl,ochameau
https://hg.mozilla.org/integration/autoland/rev/c23343373584
[devtools] Wait for toolbox-ready instead of toolbox-created in ext-devtools.js r=rpl
https://hg.mozilla.org/integration/autoland/rev/bd5cb6f8b41f
[devtools] Fix race conditions in various devtools webextensions tests r=rpl
Blocks: 1697375
Duplicate of this bug: 1697375
Flags: needinfo?(jdescottes)

(In reply to Narcis Beleuzu [:NarcisB] from comment #11)

Backed out for dt failures on browser_target_list_frames.js

For info I didn't manage to get the failure on 50 retriggers in the autoland push

sparky: hi! sorry for the ping, can someone from the perftest-reviewers group take a look at the small DAMP change in https://phabricator.services.mozilla.com/D106426 . This is a big refactor that blocks a few other devtools bugs and we'd like to get it in as soon as we can. Apologies for missing the DAMP dependency in the previous review cycles (DAMP push at https://treeherder.mozilla.org/jobs?repo=try&revision=814cebcf3f188a766374422d7ab4febe3ff0cb83)

Thanks!

Flags: needinfo?(gmierz2)

r+, sorry for the delay!

Flags: needinfo?(gmierz2)
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2e7d8db2d1f3
[devtools] Remove remaining call sites for showToolbox in extension tests r=rpl
https://hg.mozilla.org/integration/autoland/rev/7b9edbf658b2
[devtools] Use descriptors instead of targets in all toolbox APIs r=ochameau,nchevobbe,rpl,perftest-reviewers,sparky
https://hg.mozilla.org/integration/autoland/rev/b10a7e2b7e8c
[devtools] Add new option to createDescriptorForTab to support devtools webextensions r=rpl,ochameau
https://hg.mozilla.org/integration/autoland/rev/962ff8dd112f
[devtools] Wait for toolbox-ready instead of toolbox-created in ext-devtools.js r=rpl
https://hg.mozilla.org/integration/autoland/rev/969dd6958089
[devtools] Fix race conditions in various devtools webextensions tests r=rpl
Regressions: 1697354
Whiteboard: dt-fission-m3-mvp
You need to log in before you can comment on or make changes to this bug.