Closed Bug 1013745 Opened 6 years ago Closed 6 years ago

Refine telephony dialling flow and pending outgoing call mechanism

Categories

(Firefox OS Graveyard :: RIL, defect)

x86_64
Linux
defect
Not set

Tracking

(tracking-b2g:backlog)

RESOLVED FIXED
2.0 S3 (6june)
tracking-b2g backlog

People

(Reporter: aknow, Assigned: aknow)

References

Details

(Whiteboard: [p=2])

Attachments

(4 files, 6 obsolete files)

1.33 KB, patch
aknow
: review+
Details | Diff | Splinter Review
8.28 KB, patch
aknow
: review+
Details | Diff | Splinter Review
2.92 KB, patch
aknow
: review+
Details | Diff | Splinter Review
12.87 KB, patch
aknow
: review+
Details | Diff | Splinter Review
I think we could remove the pending outgoing call and the placeholder callIndex.

When dialling, we don't have to create a call in the very beginning. Let's wait the response of GET_CURRENT_CALL, and then create the call when the dialling is successful and the new call is reported from the call list.

So the overall asyn dialling flow will be:

Telephony.dial() => ... => REQUEST_DIAL
                        <= REQUEST_DIAL (dial success)
                        <= CALL_STATE_CHANGED
                        => GET_CURRENT_CALL
                        <= GET_CURRENT_CALL

       (process the call list and check whether the new dialling call exists)

   call or error <=
Attachment #8426020 - Flags: review?(htsai) → review+
Comment on attachment 8426021 [details] [diff] [review]
Part 2: Get callIndex when dialling success (dom)

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

r=me with comment addressed, thank you!

::: dom/telephony/Telephony.cpp
@@ -27,5 @@
>  #include "TelephonyCallGroup.h"
>  
>  using namespace mozilla::dom;
>  using mozilla::ErrorResult;
> -using mozilla::dom::telephony::kOutgoingPlaceholderCallIndex;

Please also remove related code in TelephonyCommon.h.
Attachment #8426021 - Flags: review?(htsai) → review+
Attachment #8426022 - Flags: review?(htsai) → review+
Comment on attachment 8426023 [details] [diff] [review]
Part 4: Refine dialling process and pending call mechanism

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

::: dom/system/gonk/ril_worker.js
@@ +537,5 @@
>      };
>      this.mergedCellBroadcastConfig = null;
>  
>      /**
> +     * Whether we have a successful pending MO call.

The comment isn't so right. It should be "a successful dialing request" as you describe else where in the patch.

@@ +4045,4 @@
>        } else {
> +        options.success = true;
> +        // Use the callIndex in the first entry.
> +        options.callIndex = newCalls[Object.keys(newCalls)[0]].callIndex;

I think we should at least make sure get a call whose state isn't incoming.

::: dom/telephony/gonk/TelephonyProvider.js
@@ -38,5 @@
>  const DIAL_ERROR_INVALID_STATE_ERROR = "InvalidStateError";
>  const DIAL_ERROR_OTHER_CONNECTION_IN_USE = "OtherConnectionInUse";
>  
> -// Should match the value we set in dom/telephony/TelephonyCommon.h
> -const OUTGOING_PLACEHOLDER_CALL_INDEX = 0xffffffff;

ya~ nice clean-up!
Attachment #8426023 - Flags: review?(htsai)
Whiteboard: [p=2]
Target Milestone: --- → 2.0 S3 (6june)
Attachment #8426023 - Attachment is obsolete: true
Attachment #8428527 - Flags: review?(htsai)
Comment on attachment 8428527 [details] [diff] [review]
Part 4#2: Refine dialling process and pending call mechanism

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

::: dom/system/gonk/ril_worker.js
@@ +4045,2 @@
>        } else {
> +        options.success = true;

What if there's only one call and the call state is INCOMING? The behaviour is different from my expectation. I think in this case |options.success = false| is expected. How do you think?
Attachment #8428527 - Flags: review?(htsai)
blocking-b2g: --- → backlog
Attachment #8428527 - Attachment is obsolete: true
Attachment #8428612 - Flags: review?(htsai)
Comment on attachment 8428612 [details] [diff] [review]
Part 4#3: Refine dialling process and pending call mechanism

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

Nice! Thank you.
Attachment #8428612 - Flags: review?(htsai) → review+
Pasted the wrong link in previous comment. This is the correct one.
https://tbpl.mozilla.org/?tree=Try&rev=2f5273b97265
Duplicate of this bug: 1106773
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.