1497264 - Remove AddonTargetActor for legacy extension

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

We still have code in devtools to connect to legacy extensions. They are still used internally within Mozilla for a few privileged bootstrapped extensions (should go away in 65) as well as for some system addons.

- Bug 1449052 : remove bootstrapped addons
- Bug 1419884 : remove legacy mozilla addons

The second one is not very likely to be addressed soon, but maybe we can re-assess after the first one is closed.

Comment 1

[Julian Descottes [:jdescottes]]

Andrew, we are looking at removing the code that supports debugging legacy extensions in DevTools. After Bug 1449052 is resolved, will there still be some internal legacy extensions maintained or will it be safe for us to start removing things?

Comment 2

[Andrew Swan [:aswan]]

The ones listed in that bug are all the in-tree ones.  I think you're clear to start removing things right now, The unresolved bugs are either close to being resolved or things that people are not likely to use the debugger for.  If somebody really needs to debug a legacy extension they can use an older build.  I can't come up with a scenario where somebody really needs to debug a legacy extension running in a very recent Firefox build.

One thing to consider though, I believe Thunderbird plans to continue supporting legacy extensions.  We won't support it in mozilla-central but we'll provide hooks for them so they can move the necessary code to comm-central and keep supporting them without having to maintain patches against toolkit/  I don't know if devtools even runs in Thunderbird or if such a thing would be possible for the addon debugger, but its something to think about.

Comment 3

[Brian Grinstead [:bgrins]]

