1147296 - [B2G] [Emulator] Support CDMA call operations

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Emulator, defect)

Description

[Ben Hsu [:HoPang]]

Currently, call operation console commands belong to |gsm| command group. To make future CDMA testcases more understandable, we should design new call operation console commands dedicated to |cdma| group. Otherwise, it's weird to see a |gsm answer| in a CDMA testcase. Besides, by designing new console commands, we can have better checks when executing those commands.

Comment 1

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

[Tracking Requested - why for this release]:

Comment 2

[Ben Hsu [:HoPang]]

After offline discussion, I think creating a new command group for both GSM and CDMA is a better approach, where the commands in that command group take action according to the current modem technology (GSM or CDMA). One thing is that, with different handler functions, we can well separate the code handling the request of GSM or CDMA. The other thing is that, we can have more shared code in testcases with both handler functions for both GSM and CDMA wrapped in the same command group.

Comment 3

[Ben Hsu [:HoPang]]

Attachment: Add two cdma specified testcases and update related components

Hi Bevis,

Though I am still cleaning up the patches for the emulator, do you mind reviewing this patch first? In this patch, there is a new check function for CDMA call attributes and two testcases for CDMA single call operations.

Comment 4

[Ben Hsu [:HoPang]]

Attachment: [hardware/ril] Pull request 66 (for Emulator-ICS)

Comment 5

[Ben Hsu [:HoPang]]

Attachment: [external/qemu] pull request 157 (for Emulator-ICS)

Comment 6

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8650826 [details] [diff] [review]
Add two cdma specified testcases and update related components

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

To reduce redundant codes,
I think we could 
1. rename test_incoming_basic_operations.js and test_outgoing_basic_operations.js to
   test_cdma_gsm_incoming_basic_operations.js & test_cdma_gsm_outgoing_basic_operations.js.
2. have a meta CheckAll in head.js to dispatch the checking to checkAll & cdmaCheckAll accordingly.
3. have each .js runing cdma operations and gsm operations once.

BTW, please also address the following nits: in head.js.

Thanks!

