Remove AddonTargetActor for legacy extension

RESOLVED FIXED in Firefox 66

Status

enhancement
P3
normal
RESOLVED FIXED
7 months ago
4 months ago

People

(Reporter: jdescottes, Assigned: jdescottes)

Tracking

(Blocks 1 bug)

unspecified
Firefox 66
Dependency tree / graph

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

7 months ago
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.
(Assignee)

Comment 1

6 months ago
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?
Flags: needinfo?(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.
Flags: needinfo?(aswan)
(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.
(Assignee)

Updated

6 months ago
See Also: → 1451532
(Assignee)

Comment 4

6 months ago
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.
Flags: needinfo?(ystartsev)
Flags: needinfo?(poirot.alex)
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...
Flags: needinfo?(poirot.alex)
(Assignee)

Comment 6

6 months ago
(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.
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.
Flags: needinfo?(ystartsev)
(Assignee)

Comment 8

6 months ago
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.
Flags: needinfo?(jorgk)

Comment 9

6 months ago
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).
Flags: needinfo?(jorgk) → needinfo?(geoff)
(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?
Flags: needinfo?(geoff)
(Assignee)

Comment 11

6 months ago
(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.
Flags: needinfo?(geoff)
(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.
Flags: needinfo?(geoff)
(Assignee)

Comment 13

6 months ago
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.
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?
Flags: needinfo?(jdescottes)
(Assignee)

Comment 15

5 months ago
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!
Flags: needinfo?(jdescottes)
(Assignee)

Updated

5 months ago
Depends on: 1510620
(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
(Assignee)

Updated

4 months ago
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Attachment #9033326 - Attachment is obsolete: true
(Assignee)

Comment 18

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

Comment 21

4 months ago
Pushed by jdescottes@mozilla.com:
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

4 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
You need to log in before you can comment on or make changes to this bug.