Use |StreamSocket| for RIL IPC

RESOLVED FIXED in 2.2 S3 (9jan)

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: tzimmermann, Assigned: tzimmermann)

Tracking

unspecified
2.2 S3 (9jan)
All
Gonk (Firefox OS)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

We have reached the limits of what is possible with |UnixSocketConsumer|. The replacement is a combination of |ListenSocket| and |StreamSocket|. All IPC code should be converted to these classes. RIL IPC only requires functionality from |StreamSocket|.
The first two patches are just cleanups. The final patch changes |RilConsumer| to use |StreamSocket|.
Comment on attachment 8535534 [details] [diff] [review]
[01] Bug 1110669: Fix coding style of |RilConsumer| and it's helpers

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

Looks good to me.

::: ipc/ril/Ril.cpp
@@ +61,5 @@
>  
> +    if (sRilConsumers.Length() <= mClientId ||
> +        !sRilConsumers[mClientId] ||
> +        sRilConsumers[mClientId]->GetConnectionStatus() != SOCKET_CONNECTED) {
> +        // Probably shuting down.

nit: indention

@@ +62,5 @@
> +    if (sRilConsumers.Length() <= mClientId ||
> +        !sRilConsumers[mClientId] ||
> +        sRilConsumers[mClientId]->GetConnectionStatus() != SOCKET_CONNECTED) {
> +        // Probably shuting down.
> +        delete mRawData;

ditto.

@@ +67,1 @@
>          return NS_OK;

ditto.
Attachment #8535534 - Flags: review?(htsai) → review+
Attachment #8535535 - Flags: review?(htsai) → review+
Comment on attachment 8535537 [details] [diff] [review]
[03] Bug 1110669: Inherit |RilConsumer| from |StreamSocket|

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

Thomas, thanks for the patch. I think qDot should do the formal review for this part though it looks good to me.

hi Kyle, could you help take a look at this? Thank you.
Attachment #8535537 - Flags: review?(kyle)
Attachment #8535537 - Flags: review?(htsai)
Attachment #8535537 - Flags: feedback+
Comment on attachment 8535537 [details] [diff] [review]
[03] Bug 1110669: Inherit |RilConsumer| from |StreamSocket|

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

lgtm
Attachment #8535537 - Flags: review?(kyle) → review+
Changes since v1:

  - rebased onto latest m-c
  - fixed style issues
Attachment #8535534 - Attachment is obsolete: true
Attachment #8545165 - Flags: review+
Changes since v1:

  - rebased onto [01](v2)
Attachment #8535535 - Attachment is obsolete: true
Attachment #8545167 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/9a9147872e5b
https://hg.mozilla.org/mozilla-central/rev/14e25d5825f7
https://hg.mozilla.org/mozilla-central/rev/9bf521b43c84
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S3 (9jan)
See Also: → 1123651
You need to log in before you can comment on or make changes to this bug.