1134166 - The Inspector is useless after killing the application

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect, P3)

Description

[Julien Wajsberg [:julienw]]

STR:

1. Open WebIDE
2. Plug in your Flame and open it from the top right menu
3. Open the Messages app from the top left menu (you need the "debug certified apps" pref)
4. Kill the Messages app from command line:

  adb shell kill `adb shell b2g-ps Messages | awk '!/APPLICATION/ { print $4 }'`

=> Notice that the "play" icon is now displayed in WebIDE, instead of the "reload" icon. That means the WebIDE is correctly notified that the application has stopped.
=> You can still use the Tools. This is OK.

5. Start the application again (either from WebIDE or from the device)

=> Notice the Tools are displayed but interacting with them does nothing on the device.

You need to close/reopen them.

Comment 1

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

I *think* this is another toolbox shutdown bug which is fixed by

Comment 2

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

... bug 1128027.

Comment 3

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

Okay, bug 1128027 changed some error messages here, but the issue itself still exists.

Julien, after step 4 (killing the app), you say "You can still use the Tools. This is OK.", but for me the toolbox seems pretty dead.  After all, we just killed the app, so how can the toolbox work anymore?

To me, the logical fix is for WebIDE's toolbox to close when the app is killed.  Does this seem right to you?

Comment 4

[Julien Wajsberg [:julienw]]

> "You can still use the Tools. This is OK."

Mmm I think I meant that this was not necessarily bad. For example, we might want to still see the console messages or the profiler information.

I don't have a strong opinion, but if we can keep the state as it is, and "just" reconnect the existing toolbox to any application we want to debug, while retaining all existing data, it would be the best behavior IMO.

If this is too difficult then do as you think :)

Comment 5

[Julien Wajsberg [:julienw]]

Actually, now that I use it once again, I exactly know how I'd like it to behave.

The fact is that I push the application using the Gaia build system, not using WebIDE (mainly because it's a shell command I can bind it in vim). So it kills the running application, and then I start it again, either from WebIDE or from the phone. And then I'd like WebIDE to automatically connect to the newly running application, but only when I start it (and I know WebIDE is aware of this because the top icons are changing).

Hope this makes sense.

Comment 6

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

(In reply to Julien Wajsberg [:julienw] (PTO April 6th) from comment #5)
> Actually, now that I use it once again, I exactly know how I'd like it to
> behave.
> 
> The fact is that I push the application using the Gaia build system, not
> using WebIDE (mainly because it's a shell command I can bind it in vim). So
> it kills the running application, and then I start it again, either from
> WebIDE or from the phone. And then I'd like WebIDE to automatically connect
> to the newly running application, but only when I start it (and I know
> WebIDE is aware of this because the top icons are changing).
> 
> Hope this makes sense.

Right, that makes sense!  If an app starts, and it's the currently selected app in WebIDE, we should auto-open the toolbox.

Comment 7

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

While working on this issue, I noticed troubles with reopen the toolbox for the same app.  The patches in bug 1145049 resolve this, hopefully they will land soon.

Comment 8

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

Attachment: 0001-Bug-1134166-Restart-toolbox-if-selected-app-re-opens.patch

Comment 9

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

Attachment: 0002-Bug-1134166-Close-toolbox-if-selected-app-dies.-r-oc.patch

Comment 10

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

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c5aa2aa5c4fc

Comment 11

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

For this to work as smoothly as possible, you should also have the patches from bug 1145049 applied.

Comment 12

[Alexandre Poirot [:ochameau]]

Comment on attachment 8588858 [details] [diff] [review]
0001-Bug-1134166-Restart-toolbox-if-selected-app-re-opens.patch

Review of attachment 8588858 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good,

Comment 13

[Alexandre Poirot [:ochameau]]

Comment on attachment 8588859 [details] [diff] [review]
0002-Bug-1134166-Close-toolbox-if-selected-app-dies.-r-oc.patch

Review of attachment 8588859 [details] [diff] [review]:
-----------------------------------------------------------------

thanks!

Comment 14

[Alexandre Poirot [:ochameau]]

Note that I'm seeing the following exception, not sure it is related to your patch or one of mine:

Full message: TypeError: AppManager.selectedProject is null
Full stack: UI.updateToolboxFullscreenState@chrome://webide/content/webide.js:1058:1
UI._closeToolboxUI@chrome://webide/content/webide.js:1082:5
UI.destroyToolbox/<@chrome://webide/content/webide.js:1012:21

STR is to open simulator, connect to clock, open toolbox and shutdown the simulator.

Comment 15

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

(In reply to Alexandre Poirot [:ochameau] from comment #14)
> Note that I'm seeing the following exception, not sure it is related to your
> patch or one of mine:
> 
> Full message: TypeError: AppManager.selectedProject is null
> Full stack:
> UI.updateToolboxFullscreenState@chrome://webide/content/webide.js:1058:1
> UI._closeToolboxUI@chrome://webide/content/webide.js:1082:5
> UI.destroyToolbox/<@chrome://webide/content/webide.js:1012:21
> 
> STR is to open simulator, connect to clock, open toolbox and shutdown the
> simulator.

It happens on fx-team, but I'll throw in an extra check here to fix it.

Comment 16

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

remote:   https://hg.mozilla.org/integration/fx-team/rev/c8066e57077e
remote:   https://hg.mozilla.org/integration/fx-team/rev/665b9fb029e1
remote:   https://hg.mozilla.org/integration/fx-team/rev/698da9956e20

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c8066e57077e
https://hg.mozilla.org/mozilla-central/rev/665b9fb029e1
https://hg.mozilla.org/mozilla-central/rev/698da9956e20