1161635 - Remove sdk/addon/window

Status: RESOLVED INCOMPLETE

(Add-on SDK Graveyard :: General, defect, P2)

Description

[Erik Vold [:erikvold] (please needinfo? me)]

This is unnecessary bloat for the majority of add-ons made with the sdk, and on mobile devices this can really harm performance, I wonder if we can remove it or make it opt-in?

A quick search of the addons on amo shows that there are very few add-ons usng the api.

https://mxr.mozilla.org/addons/search?string=sdk/addon/window

Comment 1

[Erik Vold [:erikvold] (please needinfo? me)]

This would require reverting bug 820953 and reverting some or all of bug 820953

Comment 2

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Erik Vold [:erikvold] (please needinfo? me) from comment #1)
> This would require reverting bug 820953 and reverting some or all of bug
> 820953

So it looks like https://github.com/mozilla/addon-sdk/pull/996 and https://github.com/mozilla/addon-sdk/pull/698 did not have separate bugs and both changes were assigned to bug 820953.

Comment 3

[Erik Vold [:erikvold] (please needinfo? me)]

Any objections/suggestions?

Comment 4

[Erik Vold [:erikvold] (please needinfo? me)]

Mark is my assumption that using background windows are expensive on mobile accurate?

Comment 5

[Dave Townsend [:mossop]]

To my understanding the issue is that some of our APIs use it (pageworker off the top of my head) and assume it is already available and loaded as soon as they try to use it so currently we have to create it and wait for it to load before allowing anything else to happen. If we can avoid that then I'm all for it.

Comment 6

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Dave Townsend [:mossop] from comment #5)
> To my understanding the issue is that some of our APIs use it (pageworker
> off the top of my head) and assume it is already available and loaded as
> soon as they try to use it so currently we have to create it and wait for it
> to load before allowing anything else to happen. If we can avoid that then
> I'm all for it.

sdk/page-worker and sdk/request modules existed long before the sdk/addon/window module implemented in bug 820953 so they can certain work without it again.

The sdk/page-worker module and sdk/addon/window both depend on sdk/frame/utils and not each other, so sdk/page-worker would be unaffected I think.

The sdk/request module depends on sdk/addon/window, which is a change that is easy to revert.

There is also now an sdk/messaging api added in bug 980410 which has no docs even though it is high level, and it is used by the sdk/dev/debuggee module and I'm not sure if we can remove this dependency or not atm, I'd have to look in to that some more.  Maybe Irakli knows?

Comment 7

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Erik Vold [:erikvold] (please needinfo? me) from comment #4)
> Mark is my assumption that using background windows are expensive on mobile
> accurate?

Mark, I'm mainly wondering if you'd consider this premature optimization or not.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Erik Vold [:erikvold] (please needinfo? me) from comment #7)
> (In reply to Erik Vold [:erikvold] (please needinfo? me) from comment #4)
> > Mark is my assumption that using background windows are expensive on mobile
> > accurate?
> 
> Mark, I'm mainly wondering if you'd consider this premature optimization or
> not.

Background windows end up using some memory, that's true. If the window is created in the startup flow, it can cause some performance issues, but I can't be definitive on that.

Since we decided not to rush ahead and remove the main hidden window, I think you could move more slowly with some optimizations until we have some measurements.

On a related note, the mobile team has started thinking about shipping more of our features as add-ons. We might find more actionable optimizations to consider as that work moves ahead. Add-ons and startup performance have always been at odds on mobile.

Comment 9

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1399562