::: dom/telephony/test/marionette/head.js
@@ +1209,5 @@
>  
>    /**
> +   * The examination Function for CDMA Testcases
> +   *
> +   * @param aTelephonyCallRecs

nit: s/aTelephonyCallRecs/aExpectedCalls

@@ +1211,5 @@
> +   * The examination Function for CDMA Testcases
> +   *
> +   * @param aTelephonyCallRecs
> +   *        An array consisting of |callRec|s should be in telephony
> +   * @param aConferenceCallRecs

nit: s/ConferenceCallRecs/aExpectedCallsInConference

@@ +1220,5 @@
> +    if (typeof aTelephonyCallRecs === Array) {
> +      is(Telephony.calls.length, aTelephonyCallRecs.length,
> +         "Check the length of telephony calls.");
> +
> +      aTelephonyCallRecs.map(aRec => {

nit:
1. s/aRec/aExpectedCall
2. we are not mapping something. forEach() makes more sense here.

Comment 7

[Ben Hsu [:HoPang]]

Hi Bevis,

Thanks for your review.

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #6)
> 2. have a meta CheckAll in head.js to dispatch the checking to checkAll &
> cdmaCheckAll accordingly.

Can I apply the new CheckAll function only to the new shared testcases in this bug? I think we can apply the new CheckAll function to all the other marionette testcases in another bug, where we can do more refactoring things there.

Comment 8

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Ben Hsu [:bhsu] from comment #7)
> Hi Bevis,
> 
> Thanks for your review.
> 
> (In reply to Bevis Tseng[:bevistseng][:btseng] from comment #6)
> > 2. have a meta CheckAll in head.js to dispatch the checking to checkAll &
> > cdmaCheckAll accordingly.
> 
> Can I apply the new CheckAll function only to the new shared testcases in
> this bug? I think we can apply the new CheckAll function to all the other
> marionette testcases in another bug, where we can do more refactoring things
> there.

Sure, that's what I expect as well. :)

Comment 9

[Ben Hsu [:HoPang]]

Comment on attachment 8651594 [details] [review]
[external/qemu] pull request 157 (for Emulator-ICS)

Sorry Edgar, since I am working on another differnece CDMA and GSM found when discussing with Bevis, I cancelled this review. Hoping it won't bother you too much ...

Comment 10

[Ben Hsu [:HoPang]]

Comment on attachment 8651594 [details] [review]
[external/qemu] pull request 157 (for Emulator-ICS)

Hi Edgar,

Though the testcases are still being revised, the emulator parts have been tested with previous stable testcases. Do you mind review these patch first?

Comment 11

[Edgar Chen [:edgar] (OOO untils 09/01, slow response)]

Comment on attachment 8650831 [details] [review]
[hardware/ril] Pull request 66 (for Emulator-ICS)

Looks good to me, thank you.

Comment 12

[Ben Hsu [:HoPang]]

Attachment: Part 1: Create |Modem| helper object (head.js)

Comment 13

[Ben Hsu [:HoPang]]

Attachment: Part 2: Move to telephony command group to support CDMA operations (head.js).

Comment 14

[Ben Hsu [:HoPang]]

Attachment: Part 3: Introduce a new check function for both GSM and CDMA (head.js)

Comment 15

[Ben Hsu [:HoPang]]

Attachment: Part 3: Introduce a new check function for both GSM and CDMA (head.js)

Comment 16

[Ben Hsu [:HoPang]]

Attachment: Part 1: Create |Modem| helper object (head.js)

Comment 17

[Ben Hsu [:HoPang]]

Attachment: Part 1: Create |Modem| helper object (head.js)

Comment 18

[Ben Hsu [:HoPang]]

Attachment: Part 2: Move to telephony command group to support CDMA operations (head.js)

Comment 19

[Ben Hsu [:HoPang]]

Attachment: Part 3: Introduce a new check function for both GSM and CDMA (head.js)

Comment 20

[Ben Hsu [:HoPang]]

Attachment: Part 4: Expose new functions (head.js)

Comment 21

[Ben Hsu [:HoPang]]

Attachment: Part 5: Replace two testcases (Testcase)

Comment 22

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8655926 [details] [diff] [review]
Part 1: Create |Modem| helper object (head.js)

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

Please see my comment below.

Thanks!

::: dom/telephony/test/marionette/head.js
@@ +154,5 @@
> +
> +  /**
> +   * @return Promise:
> +   */
> +  function changeModemTech(aClientID, aTech, aPreferredMask) {

nit: you are a modem already. I think we can rename to ChangeRadioTech instead.

@@ +167,5 @@
> +        target.addEventListener("voicechange", function onEvent(aEvent) {
> +          log("MobileConnection[" + aClientID + "] " +
> +              "received event 'voicechange'");
> +
> +          if (isTechMatched(aClientID)) {

nit: no need for aClientID.

@@ +185,5 @@
> +      .then(result => is(result[0],
> +                         aTech + " " + aPreferredMask,
> +                         "Check modem 'tech/preferred mask'"));
> +
> +    return Promise.all([promise1, promise2]);

How about:
function switchToExpectedTech() {
  return Promise.resolve()
  .then(() => emulator.runCmd("modem tech " + aTech + " " + aPreferredMask))
  .then(() => emulator.runCmd("modem tech"))
  .then(result => is(result[0],
                     aTech + " " + aPreferredMask,
                     "Check modem 'tech/preferred mask'"));
}

return isTechMatched() ? Promise.resolve() : 
                         Promise.all([waitForExpectedTech(), 
                                      switchToExpectedTech()]);

@@ +192,5 @@
> +  return [{
> +    changeTech: changeModemTech.bind(null, 0),
> +    isGSM:      isGSM.bind(null, 0),
> +    isCDMA:     isCDMA.bind(null, 0)
> +  }];

How about define a Modem Class with attribute |clientId| and all the helper functions which requires aClientID as 1st argument moved into this new class.

Then when returning the Modem[].
You can write it as followed:
  return [new Modem(1), new Modem(2)];

How do you think?

Comment 23

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8655928 [details] [diff] [review]
Part 2: Move to telephony command group to support CDMA operations (head.js)

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

r=me after the question is answered.

::: dom/telephony/test/marionette/head.js
@@ +598,5 @@
> +        // A CDMA call goes to connected state directly when the operator find
> +        // its callee, which makes the "connected" state in CDMA calls behaves
> +        // like the "alerting" state in GSM calls.
> +        let state = Modems[serviceId].isGSM() ? "alerting" : "connected";
> +        return waitForNamedStateEvent(outCall, state);

Don't we need this change in dialEmergency() as well?

Comment 24

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8655930 [details] [diff] [review]
Part 3: Introduce a new check function for both GSM and CDMA (head.js)

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

Please see my comments below.

Thanks!

::: dom/telephony/test/marionette/head.js
@@ +582,5 @@
> +   *     disconnectedReason, -- the disconnected reason of a disconnected call.
> +   *     number,             -- the call number.
> +   *     direction,          -- "in" for inbound, and "out" for outbound.
> +   *     conference,         -- shows whether it belongs to the conference.
> +   *   }

I didn't see any checking about this attributes in the implementation of checkActive() except |reference|.
Did I miss something?

@@ +600,5 @@
> +
> +    ok(calls.length < 2, "Too many actives call in telephony.calls");
> +    let activeCall = calls.length ? calls[0].reference : null;
> +
> +    // Get the actice conferece

nit: "active conference"

@@ +601,5 @@
> +    ok(calls.length < 2, "Too many actives call in telephony.calls");
> +    let activeCall = calls.length ? calls[0].reference : null;
> +
> +    // Get the actice conferece
> +    calls = aExpectedCallsInConference || [];

let callsInConf = aExpectedCallsInConference || []; for better reading.

@@ +602,5 @@
> +    let activeCall = calls.length ? calls[0].reference : null;
> +
> +    // Get the actice conferece
> +    calls = aExpectedCallsInConference || [];
> +    let acticeConference =

nit: s/acticeConference/activeConference

@@ +603,5 @@
> +
> +    // Get the actice conferece
> +    calls = aExpectedCallsInConference || [];
> +    let acticeConference =
> +        (calls.length && calls[0].state === "connected") ? conference : null;

(callsInConf.length && callsInConf[0].state == "connected") ? conference: null;

@@ +608,5 @@
> +
> +    // Check telephony.active
> +    ok(!(activeCall && acticeConference),
> +       "An active call cannot coexist with an active conference call.");
> +    is(telephony.active, activeCall || acticeConference, "check Active");

This is confusing me because
1. For activeCall, we check with the reference provided from aExpectedCalls.
2. For activeConference, we check the referrence from the Telphony.conferenceGroup.

Is this intended?

@@ +637,5 @@
> +  function equals(aExpectedCalls, aClientID = 0) {
> +    // Classify calls
> +    let callsInTelephony  = [];
> +    let CallsInConference = [];
> +    let disconnectedCalls = [];

no need for disconnectedCalls.

@@ +654,5 @@
> +      }
> +
> +      callsInTelephony.push(aCall);
> +      ok(!aCall.secondId, "For a telephony call, the secondId must be null");
> +    })

nit: you need a semicolon here: 
});

