Closed Bug 991582 Opened 6 years ago Closed 5 years ago

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

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(tracking-b2g:backlog, firefox40 fixed)

RESOLVED FIXED
2.2 S11 (1may)
tracking-b2g backlog
Tracking Status
firefox40 --- fixed

People

(Reporter: edgar, Assigned: edgar)

References

Details

(Whiteboard: [grooming][p=3])

Attachments

(2 files, 15 obsolete files)

63.32 KB, patch
edgar
: review+
Details | Diff | Splinter Review
42.90 KB, patch
edgar
: review+
Details | Diff | Splinter Review
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
Put this bug into backlog.
blocking-b2g: --- → backlog
Whiteboard: [p=3]
Attachment #8423673 - Attachment description: WIP, part 2-3: Part 2-3: Handle the result of RIL request in a consistent way (Icc), v1 → WIP, part 2-3: Handle the result of RIL request in a consistent way (Icc), v1
Whiteboard: [p=3] → [grooming][p=3]
See Also: → 1054129
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Attachment #8423665 - Attachment is obsolete: true
Attachment #8423669 - Attachment is obsolete: true
Attachment #8423672 - Attachment is obsolete: true
Attachment #8423673 - Attachment is obsolete: true
Attachment #8423676 - Attachment is obsolete: true
Attachment #8423677 - Attachment is obsolete: true
Duplicate of this bug: 1054129
blocking-b2g: backlog → ---
Blocks: 831637
Attachment #8569045 - Attachment is obsolete: true
Attachment #8569065 - Attachment is obsolete: true
Attachment #8569049 - Attachment is obsolete: true
Attachment #8569050 - Attachment is obsolete: true
Attachment #8569052 - Attachment is obsolete: true
Attachment #8569064 - Attachment is obsolete: true
Attachment #8597011 - Attachment is obsolete: true
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.
Attachment #8596980 - Flags: review?(szchen)
Attachment #8597012 - Flags: review?(szchen)
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.
Attachment #8596980 - Flags: review?(szchen) → review+
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.
Attachment #8597012 - Flags: review?(szchen) → review+
Address review comment #19. Thank you, Aknow.
Attachment #8596980 - Attachment is obsolete: true
Attachment #8597084 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/a46d82663741
https://hg.mozilla.org/mozilla-central/rev/2707dcbd473b
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S11 (1may)
You need to log in before you can comment on or make changes to this bug.