962541 - E/GeckoConsole( 4482): [JavaScript Error: "redeclaration of const DevToolsUtils" {file: "resource://gre/modules/devtools/server/child.js" line: 1}]

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect)

Description

[Paul Rouget [:paul]]

I think this explains why sometimes I can't connect to B2G. I need to restart B2G to fix this issue. Not clear STR yet.

Comment 1

[Alexandre Poirot [:ochameau]]

Most likely because of this loadFrameScript, evaluating child.js multiple times in the app frame:
http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/webapps.js#796

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: loadFrameScript, execute scripts only once

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Allows debugging the same app on subsequent connections

Comment 4

[Alexandre Poirot [:ochameau]]

Comment on attachment 8364250 [details] [diff] [review]
loadFrameScript, execute scripts only once

Currently, each time you call loadFrameScript, the related script is executed a new time. We discussed with Vivien and tend to agree on the fact it should be executed only once.
Do you think we could enforce such behavior in loadFrameScript? Or it is up to the caller to ensure calling it once?

If you think that's ok, I'll write some test and push it to try,
otherwise I'll address that in the call site.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

It is up to the caller. Using data: url is very useful for testing and I certainly want those
to be executed every time I pass such to loadFrameScript.

Comment 6

[Alexandre Poirot [:ochameau]]

Attachment: patch

Ok, so in this patch, we just let the frame script being evaluated more than once
and prevent any exception due to const or strict mode.

Comment 7

[Alexandre Poirot [:ochameau]]

The patch looks big, but that's just a indentation shift, I'm encapsulation the script in an anonymous function.

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Does this change mean we'll have an extra set of actors running each time you start debugging an app?  As in, if I start debugging app 1, close the toolbox, then start debugging app 1 again on the same connection, are there now 2 sets of actors loaded for this app?

I'm wondering because it doesn't seem like we clean up all the things being added here.

Comment 9

[Alexandre Poirot [:ochameau]]

Hum in that particular scenario, the appActorsMap in the webapps actor will prevent spawing another actor, but that's a still a relevant question for the current case. We will spawn another actor whereas the one spawned during the previous connection is still around. They will all be cleaned up once the app is killed.

There is multiple improvement to be made:
 - destroy all actors on disconnect? We never do such thing yet. We never try to clean up stuff on the child side. It is not really clear what we should be doing to clean stuff properly on child side. Would calling `disconnect()` on the ContentAppActor (which is a BrowserTabActor) be enough??
 - factor out connectToApp (bug 962577) and may be also the actor map. Move it to a global component that would keep the actor alive between connections.

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

The current behavior in all other cases is that when the connection is gone, all actors will be GCed. I think the child process actors should behave the same.

Comment 11

[Alexandre Poirot [:ochameau]]

We will improve actors cleanup in a followup as this breakage is quite painfull and current patch doesn't improve/regress actors cleanup.

Comment 12

[Alexandre Poirot [:ochameau]]

See bug 966991 for followup.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/9ea9e72a11b6

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9ea9e72a11b6