@@ +683,5 @@
> +    // Check conference.calls
> +    // NOTE: This function doesn't support conference call now, so the length of
> +    // |CallsInConference| should be 0, and the conference state shoul be "".
> +    is(conference.state, "", "Conference call is not supported yet.");
> +    ok(!CallsInConference.length, "Conference call is not supported yet.");

nit: is(CallsInConference.length, 0, "Conference call is not supported yet.");

@@ +695,5 @@
> +        case "connected": state = "active";   break;
> +        case "held":      state = "held";     break;
> +        case "incoming":  state = "incoming"; break;
> +        default:          state = null;
> +      }

Let's replace the switch case as followed:
let state = {
  "alerting": "ringing",
  "connected": "active",
  "held": "held",
  "incoming": "incoming"
}[aCall.state] || null;

@@ +697,5 @@
> +        case "incoming":  state = "incoming"; break;
> +        default:          state = null;
> +      }
> +
> +      state = aCall.emulatorState || state;

Why shall we have this additional check to convert the state again?

Comment 25

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8655931 [details] [diff] [review]
Part 4: Expose new functions (head.js)

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

LGTM, thanks!

Comment 26

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8655932 [details] [diff] [review]
Part 5: Replace two testcases (Testcase)

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

Please use 'git mv' to generate the patch again for review.
This could minimize the patch with the rename information.

Thanks!

Comment 27

[Ben Hsu [:HoPang]]

Comment on attachment 8655928 [details] [diff] [review]
Part 2: Move to telephony command group to support CDMA operations (head.js)

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

::: dom/telephony/test/marionette/head.js
@@ +598,5 @@
> +        // A CDMA call goes to connected state directly when the operator find
> +        // its callee, which makes the "connected" state in CDMA calls behaves
> +        // like the "alerting" state in GSM calls.
> +        let state = Modems[serviceId].isGSM() ? "alerting" : "connected";
> +        return waitForNamedStateEvent(outCall, state);

