1021775 - Bulk app upload never replies

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect)

Description

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

In bug 924574, we added support for sending packaged apps via bulk data.  However, the upload actor never replies to say it's done.

We should add a reply here.  On a related note, consider whether the client should log an actual error in case a new request is sent while the same actor is still active[1].

[1]: http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/client/dbg-client.jsm#792

Comment 1

[Paul Rouget [:paul]]

How comes everything works well even if we don't get notified when the app has been installed?

Comment 2

[Jukka Jylänki]

It's only the bulk upload operation that does not get a reply. After that App Manager still needs to send the 'install' command to the webapps actor. I think currently App Manager doesn't wait for a reply to the bulk upload command, and therefore it just assumes that the bulk upload always succeeds and it's ok to proceed to asking the device to install what was just uploaded.

Comment 3

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

Yeah, app-actor-front queues up the install command once the *client* is done transferring the app via bulk data (without waiting to hear the server's status), and it's not possible for this message to be processed by the server until bulk app upload completes because bulk data is "greedy" and takes over the entire transport until it completes.

So, the App Manager does not rely on a reply after upload, but I think it should be added to follow the same pattern that other messages use.

Comment 4

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

Attachment: Reply to bulk app install message

At the moment, bulk upload waits for a reply to the upload message (because the debugger client wants a reply to every message).  So, let's give it one for completeness.

Try: https://tbpl.mozilla.org/?tree=Try&rev=387ecbe30040

Comment 5

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

(In reply to J. Ryan Stinnett [:jryans] from comment #4)
> Created attachment 8441050 [details] [diff] [review]
> Reply to bulk app install message
> 
> At the moment, bulk upload waits for a reply to the upload message (because
> the debugger client wants a reply to every message).  So, let's give it one
> for completeness.

Sorry, to clarify... we don't actually pause to literally "wait", but I just mean that debugger client expects some reply from every request it sends.  Without this, it's left waiting forever.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e9ebee16aea5

Comment 8

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

Comment on attachment 8441050 [details] [diff] [review]
Reply to bulk app install message

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New feature added in bug 924574
User impact if declined: Slight Dev Tools protocol difference when installing apps.  If declined, building external tools to communicate with FxOS devices to install apps is slightly harder, as there are more protocol variations to handle.
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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b6fe1fea0b30