(In reply to Andrew Swan [:aswan] from comment #2)
> One thing to consider though, I believe Thunderbird plans to continue
> supporting legacy extensions.  We won't support it in mozilla-central but
> we'll provide hooks for them so they can move the necessary code to
> comm-central and keep supporting them without having to maintain patches
> against toolkit/  I don't know if devtools even runs in Thunderbird or if
> such a thing would be possible for the addon debugger, but its something to
> think about.

Yeah, I believe TB runs devtools. I don't know if it surfaces legacy addon debugging, but cc'ing some folks to give them a heads up about this incoming change.

Comment 4

[Julian Descottes [:jdescottes]]

Yulia, Alex, is this a cleanup that would make sense for devtools fission? It would also help for remote debugging so if it's not a priority for fission I can prioritize it in the remote debugging backlog.

Comment 5

[Alexandre Poirot [:ochameau]]

This isn't strictly necessary for fission. We can always refactor this code to align with new fission architecture.

I raised this topic as getting rid of these actors would be easier than maintaining them during fission's transition.
Now, it is not clear how we could help TB keep being able to debug legacy addons *and* remove these actors.
We could possibly move them to comm-central but they would have to do the fission refactoring, which won't be obvious...

Comment 6

[Julian Descottes [:jdescottes]]

(In reply to Alexandre Poirot [:ochameau] PTO back on 29th from comment #5)
> This isn't strictly necessary for fission. We can always refactor this code
> to align with new fission architecture.
> 

Yes this is more about technical debt than strictly remote debugging or fission, but as it would make sense in one of the two backlogs, that would help not forget about this.

> I raised this topic as getting rid of these actors would be easier than
> maintaining them during fission's transition.
> Now, it is not clear how we could help TB keep being able to debug legacy
> addons *and* remove these actors.
> We could possibly move them to comm-central but they would have to do the
> fission refactoring, which won't be obvious...

I don't think we're saying we want to keep it working. If no one is testing it and supporting it in m-c, if we need to remove all our mochitests testing the feature because the necessary code to run such addons is removed, things will just break of their own. I think the idea is to proactively remove the support now.

Comment 7

[Yulia Startsev [:yulia]]

As Alex mentioned, it isn't necessary for fission but would make the work easier. I guess we should wait for feedback from TB before moving ahead.

Comment 8

[Julian Descottes [:jdescottes]]

Jorg, we are trying to find out the current status of legacy extensions in Thunderbird. Are they still used and developed? If yes are DevTools used to debug them? What would be the impact if we removed DevTools support for legacy extensions?

Removing support for legacy extensions would simplify the maintenance on our side. Also when m-c drops the code needed to run legacy extensions, we will no longer be able to run related devtools tests, and at that point we will no longer catch regressions.

Comment 9

[Jorg K (CEST = GMT+2)]

The person to talk to is Geoff, so I'm forwarding the NI request.

Thunderbird's biggest legacy extension is Calendar/Lightning which uses XUL overlays. So we've ever written an overlay loader so that it and other legacy extensions can work. Do bootstrapped extensions also fall into the legacy category? We still support them and will do so until they are dropped during the 65 cycle (oh, that has just started).

Comment 10

[Geoff Lankow (:darktrojan)]

(In reply to Julian Descottes [:jdescottes][:julian] from comment #8)
> Jorg, we are trying to find out the current status of legacy extensions in
> Thunderbird. Are they still used and developed? If yes are DevTools used to
> debug them? What would be the impact if we removed DevTools support for
> legacy extensions?

The only thing I can think of that the devtools provides for extensions is the ability to run in the scope of a bootstrap.js file. (Yes, we want to keep that.) What else is there?

Comment 11

[Julian Descottes [:jdescottes]]

(In reply to Geoff Lankow (:darktrojan) from comment #10)
> (In reply to Julian Descottes [:jdescottes][:julian] from comment #8)
> > Jorg, we are trying to find out the current status of legacy extensions in
> > Thunderbird. Are they still used and developed? If yes are DevTools used to
> > debug them? What would be the impact if we removed DevTools support for
> > legacy extensions?
> 
> The only thing I can think of that the devtools provides for extensions is
> the ability to run in the scope of a bootstrap.js file. (Yes, we want to
> keep that.) What else is there?

Can you give more details on the typical workflow? Are you using scratchpad? the Browser toolbox?

The two main entry points to debug addons with DevTools in Firefox are about:debugging or the connect page. But I doubt those are accessible in TB.
- aboutdebugging https://developer.mozilla.org/en-US/docs/Tools/about:debugging
- connect page https://developer.mozilla.org/en-US/docs/Tools/Remote_Debugging/Debugging_Firefox_Desktop#Connect

(In reply to Jorg K (GMT+2) from comment #9)
> The person to talk to is Geoff, so I'm forwarding the NI request.
> 
> Thunderbird's biggest legacy extension is Calendar/Lightning which uses XUL
> overlays. So we've ever written an overlay loader so that it and other
> legacy extensions can work. Do bootstrapped extensions also fall into the
> legacy category? We still support them and will do so until they are dropped
> during the 65 cycle (oh, that has just started).

Yes bootstrapped extensions are being removed (will end in 65 I guess?). But according to Andrew in comment 2, TB might keep suppporting them after that?

> I believe Thunderbird plans to continue supporting legacy extensions.  We won't support
> it in mozilla-central but we'll provide hooks for them so they can move the necessary 
> code to comm-central and keep supporting them without having to maintain patches against toolkit

If this is correct and TB plans to port the code necessary to run bootstrapped extensions to comm-central, then we should try to preserve the current workflow used with DevTools for extensions in TB.

Comment 12

[Geoff Lankow (:darktrojan)]

(In reply to Julian Descottes [:jdescottes][:julian] from comment #11)
> Can you give more details on the typical workflow? Are you using scratchpad?
> the Browser toolbox?
> 
> The two main entry points to debug addons with DevTools in Firefox are
> about:debugging or the connect page. But I doubt those are accessible in TB.
> - aboutdebugging
> https://developer.mozilla.org/en-US/docs/Tools/about:debugging
> - connect page
> https://developer.mozilla.org/en-US/docs/Tools/Remote_Debugging/
> Debugging_Firefox_Desktop#Connect

Yes, we use (well I do, anyway) about:debugging and the scratchpad and browser toolbox, but we don't have the connect page. We do hope to keep bootstrapped extensions, although I don't yet know what that means for us.

Comment 13

[Julian Descottes [:jdescottes]]

After discussing with Yulia and Alex, the plan will be to migrate the AddonTargetActor to be compatible with DevTools fission before legacy extension support is completely removed. After that, it should be possible to migrate the code to Thunderbird so that TB can keep debugging extensions. 

We will add the AddonTargetActor migration bug as a blocker for this one once it's created.

Comment 14

[Andrew Swan [:aswan]]

Julian, is this targeted for 65?  This is one of the last remaining blockers for removing install.rdf support.
If this isn't going to make 65, I'd like to at least disable the affected tests unless you have another idea?

Comment 15

[Julian Descottes [:jdescottes]]

Most of the refactors we mentioned are landing in Bug 1506546, so we should be good to start migrating our tests using install.rdf to webextensions (or remove them when no longer relevant). I will try to do it as soon as possible, but in the meantime, if you need to disable them, feel free to do so. If you can just ping me in the bug where it happens it would be great. Thanks!

Comment 16

[Andrew Swan [:aswan]]

(In reply to Julian Descottes [:jdescottes][:julian] from comment #15)
> but in the meantime, if you need to disable them, feel free to do
> so. If you can just ping me in the bug where it happens it would be great.
> Thanks!

Whoops forgot to mention this eariler, I disabled a handful of tests here:
https://hg.mozilla.org/mozilla-central/rev/efac27a2bec4

Comment 17

[Julian Descottes [:jdescottes]]

Attachment: Bug 1497264 - Remove support for legacy addon target;r=ochameau,yulia

Comment 18

[Julian Descottes [:jdescottes]]

Attachment: Bug 1497264 - Remove AddonTargetActor and AddonConsoleActor;r=ochameau,yulia

This should be rather safe. We can do more refactoring and renaming once LegacyExtension support is fully removed (ie Fx 65 hits release)

Comment 19

[Julian Descottes [:jdescottes]]

Attachment: Bug 1497264 - Remove references to AddonTargetActor setAddonOptions;r=ochameau,yulia

Depends on D15383

Comment 20

[Julian Descottes [:jdescottes]]

Attachment: Bug 1497264 - Split addon/webextension.js into webextension.js and webextension-proxy.js;r=ochameau,yulia

Depends on D15384

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a894d406f4ae
Remove AddonTargetActor and AddonConsoleActor;r=ochameau,yulia
https://hg.mozilla.org/integration/autoland/rev/033f84c32bce
Remove references to AddonTargetActor setAddonOptions;r=ochameau,yulia
https://hg.mozilla.org/integration/autoland/rev/e62e0dfddefe
Split addon/webextension.js into webextension.js and webextension-proxy.js;r=ochameau,yulia

Comment 22

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/a894d406f4ae
https://hg.mozilla.org/mozilla-central/rev/033f84c32bce
https://hg.mozilla.org/mozilla-central/rev/e62e0dfddefe