1568157 - Replace toolbox.{inspector,walker,selection,highlighter} with something based on target.getInspector() or target.getFront("inspector")

Status: RESOLVED FIXED

(DevTools :: Inspector, enhancement, P1)

Description

[Alexandre Poirot [:ochameau]]

There attributes on the toolbox: inspector, highlighter, walker, selection:
https://searchfox.org/mozilla-central/source/devtools/client/framework/toolbox.js#487-517
should be removed.

The usage of these attributes is overly convoluted.
You somehow have to ensure that toolbox.initInspector() has been called before you try using them.
Also, this is related to bug 1568151 as that's another place, where we store a unique reference to only the top level tab document's inspector front. This will most likely be an issue for fission.
Bug 1568151 may actually fix a couple of places where we use these toolbox attribute, but most likely not all of them.
In the few left, we would still have to remove them.

As of today, we would replace them like this:

• toolbox.inspector => await target.getInspector()
• toolbox.walker => (await target.getInspector()).walker
• toolbox.highlighter => (await target.getInspector()).highlighter
• toolbox.selection => (await target.getInspector()).selection
  And you would no longer have to ensure that toolbox.initInspector is called!

Note that bug 1568151 is also related to this. If bug 1568151 lands before this work,
we would have to replace getInspector() by getFront("inspector").

Comment 1

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1568157 - Part 1: Replace `toolbox.inspectorFront` with the `inspectorFront` from `target.getFront("inspector")`. r=yulia,ochameau

Comment 2

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1568157 - Part 2: Replace `toolbox.{inspector,walker,selection,highlighter}` usage with the attributes from `target.getFront("inspector")` in inspector.js. r=yulia,ochameau

Depends on D40313

Comment 3

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1568157 - Part 3: Replace `toolbox.selection` with the `selection` from `target.getFront("inspector")`. r=yulia,ochameau

Depends on D40314

Comment 4

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1568157 - Part 3: Replace `toolbox.highlighter` with the contextual HighlighterFront. r=yulia,ochameau

Depends on D40315

Comment 5

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1568157 - Part 4: Replace `toolbox.walker` with the contextual WalkerFront. r=yulia,ochameau

Depends on D40316

Comment 6

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1568157 - Part 6: Replace `toolbox.inspectorFront.nodePicker` with `nodePicker` from `target.getFront("inspector"). r=yulia,ochameau

Also, removes the inspectorFront getter in toolbox.js.

Depends on D40317

Comment 7

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1568157 - Part 7: Remove `highlighter-ready` and `highlighter-hide` events from the walker. r=yulia,ochameau

It doesn't seem like we run into any test failures from removing these events.
So, this is an attempt to remove them.

Depends on D40318

Comment 9

[Gabriel Luong [:gl] (ΦωΦ)]

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

Comment 10

[Gabriel Luong [:gl] (ΦωΦ)]

This patch series depend on Bug 1568151.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4ed2ae72d108
Part 1: Replace `toolbox.inspectorFront` with the `inspectorFront` from `target.getFront("inspector")`. r=yulia,ochameau

Comment 12

[Andreea Pavel [:apavel]]

Backed out for multiple dt failrues

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=4ed2ae72d1083cd3e31288546c99d83eb479c61d

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&searchStr=linux64%2Cdevtools&revision=b1aef4fe06b176c13035904604273426d352b61d&selectedJob=260230566

Failure logs: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=260230566&repo=autoland&lineNumber=2028
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=260229859&repo=autoland&lineNumber=5304
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=260229882&repo=autoland&lineNumber=13467

Backout: https://hg.mozilla.org/integration/autoland/rev/562d312e70e2dfebbfb32fc6194bb24f51d321d6

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a40f7db7b32
Part 1: Replace `toolbox.inspectorFront` with the `inspectorFront` from `target.getFront("inspector")`. r=yulia,ochameau
https://hg.mozilla.org/integration/autoland/rev/c9ba1246ad59
Part 2: Replace `toolbox.{inspector,walker,selection,highlighter}` usage with the attributes from `target.getFront("inspector")` in inspector.js. r=yulia,ochameau

Comment 14

[Gabriel Luong [:gl] (ΦωΦ)]

(In reply to Andreea Pavel [:apavel] from comment #12)

Backed out for multiple dt failrues

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=4ed2ae72d1083cd3e31288546c99d83eb479c61d

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&searchStr=linux64%2Cdevtools&revision=b1aef4fe06b176c13035904604273426d352b61d&selectedJob=260230566

Failure logs: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=260230566&repo=autoland&lineNumber=2028
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=260229859&repo=autoland&lineNumber=5304
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=260229882&repo=autoland&lineNumber=13467

Backout: https://hg.mozilla.org/integration/autoland/rev/562d312e70e2dfebbfb32fc6194bb24f51d321d6

Relanded.

Comment 15

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/7a40f7db7b32
https://hg.mozilla.org/mozilla-central/rev/c9ba1246ad59

Comment 16

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1568157 - Part 5: Move the NodePicker initialization into a getter. r=yulia,ochameau

Depends on D40317

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c2546c971118
Part 3: Replace `toolbox.highlighter` with the contextual HighlighterFront. r=yulia

Comment 18

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/c2546c971118

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/949a3bb4cab0
Part 4: Replace `toolbox.walker` with the contextual WalkerFront. r=yulia
https://hg.mozilla.org/integration/autoland/rev/8231b28c7507
Part 5: Move the NodePicker initialization into a getter. r=yulia

Comment 20

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/949a3bb4cab0
https://hg.mozilla.org/mozilla-central/rev/8231b28c7507