896773 - setDisplayedApp case 2 does not even try to call the callback

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://github.com/mozilla-b2g/gaia/blob/4ecc18edfe9a41a057329751ddeb869af2969969/apps/system/js/window_manager.js#L827

'callback' is nowhere to be found.

Comment 1

[Justin Lebar (not reading bugmail)]

In particular this is bad because it means that we leak the <iframe mozbrowser>.  Leaking leaking an <iframe mozbrowser> is particularly bad because it leaks a bunch of IPC machinery.

Comment 2

[(inactive after 6/18) Alive Kuo [:alive]]

Which section do you mean?

 // Just run the callback right away if it is not homescreen
        if (callback) {
          callback();
        }

this one?

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

This one: https://github.com/mozilla-b2g/gaia/blob/d6db2aa8b39384433969cc807f9fbf5500aec21b/apps/system/js/window_manager.js#L828

Comment 4

[(inactive after 6/18) Alive Kuo [:alive]]

I know what you mean now. I think we could simply add callback() in the |windowOpened|.

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

Attachment: Do invoke callback anyway.

Though this is an edge case but the callback indeed isn't invoked in case 2.
(Only used for removeFrame)

Comment 6

[Justin Lebar (not reading bugmail)]

Blocks a blocker.

Comment 7

[(inactive after 6/18) Alive Kuo [:alive]]

https://github.com/mozilla-b2g/gaia/commit/bcc157b46e364856cdbc1eaa0ce159f2e10df1d6
master

Comment 8

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted bcc157b46e364856cdbc1eaa0ce159f2e10df1d6 to:
v1-train: 5bffab09198da3abfaf2c937f1f8b34fbe2b6046

Comment 9

[John Ford [:jhford] CET/CEST Berlin Time]

v1.1.0hd: 5bffab09198da3abfaf2c937f1f8b34fbe2b6046