999300 - Remove RIL V5 legacy support

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Vicamo Yang [:vicamo][:vyang]]

RIL v5 has been deprecated for a long time, and its behaviours differ from v6 and following version a lot.  There is no supported devices running on gingerbread now and this leaves some lines in ril worker untested.  That goes worse when we're having some serious refactoring on ril worker.  I think it's about time to remove these legacy lines.

RILv4: 3b1530aef62476248825a2092c9aea800395c3b1, 2.3_r10 <= x <= 2.3.3_r1
RILv5: 4380897c6cdc99486a061b819943e3b290ebcf09, 2.3.3_r1 < x < c0114b3
RILv6: c0114b325877907fcdf7a5baa24e54a752e7e58b, 4.0.1_r1 <= x <= 4.0.4_r2.1,
RILv7: 2bc78d614e349574426d198c37e51ccb7455b5bb, 4.1_r1 <= x < 4.3_r0.9, (emulator-ics)
RILv8: 8a9e02161271505de274db0c3a88087056dd5dfc, 4.3_r0.9 <= x < 4.4_r0.9 (emulator-jb)
RILv9: a18b9d1e1a014290691d63a7f335085dadc83e46, 4.4_r0.9 <= x <= 4.4_r1.2, 
RILv10: 2458d8d1e56faae7b00511ceeab19730572c22d9, >= 4.4.1_r1 (emulator-kk)

Comment 1

[Vicamo Yang [:vicamo][:vyang]]

We had emulator-ics merge 4.2.2 because of CDMA emulation.

Comment 2

[a2882525224]

Attachment: Bug 999300 - Remvoed the Ril v5 legacy support

Comment 3

[a2882525224]

Hi Edgar,

I have removed as much v5 legacy related code as I can. Do you mind reviewing this patch?

Comment 4

[a2882525224]

Attachment: Bug 999300 - Part1:Remvoed the Ril v5 legacy support

Comment 5

[a2882525224]

Attachment: Bug 999300 - Part2:Update the related testcases

Comment 6

[a2882525224]

Attachment: Bug 999300 - Part1:Remvoed the Ril v5 legacy support(V2)

Comment 7

[a2882525224]

Attachment: Bug 999300 - Part1:Remvoed the Ril v5 legacy support(V3)

Comment 8

[Edgar Chen [:edgar]]

Comment on attachment 8608667 [details] [diff] [review]
Bug 999300 - Part1:Remvoed the Ril v5 legacy support(V3)

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

Thank for working on this, please see my comments below.

::: dom/system/gonk/ril_worker.js
@@ +5635,5 @@
>        newState = GECKO_RADIOSTATE_ENABLED;
> +      // We will retrieve radio tech by another request. We leave _isCdma
> +      // untouched, and it will be set once we get the radio technology.
> +      this._waitingRadioTech = true;
> +      this.getVoiceRadioTechnology();

We bail out early in line #5649 if radioState isn't changed. Please just leave this in original place which can prevent unnecessary radio technology querying.

Comment 9

[Edgar Chen [:edgar]]

Comment on attachment 8606833 [details] [diff] [review]
Bug 999300 - Part2:Update the related testcases

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

Looks good, but you miss removing the v5Legacy in test_ril_worker_icc_SimRecordHelper.js [1].

[1] https://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/tests/test_ril_worker_icc_SimRecordHelper.js#358

Comment 10

[a2882525224]

Hi Edgar,

Thanks for your suggestion but I can't fix it immediately because I am traveling abroad. I will deal with it when I come back.

Comment 11

[a2882525224]

Attachment: Bug 999300 - Part1:Remvoed the Ril v5 legacy support(V4)

Comment 12

[a2882525224]

Attachment: Bug 999300 - Part2:Update the related testcases(V2)

Comment 13

[a2882525224]

Hi Edgar,

I fixed up the patches. Do you mind reviewing them ?

Comment 14

[Edgar Chen [:edgar]]

Comment on attachment 8617259 [details] [diff] [review]
Bug 999300 - Part1:Remvoed the Ril v5 legacy support(V4)

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

Looks good, but please help to revise the commit message:
- Add space after `Part` and `:`, e.g. "Bug 999300 - Part 1: ....".
- Remove the patch version.

And please remember to add `r=edgar` or `r=echen` after review granted.
Thank you.

Comment 15

[Edgar Chen [:edgar]]

Comment on attachment 8617260 [details] [diff] [review]
Bug 999300 - Part2:Update the related testcases(V2)

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

Same as comment #14.
And remember to provide try result [1] before marking checkin-needed, if you need help please just let me know.
Thank you. 

[1] https://wiki.mozilla.org/Build:TryServer#How_to_push_to_try

Comment 16

[a2882525224]

Attachment: Bug 999300 - Part 1:Remvoed the Ril v5 legacy support(V5)

Revise the commit message

Comment 17

[a2882525224]

Attachment: Bug 999300 - Part 1:Remvoed the Ril v5 legacy support(V5). r=edgar

Revise the commit message

Comment 18

[a2882525224]

Attachment: Bug 999300 - Part 1:Remvoed the Ril v5 legacy support(V5). r=edgar

Revise the commit message

Comment 19

[a2882525224]

Attachment: Bug 999300 - Part 1: Remvoed the Ril v5 legacy support(V5). r=edgar

Updated the commit message

Comment 20

[a2882525224]

Attachment: Bug 999300 - Part 2: Update the related testcases(V3). r=edgar

Updated the commit message

Comment 21

[Ben Hsu [:HoPang]]

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

Comment 22

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/ec1e3811e1eb
https://hg.mozilla.org/integration/b2g-inbound/rev/103ddf47f489

Comment 23

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

https://hg.mozilla.org/mozilla-central/rev/ec1e3811e1eb
https://hg.mozilla.org/mozilla-central/rev/103ddf47f489