991582 - B2G RIL: Handle the result of RIL request in a consistent way

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Now in ril_worker, there are two different attributes to indicate the request result, |success| and |errorMsg|. Some parcel response handler returns both [1]. Some returns only |errorMsg|[2]. It will be good to have a consistent way for this.

And in fact, these two attributes have the same meaning.
1). if |success| is `true`, |errorMsg| will be `null`. 
2). if |success| is `false`, |errorMsg| will be a string indicate the error message.

It seems having |errorMsg| is enough, so we can consider to deprecate |success|.

[1] http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js?from=ril_worker.js&case=true#5589-5608
[2] http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js?from=ril_worker.js&case=true#5880-5886

Comment 1

[Ken Chang[:kenkai]]

Put this bug into backlog.

Comment 2

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: WIP, part1: Refactor ota status, v1

Comment 3

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: WIP, part 2-1: Handle options.errorMsg in processParcel(), v1

Comment 4

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: WIP, part 2-2: Handle the result of RIL request in a consistent way (Telephony), v1

Comment 5

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: WIP, part 2-3: Handle the result of RIL request in a consistent way (Icc), v1

Comment 6

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: WIP, part 2-4: Handle the result of RIL request in a consistent way (Cellbroadcast), v1

Comment 7

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: WIP, part 2-5: Handle the result of RIL request in a consistent way (MobileConnection), v1

Comment 8

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: Part 1: Handle options.errorMsg in processParcel(), v2

Comment 9

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: Part 2-1: Handle the result of RIL request in a consistent way (MobileConnection), v2

Comment 10

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: Part 2-2: Handle the result of RIL request in a consistent way (Cellbroadcast), v2

Comment 11

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: Part 2-3: Handle the result of RIL request in a consistent way (Telephony), v2

Comment 12

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: Part 2-4: Handle the result of RIL request in a consistent way (Data), v1

Comment 13

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: Part 3: Deprecate rilRequestError, v1

Comment 15

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: Part 1: Deprecate rilRequestError, v2

Comment 16

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: Part 2: Handle the result of RIL request in a consistent way, v3

Comment 17

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: Part 2: Handle the result of RIL request in a consistent way, v4

Comment 18

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Comment on attachment 8596980 [details] [diff] [review]
Part 1: Deprecate rilRequestError, v2

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

Hi Aknow, would you mind reviewing this? Thank you.

Comment 19

[Szu-Yu Chen [:aknow] (inactive)]

Comment on attachment 8596980 [details] [diff] [review]
Part 1: Deprecate rilRequestError, v2

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

::: dom/system/gonk/ril_worker.js
@@ +3965,5 @@
> +        return;
> +      }
> +
> +      // Fallback to default error handling if it meets max retry count.
> +      // Fall through.

Remove // Fall through
It's not switch case now.

Comment 20

[Szu-Yu Chen [:aknow] (inactive)]

Comment on attachment 8597012 [details] [diff] [review]
Part 2: Handle the result of RIL request in a consistent way, v4

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

Perfect! Thank you.

Comment 21

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: Part 1: Deprecate rilRequestError, v3, r=aknow

Address review comment #19. Thank you, Aknow.

Comment 22

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=46fb9bf78769&exclusion_profile=false

Comment 23

[Edgar Chen [:edgar] (PTO ~ 02/03)]

Attachment: Part 2: Handle the result of RIL request in a consistent way, v5, r=aknow

Rebase

Comment 24

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/a46d82663741
https://hg.mozilla.org/integration/b2g-inbound/rev/2707dcbd473b

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a46d82663741
https://hg.mozilla.org/mozilla-central/rev/2707dcbd473b