Yes, you're totally right.

Comment 28

[Ben Hsu [:HoPang]]

Attachment: (V2) Part 1: Create |Modem| helper object (head.js)

Comment 29

[Ben Hsu [:HoPang]]

Attachment: (V2) Part 2: Move to telephony command group to support CDMA operations (head.js)

Comment 30

[Ben Hsu [:HoPang]]

Attachment: (V2) Part 3: Introduce a new check function for both GSM and CDMA (head.js)

Comment 31

[Ben Hsu [:HoPang]]

Attachment: (V2) Part 5: Replace two testcases (Testcase)

Comment 32

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8658079 [details] [diff] [review]
(V2) Part 1: Create |Modem| helper object (head.js)

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

r=me after the comments are addressed.

Thanks!

::: dom/telephony/test/marionette/head.js
@@ +109,5 @@
> +  function Modem(aClientID) {
> +    this.clientID = aClientID;
> +  }
> +  Modem.prototype = {
> +    clietID: 0,

s/clietID/clientID

@@ +135,5 @@
> +        case "evdoa":
> +        case "evdob":
> +          return "evdo";
> +
> +        case "ehrpd":

I think we can have "ehrpd" map to "ehrpd" since it doesn't belong to lte and "ehrpd" is cdma only technology.

@@ +147,5 @@
> +
> +    isCDMA: function() {
> +      var mobileConn = navigator.mozMobileConnections[this.clientID];
> +      var tech = mobileConn && this.voiceTypeToTech(mobileConn.voice.type);
> +      return tech === "cdma" || tech === "evdo";

return tech === "cdma" || tech === "evdo" || tech === "ehrpd";

Comment 33

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8658080 [details] [diff] [review]
(V2) Part 2: Move to telephony command group to support CDMA operations (head.js)

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

Per discussion offline, there are other use cases of the gsm command for telephony in 
test_TelephonyUtils.js
test_consecutive_hold.js
test_outgoing_auto_hold.js
test_outgoing_busy.js

Please also have these be replaced with new "telephony" command defined in emulator.
Then, we have all telephony related commands move from "gsm" to "telephony".

Thanks!

Comment 34

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8658081 [details] [diff] [review]
(V2) Part 3: Introduce a new check function for both GSM and CDMA (head.js)

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

r=me after the comments are addressed.

::: dom/telephony/test/marionette/head.js
@@ +592,5 @@
> +    let activeCall = calls.length ? calls[0].reference : null;
> +
> +    // Get the actice conference
> +    let callsInConference = aExpectedCallsInConference || [];
> +    let acticeConference = callsInConference.length &&

s/acticeConference/activeConference

@@ +595,5 @@
> +    let callsInConference = aExpectedCallsInConference || [];
> +    let acticeConference = callsInConference.length &&
> +                           callsInConference[0].state === "connected"
> +                         ? navigator.mozTelephony.conferenceGroup
> +                         : null;

let ? and : aligned with callsInConference[0].state === "connected"

@@ +598,5 @@
> +                         ? navigator.mozTelephony.conferenceGroup
> +                         : null;
> +
> +    // Check telephony.active
> +    ok(!(activeCall && acticeConference),

s/acticeConference/activeConference

@@ +600,5 @@
> +
> +    // Check telephony.active
> +    ok(!(activeCall && acticeConference),
> +       "An active call cannot coexist with an active conference call.");
> +    is(telephony.active, activeCall || acticeConference, "check Active");

s/acticeConference/activeConference

@@ +621,5 @@
> +   *   }
> +   *
> +   * @param aExpectedCalls
> +   *        An array of call records.
> +   * @param aClientID

I didn't see the reason why we need |aClientID| in the implementation.
Please remove it if not needed.

@@ +625,5 @@
> +   * @param aClientID
> +   *        The ID of the modem to be looked up.
> +   * @return Promise
> +   */
> +  function equals(aExpectedCalls, aClientID = 0) {

ditto

Comment 35

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8658082 [details] [diff] [review]
(V2) Part 5: Replace two testcases (Testcase)

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

Looks good to me. :)

r=me after the question is answered?

Thanks!

::: dom/telephony/test/marionette/test_gsm_cdma_outgoing_basic_operations.js
@@ +9,5 @@
> + ******************************************************************************/
> +
> +const OutgoingNumber = "5555551111";
> +
> +function createExptectedCall(aCall, aState, aDisconnectedReason = null) {

Maybe we can move |createExptectedCall| to head.js if this will be used in other test cases.
How do you think?

Comment 36

[Ben Hsu [:HoPang]]

Comment on attachment 8651594 [details] [review]
[external/qemu] pull request 157 (for Emulator-ICS)

Sorry, Edgar

Since I want to address Bevis's comment in comment 33, I have to cancel this review request again.

> Per discussion offline, there are other use cases of the gsm command for
> telephony in 
> test_TelephonyUtils.js
> test_consecutive_hold.js
> test_outgoing_auto_hold.js
> test_outgoing_busy.js

Comment 37

[Ben Hsu [:HoPang]]

Attachment: (V3) Part 1: Create |Modem| helper object (head.js)

Comment 38

[Ben Hsu [:HoPang]]

Attachment: (V3) Part 2: Move to telephony command group and support CDMA operations (head.js)

Comment 39

[Ben Hsu [:HoPang]]

Attachment: (V3) Part 3: Introduce a new check function for both GSM and CDMA (head.js)

Comment 40

[Ben Hsu [:HoPang]]

Attachment: (V2) Part 4: Expose new functions (head.js)

Comment 41

[Ben Hsu [:HoPang]]

Attachment: (V3) Part 5: Replace two testcases (Testcase)

Comment 42

[Ben Hsu [:HoPang]]

Comment on attachment 8659809 [details] [diff] [review]
(V3) Part 1: Create |Modem| helper object (head.js)

Hi Bevis,

In this patch, in addition to your suggestion, I also make |aMask| of |changeTech()| become nullable. I think doing this will make this function more easy to use.

>+    changeTech: function(aTech, aMask) {
>+      let target = navigator.mozMobileConnections[this.clientID];
>+
>+      let mask = aMask || {
>+        gsm:   "gsm",
>+        wcdma: "gsm/wcdma",
>+        cdma:  "cdma",
>+        evdo:  "evdo0",
>+        ehrpd: "ehrpd",
>+        lte:   "lte"
>+      }[aTech];

Comment 43

[Ben Hsu [:HoPang]]

Comment on attachment 8659813 [details] [diff] [review]
(V3) Part 2: Move to telephony command group and support CDMA operations (head.js)

Since a CDMA call never go to |held| state, there is also no state event dispatched when the CDMA call becomes |held|.

>   /**
>    * Hold a call.
>    *
>-   * @param call
>+   * @param aCall
>    *        A TelephonyCall object.
>+   * @param aWaitForEvent
>+   *        Decide whether to wait for the state event.
>    * @return Promise<TelephonyCall>
>    */
>-  function hold(call) {
>+  function hold(aCall, aWaitForEvent = true) {
>     log("Putting the call on hold.");
> 
>     let promises = [];
> 
>-    promises.push(waitForNamedStateEvent(call, "held"));
>-    promises.push(call.hold());
>+    if (aWaitForEvent) {
>+      promises.push(waitForNamedStateEvent(aCall, "held"));
>+    }
>+    promises.push(aCall.hold());
> 
>-    return Promise.all(promises).then(() => call);
>+    return Promise.all(promises).then(() => aCall);
>   }

Comment 44

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8659809 [details] [diff] [review]
(V3) Part 1: Create |Modem| helper object (head.js)

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

LGTM, thanks!

::: dom/telephony/test/marionette/head.js
@@ +171,5 @@
> +        cdma:  "cdma",
> +        evdo:  "evdo0",
> +        ehrpd: "ehrpd",
> +        lte:   "lte"
> +      }[aTech];

nice!

Comment 45

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8659813 [details] [diff] [review]
(V3) Part 2: Move to telephony command group and support CDMA operations (head.js)

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

LGTM, thanks!

Comment 46

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8659814 [details] [diff] [review]
(V3) Part 3: Introduce a new check function for both GSM and CDMA (head.js)

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

LGTM, thanks!

Comment 47

[Edgar Chen [:edgar] (OOO untils 09/01, slow response)]

Comment on attachment 8651594 [details] [review]
[external/qemu] pull request 157 (for Emulator-ICS)

Thanks for the effort and sorry for taking so long to response.
I have left some comments on github, let's discuss them on next Monday. ;)

Comment 48

[Ben Hsu [:HoPang]]

Comment on attachment 8650831 [details] [review]
[hardware/ril] Pull request 66 (for Emulator-ICS)

Sorry Edgar, 

Since I've extended this AT command for CDMA 3-way call(Bug 1200480) and I don't want to handle one more pull request in that bug, do you mind reviewing this pull request now?

Comment 49

[Ben Hsu [:HoPang]]

Comment on attachment 8651594 [details] [review]
[external/qemu] pull request 157 (for Emulator-ICS)

Hi Edgar, thanks for your review, and I did some modification in this revision. First, I squashed these commits to reduce the number of patches and for a better readibility. Second, I've addressed your advice. However, I kept some of them in the origin form, if you have any thought, please feel free to tell me.

Comment 50

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8659814 [details] [diff] [review]
(V3) Part 3: Introduce a new check function for both GSM and CDMA (head.js)

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

::: dom/telephony/test/marionette/head.js
@@ +632,5 @@
> +
> +    ok(calls.length < 2, "Too many actives call in telephony.calls");
> +    let activeCall = calls.length ? calls[0].reference : null;
> +
> +    // Get the actice conference

s/actice/active/

sorry to jump in :P

Comment 51

[Ben Hsu [:HoPang]]

Attachment: Part 5: Replace two testcases (Testcase). r=btseng

Comment 52

[Ben Hsu [:HoPang]]

Attachment: (V4) Part 5: Replace two testcases (Testcase). r=btseng

Comment 53

[Edgar Chen [:edgar] (OOO untils 09/01, slow response)]

Comment on attachment 8659809 [details] [diff] [review]
(V3) Part 1: Create |Modem| helper object (head.js)

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

::: dom/telephony/test/marionette/head.js
@@ +203,5 @@
> +           : Promise.all([waitForExpectedTech(), changeToExpectedTech()]);
> +    }
> +  };
> +
> +  return [new Modem(0)];

I tried to apply these changes and run telephony tests on emulator-kk and I got a failure in test_dsds_normal_call.js which will try to dial outgoing call from second slot.

Comment 54

[Edgar Chen [:edgar] (OOO untils 09/01, slow response)]

Comment on attachment 8651594 [details] [review]
[external/qemu] pull request 157 (for Emulator-ICS)

r=me with the comments on github addressed.

Comment 55

[Ben Hsu [:HoPang]]

Attachment: (V4) Part 1: Create |Modem| helper object (head.js)

Fixing the bug Edgar discovered in comment 53.

Comment 56

[Ben Hsu [:HoPang]]

Attachment: (V4) Part 3: Introduce a new check function for both GSM and CDMA (head.js)

Addressing comment 50 :P

Comment 57

[Ben Hsu [:HoPang]]

https://bugzilla.mozilla.org/show_bug.cgi?id=1147296

Comment 58

[Ben Hsu [:HoPang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7d3b6a6473c7&exclusion_profile=false

Comment 59

[Ben Hsu [:HoPang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7d3b6a6473c7&exclusion_profile=false

Comment 60

[Edgar Chen [:edgar] (OOO untils 09/01, slow response)]

b2g-ril_v7:
https://github.com/mozilla-b2g/platform_hardware_ril/commit/4ace9aaee0e048dfda11bb787646c59982a3dc80

b2g-ics:
https://github.com/mozilla-b2g/platform_external_qemu/commit/065f6361461030d32c6dc08d716b013bfadab1d9

Comment 61

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/1731f93991cc
https://hg.mozilla.org/integration/b2g-inbound/rev/b6c3d38e34b9
https://hg.mozilla.org/integration/b2g-inbound/rev/23035c45e742
https://hg.mozilla.org/integration/b2g-inbound/rev/e8e26bc26817
https://hg.mozilla.org/integration/b2g-inbound/rev/11da6ec60db4

Comment 62

[Edgar Chen [:edgar] (OOO untils 09/01, slow response)]

b2g-kitkat:
https://github.com/mozilla-b2g/platform_hardware_ril/commit/575799df119771fcfbe9c99c8eee43f8100cd9a2
https://github.com/mozilla-b2g/platform_external_qemu/commit/6bbed5a99dd0f347bcee7fdde0463467fe3002f3

Comment 63

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1731f93991cc
https://hg.mozilla.org/mozilla-central/rev/b6c3d38e34b9
https://hg.mozilla.org/mozilla-central/rev/23035c45e742
https://hg.mozilla.org/mozilla-central/rev/e8e26bc26817
https://hg.mozilla.org/mozilla-central/rev/11da6ec60db4