If you disconnect from the device while an app toolbox is open, the toolbox is left open but completely useless and broken, since the connection has been closed already. We should close any toolboxes we opened when the connection closes.

Reduce repetition by having the device controller do the work. Projects page will just tell it what to do.

Close the toolboxes on device disconnect. I started thinking about how to test this, but it's a bit tricky since you want to connect to an actual app. Assuming these patches are okay, I'll file a bug to look at testing again once we have a simulator.

Try push: https://tbpl.mozilla.org/?tree=Try&rev=ab40f1e6a41a

Comment on attachment 801924 [details] [diff] [review] Part 1: Refactor projects to forward to device controller Review of attachment 801924 [details] [diff] [review]: ----------------------------------------------------------------- I'm fine with this factorization, I like the fact that avoid calling listTabs multiple times. But also note that we should be able to make actor related code more united by switching to protocol.js (bug 912476). I don't think it is reasonable to switch to protocol.js just before the merge, but I started writing sort of "front", in bug 911785. The code you factored should at some point move to it.

Thanks for the review! I just realized that the refactor will conflict with your pending patch in bug 911785 to add install, since it needs the listTabs info too, and there is no "Install" in the devices panel, so it wouldn't make as much sense to continue the forwarding pattern. Perhaps it makes sense for me to move the remote communication to some new file entirely, that both projects.js and device.js would talk to? That file would centralize the work in one place, and has an additional benefit of separating this out of the controller for a specific UI panel, which is really the main goal of device.js (it tries to do too much right now).

I could also skip the refactoring entirely for now, since we are moving quickly, and do that later.

(In reply to J. Ryan Stinnett [:jryans] from comment #6) > I could also skip the refactoring entirely for now, since we are moving > quickly, and do that later. What about keeping the listTab but moving the rest to actor-front once it lands, would that work?

(In reply to Alexandre Poirot (:ochameau) from comment #7) > (In reply to J. Ryan Stinnett [:jryans] from comment #6) > > I could also skip the refactoring entirely for now, since we are moving > > quickly, and do that later. > > What about keeping the listTab but moving the rest to actor-front once it > lands, would that work? I am not entirely sure I follow. Do you mean that device.js and projects.js would both: * Call listTabs * Get this new actor-front * Call things like start / stop on the front If so, that could work too, but even then it may make sense to wrap listTabs and the usage of the front in a central place so that device.js and projects.js don't have to repeat those things. Anyway, I'll link the protocol.js refactor to this for more ideas when that happens, and re-focus this bug to just fixing the open toolboxes.

Carrying over Alex's r+ from attachment #801925 [details] [diff] [review]. Without the refactor from attachment #801924 [details] [diff] [review], there's two places to make the change now.