884131 - Messages to netd need sequence number on gonk-JB

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Michael Wu [:mwu]]

Messages to netd now require a sequence number at the beginning.

Comment 1

[Vincent Chang[:vchang][changyihsin]]

Vliu is interested in this bug. Assign the bug to him.

Comment 2

[Dave Hylands [:dhylands]]

For vold, the sequence number could be zero, in which case it isn't checked (and so it doesn't need to increment).

When you use a sequence number, you'll also get a sequence number in the replies as well.

Comment 3

[Vincent Chang[:vchang][changyihsin]]

Attachment: Patch v1.0 for mc

Netd also inherits the FrameworkListener class. So the sequence number could be zero like the vold case, too.

Comment 4

[Vincent Chang[:vchang][changyihsin]]

Attachment: Patch v1.0 for mc

Sorry, I posted the wrong file. Please ignore previous one.

Comment 5

[Dave Hylands [:dhylands]]

So I see you setting the sequence number to zero when you send the commands.

However, I would have expected that onNetMessage should also have some code to remove the sequence number from the response, otherwise the variable reason (from onNetdResponse) will have a leading "0 " when sequence numbers are used.

Comment 6

[Vincent Chang[:vchang][changyihsin]]

Thanks for your reminding. I'll post a new patch to address your comments soon. 
After upgrading the repo from jb 4.2 to JB 4.3, I can't startup b2g successfully. 
The screen stopped in Google .... I'll try to get a clean repo and try again.

Comment 7

[Vincent Chang[:vchang][changyihsin]]

Attachment: Patch v1.1 for mc

I have verified this patch in nexus 4 and unagi.

Comment 8

[Dave Hylands [:dhylands]]

Comment on attachment 784754 [details] [diff] [review]
Patch v1.1 for mc

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

::: dom/system/gonk/net_worker.js
@@ +324,5 @@
> +    }
> +    result += String.fromCharCode(octet);
> +  }
> +  return null;
> +}

nit: I think that its worth a comment to explain why you're writing your own splice routine and not using built in one.

Comment 9

[Vincent Chang[:vchang][changyihsin]]

Attachment: Patch v1.2 ready to land for mc

Add the comment to split() function.

Comment 10

[Vincent Chang[:vchang][changyihsin]]

push to inbound.
https://hg.mozilla.org/integration/mozilla-inbound/rev/c6c7b244f1b7

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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