777251 - B2G MMS: Configure MMS proxy settings through SettingsService

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

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

See also bug 776294.

Comment 1

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

Bug 772747 is going to introduce new settings "ril.mms.apn", "ril.mms.xxx". We should use these new settings names.

Comment 2

[Gene Lian (I already quit Mozilla)]

I used to have some experiences on the settings. If no one is looking into this one, I'm glad to take it. Please let me know.

Comment 3

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

(In reply to Gene Lian [:gene] from comment #2)
> I used to have some experiences on the settings. If no one is looking into
> this one, I'm glad to take it. Please let me know.

Thank you a lot, but please wait until bug 772747 is settled down.

Comment 4

[Gene Lian (I already quit Mozilla)]

Is this a V1 feature or basecamp-blocker?

Comment 5

[Shian-Yow Wu [:swu]]

It's not a V1 feature, but since we already have part of MMS function working, and MMS is really an important feature, supporting this earlier will benefit on MMS development & testing.

Comment 6

[Gene Lian (I already quit Mozilla)]

Since we're going to support some initiative testings recently, I'll start working on this to hook up the MMS back-end functions to Gaia-end.

Comment 7

[José Antonio Olivera Ortega [:jaoo]]

Take a look at https://github.com/mozilla-b2g/gaia/pull/6483. That code is part of the operator variant logic.

Comment 8

[Gene Lian (I already quit Mozilla)]

Attachment: WIP Patch

Comment 9

[Gene Lian (I already quit Mozilla)]

Attachment: WIP Patch V2

Hi Vicamo,

Could you please return some feedback when you're available? Thanks! The following summarizes what I've done in this patch:

1. I used the following structure to cache the current MMS settings, which will be updated whenever the settings are changed or initialized when the MMS service starts up.

  MMSProxyInfoSettings: { "mmsc": null,
                          "mmsproxy": null,
                          "mmsport": null },

2. Another local variable |MMSNetworkConnected| is used to specify whether the MMS network is currently connected. Note that the |MMSProxyInfoSettings| is valid only when the |MMSNetworkConnected| is true.

3. An |MMSProxyInfoSettingsToRead| is used to specify whether the |MMSProxyInfoSettings| is completely read during initialization.

4. I understand there could be an edge case that |MMSProxyInfoSettings| is not completely updated during run-time but I prefer firing another bug to address that since it needs lots of changes in our current settings service mechanism.

Comment 10

[Gene Lian (I already quit Mozilla)]

Attachment: WIP Patch V2.1

Just some fine tunes. Please see the main summary in the comment #9.

Comment 11

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

Comment on attachment 686036 [details] [diff] [review]
WIP Patch V2.1

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

It's nice! Let's do it this way and hopefully we can integrate all the mms related setting entries into one someday.

Comment 12

[Gene Lian (I already quit Mozilla)]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #11)
> Comment on attachment 686036 [details] [diff] [review]
> WIP Patch V2.1
> 
> Review of attachment 686036 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> It's nice! Let's do it this way and hopefully we can integrate all the mms
> related setting entries into one someday.

Thanks for the review! I'll come back with a formal patch after more testings to make sure everything is feasible.

Btw, the Bug 779381 addresses the issue of multiple settings in a single set.

Comment 13

[Gene Lian (I already quit Mozilla)]

Attachment: Patch V3

This patch perfectly synchronizes both settings and preferences by taking the settings.js into consideration. Also, the preference fortunately has an observer mechanism to listen to the preference changes, so we can listen to the "nsPref:changed" observer topic to update the cached proxy info at run-time.

Could you please take a chance to review this patch? Thanks a lot!

Comment 14

[Gene Lian (I already quit Mozilla)]

Attachment: Patch V3.1

Rename some variable/function names to make them more readable. Please see comment #13 for the main summary. Thanks!

Comment 15

[Gene Lian (I already quit Mozilla)]

Attachment: Patch V3.2

Sorry. Fix a spot.

Comment 16

[Gene Lian (I already quit Mozilla)]

Attachment: Patch V3.3

Damn... wrong patch. Sorry for the noise!

Comment 17

[Gene Lian (I already quit Mozilla)]

Attachment: Patch V3.4

Fix some nits based on the review in person.

Comment 18

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

Comment on attachment 687014 [details] [diff] [review]
Patch V3.4

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

Thank you :)

Comment 19

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/f5b542c05974

Comment 20

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f5b542c05974

Comment 21

[Gene Lian (I already quit Mozilla)]

This bug relates to MMS features and needs to be tagged as leo+ so that we can uplift it into the b2g-18 branch.

Comment 22

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: [b2g18]-006-bug-777251-fix

Comment 23

[Gene Lian (I already quit Mozilla)]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/29e37785c631

Comment 24

[Ryan VanderMeulen [:RyanVM]]

The entire set of clian's pushes was backed out for multiple reasons.
https://tbpl.mozilla.org/?tree=Mozilla-B2g18&rev=a0b06192f882

1.) The tree rules are clear that you are not to land on top of bustage. At the time you pushed, both B2G Mn and B2G xpcshell had bustage from prior commits that hadn't been backed out yet.
2.) The tree rules are also clear that you are to watch your pushes for any bustage and handle them accordingly. mozilla-inbound is the ONLY tree where this rule does not apply.
3.) Even after the earlier bustage was backed out, something in one of your many pushes was causing further B2G Mn failures as shown in the log below.
https://tbpl.mozilla.org/php/getParsedLog.php?id=20424173&tree=Mozilla-B2g18
4.) This isn't cause for backout by itself, but it is also strongly preferred to not push each commit individually as our build and testing resources are limited and doing so stretches them even thinner. Please limit your number of pushes as much as possible unless you have good reason for keeping them separate.

Comment 25

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

https://hg.mozilla.org/releases/mozilla-b2g18/rev/99680a32f297