Closed Bug 975064 Opened 6 years ago Closed 6 years ago

App Manager doesn't handle disconnection well

Categories

(DevTools :: General, defect)

defect
Not set

Tracking

(firefox29 unaffected, firefox30 verified)

VERIFIED FIXED
Firefox 30
Tracking Status
firefox29 --- unaffected
firefox30 --- verified

People

(Reporter: janx, Assigned: jryans)

References

Details

(Keywords: regression)

Attachments

(1 file)

Steps to reproduce:
- Connect App Manager to device
- Debug an app
- Unplug/replug the cable
- Connect to device again
- Try to debug the same app

Observed:
- No side tab is opened for the app, no toolbox is shown, the background is grey
- Debugging other apps works

Expected:
- Debugging the same app again after disconnect should work
The app actor front caches targets when you connect to an app.  It uses the target's close event to remove the target from its cache.

However, the target close event was not being triggered if the client is closed out from under the target, so now the target watches for this.

Try: https://tbpl.mozilla.org/?tree=Try&rev=3575fc05f978
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Attachment #8381921 - Flags: review?(past)
Comment on attachment 8381921 [details] [diff] [review]
Close target when client is closed

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

Good catch!

::: browser/devtools/framework/target.js
@@ +385,5 @@
>    /**
>     * Teardown listeners for remote debugging.
>     */
>    _teardownRemoteListeners: function TabTarget__teardownRemoteListeners() {
> +    this.client.removeListener("closed", this.destroy);

You could have used addOneTimeListener and avoided this if you wanted.
Attachment #8381921 - Flags: review?(past) → review+
Keywords: checkin-needed
Blocks: 918695
Keywords: regression
https://hg.mozilla.org/integration/fx-team/rev/89184046cf4d
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/89184046cf4d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 30
Comment on attachment 8381921 [details] [diff] [review]
Close target when client is closed

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 918695
User impact if declined: App Manager broken when attempting to re-debug an app after getting disconnected, requires restarting Firefox to fix
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: None
Attachment #8381921 - Flags: approval-mozilla-beta?
Attachment #8381921 - Flags: approval-mozilla-aurora?
Hmm, never mind, seems only Firefox 30 is affected.  Regression must have been triggered by some other change then.
Attachment #8381921 - Flags: approval-mozilla-beta?
Attachment #8381921 - Flags: approval-mozilla-aurora?
Keywords: verifyme
Reproduced using Firefox OS 1.2 simulator.
Verified fixed on latest Aurora 20140423004003.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.