Merge lp:~jonas-drange/ubuntu-system-settings/no-empty-contexts-1433278-rtm into lp:ubuntu-system-settings/rtm-14.09

[Jonas G. Drange]

[cellular/apn] no creation of custom contexts on editor exit

What changed:
I moved creation of a new, custom internet context to when the user presses "Activate".

Why did I add apn.js:
I want to move the tons of JavaScript code out of the QML and into a nice, documented collection of helper code.

How to test this:
1. $ /usr/share/ofono/scripts/remove-contexts # removes your contexts. Backup your gprs* files if you want to keep them.
2. Open up the APN editor and press "Custom Internet APN"
3. Immediately press Cancel or even "<"
4. Confirm no new contexts were created.

* /var/lib/ofono/<IMSI>/gprs

[lp:~jonas-drange/ubuntu-system-settings/no-empty-contexts-1433278-rtm]

1007. By Jonas G. Drange on 2015-03-30

merge prerequisite

Unmerged revisions

=== modified file 'plugins/cellular/CMakeLists.txt'
--- plugins/cellular/CMakeLists.txt	2015-02-05 19:11:36 +0000
+++ plugins/cellular/CMakeLists.txt	2015-03-27 15:30:10 +0000
@@ -4,6 +4,7 @@
 install(FILES settings-cellular.svg DESTINATION ${PLUGIN_MANIFEST_DIR}/icons)
 
 set(QML_SOURCES
+    apn.js
     carriers.js
     CustomApnEditor.qml
     PageChooseApn.qml

=== modified file 'plugins/cellular/CustomApnEditor.qml'
--- plugins/cellular/CustomApnEditor.qml	2015-02-16 13:45:07 +0000
+++ plugins/cellular/CustomApnEditor.qml	2015-03-27 15:30:10 +0000
@@ -23,6 +23,7 @@
 import SystemSettings 1.0
 import Ubuntu.Components 1.1
 import Ubuntu.Components.ListItems 1.0 as ListItem
+import "apn.js" as APN
 
 ItemPage {
     objectName: "customapnPage"
@@ -37,6 +38,9 @@
     /// work around LP(#1361919)
     property var activateCb;
 
+    // Sim qml object
+    property var sim
+
     QtObject {
         id: d
         property var typeText : type === "internet" ? i18n.tr("Internet") : i18n.tr("MMS")
@@ -83,6 +87,12 @@
             ctx = contexts["internet"]
         }
 
+        // We do not have any context yet, we will create one, but not now.
+        // We create it once the user presses 'Activate'.
+        if (!ctx) {
+            return;
+        }
+
         apnName.text = ctx.accessPointName;
         userName.text = ctx.username;
         pword.text = ctx.password;
@@ -277,6 +287,8 @@
 
                     onClicked: {
                         var ctx;
+                        var values;
+
                         if (d.isMms)
                             ctx = contexts["mms"];
                         else
@@ -300,23 +312,71 @@
                             pageStack.pop();
                             return;
                         }
-                        ctx.accessPointName = apnName.text;
-                        ctx.username = userName.text;
-                        ctx.password = pword.text;
+
+                        values = {
+                            'accessPointName': apnName.text,
+                            'username': userName.text,
+                            'password': pword.text
+                        };
+
                         if (d.isMms) {
-                            ctx.messageCenter = mmsc.text;
+                            values['messageCenter'] = mmsc.text;
                             var proxyValue = "";
                             if (proxy.text !== "") {
                                 proxyValue = proxy.text;
                                 if (port.text !== "")
                                     proxyValue = proxyValue + ":" + port.text;
                             }
-                            ctx.messageProxy = proxyValue
-                        }
-                        /// @todo map protocol values
-
-                        activateCb(ctx.type, ctx.contextPath);
-                        pageStack.pop();
+                            values['messageProxy'] = proxyValue;
+                        }
+
+                        // If we are editing an existing context, update
+                        // its values, activate and exit.
+                        if (ctx) {
+                            APN.updateContext(ctx, values);
+                            activateCb(ctx.type, ctx.contextPath);
+                            pageStack.pop();
+                        } else {
+                            // If we do not have a context, create one and defer
+                            // editing it to when it has been created—and we
+                            // will also wait until Ofono has given it a name.
+                            // This is all very async, so we will use connect()
+                            // and dynamic QML.
+
+                            function addedCustomContext (path) {
+                                // This is a handler for when we add a context
+                                // in the custom apn editor.
+                                // We create a temporary QML object for the
+                                // context like this, until we get the APN
+                                // editor refactored.
+                                // TODO(jgdx): create some kind of libqofono
+                                // objects framework in apn.js
+                                var newCtx = Qt.createQmlObject(
+                                    'import MeeGo.QOfono 0.2;'+
+                                    'OfonoContextConnection {'+
+                                    'contextPath: "'+path+'" }',
+                                    root, "apn.js");
+
+                                // Ofono sets a default name (Internet or MMS).
+                                // We are going to change it to our default name.
+                                newCtx.nameChanged.connect(function (name) {
+                                    if (name === "Internet") {
+                                        newCtx.name = APN.CUSTOM_INTERNET_CONTEXT_NAME();
+                                        APN.updateContext(newCtx, values);
+                                    } else if (name === "MMS") {
+                                        newCtx.name = APN.CUSTOM_MMS_CONTEXT_NAME();
+                                        APN.updateContext(newCtx, values);
+                                    } else {
+                                        newCtx.destroy(100);
+                                        activateCb(newCtx.type, newCtx.contextPath);
+                                        pageStack.pop();
+                                    }
+                                });
+                            }
+
+                            sim.connMan.addContext(root.type);
+                            sim.connMan.contextAdded.connect(addedCustomContext);
+                        }
                     }
                 }
             } // item for buttons

=== modified file 'plugins/cellular/PageChooseApn.qml'
--- plugins/cellular/PageChooseApn.qml	2015-02-16 13:45:07 +0000
+++ plugins/cellular/PageChooseApn.qml	2015-03-27 15:30:10 +0000
@@ -52,31 +52,6 @@
         property bool __suppressActivation : true;
         property bool __haveCustomContexts : false;
 
-        function isEmptyCustom (type, ctx)
-        {
-            /* OK, this sucks _hard_,
-             * QOfono does not return the added context, so instead we have to "figure it out"
-             * by looking for "contextAdded" for totally empty context with default Name values.
-             * LP(#1361864)
-             */
-
-            var targetName = "";
-            var targetAccessPointName = "";
-            if (type === "internet") {
-                targetName = "Internet";
-                targetAccessPointName = "";
-            } else if (type == "mms") {
-                targetName = "MMS";
-                targetAccessPointName = "";
-            }
-
-            if (ctx.type === type &&
-                ctx.name === targetName &&
-                ctx.accessPointName === targetAccessPointName)
-                return true;
-            return false;
-        }
-
         function updateContexts(contexts)
         {
             var tmp = contexts || sim.connMan.contexts.slice(0);
@@ -113,7 +88,6 @@
                 if (mContexts[currentValue] !== undefined) {
                     throw "updateContexts: added is broken";
                 }
-
                 var ctx = connCtx.createObject(parent,
                                                {
                                                    "contextPath": currentValue
@@ -138,15 +112,26 @@
 
         // expects updateContexts() to have ran before executing.
         function checkAndCreateCustomContexts() {
+
+            // When a context is added, we assume it is a custom one.
+            function addedCustomContext (path) {
+
+                // We do not have a QML object representing this context,
+                // so we ask updateContexts to create one.
+                if (!d.mContexts[path]) {
+                    d.updateContexts();
+                }
+                d.mContexts[path].name = mCustomContextNameInternet;
+                sim.connMan.contextAdded.disconnect(addedCustomContext);
+            }
+
             var customInternet = Object.keys(mContexts).filter(function (i) {
                 var ctx = mContexts[i];
-                return ctx.name === mCustomContextNameInternet ||
-                       isEmptyCustom("internet", ctx);
+                return ctx.name === mCustomContextNameInternet;
             });
             var customMms = Object.keys(mContexts).filter(function (i) {
                 var ctx = mContexts[i];
-                return ctx.name === mCustomContextNameMms ||
-                       isEmptyCustom("mms", ctx);
+                return ctx.name === mCustomContextNameMms;
             });
 
             // make sure there is only one context per type
@@ -165,8 +150,8 @@
                 });
             }
 
-            if (customInternet.length === 0) {
-                sim.connMan.addContext("internet");
+            if (customInternet.length !== 0) {
+                d.__haveCustomContexts = true;
             }
 
             // @bug don't create the custom MMS context
@@ -251,7 +236,8 @@
                                type: type,
                                contexts: {"internet": mCustomContextInternet,
                                           "mms":      mCustomContextMms},
-                               activateCb: activateHelper
+                               activateCb: activateHelper,
+                               sim: sim
                            });
         }
 
@@ -304,9 +290,12 @@
 
             onActiveChanged: if (type === "internet") internetApnSelector.updateSelectedIndex()
             onNameChanged: {
-                if (name === "Internet") {
-                    this.name = d.mCustomContextNameInternet;
-                } else if (name === "MMS") {
+
+                if (name === d.mCustomContextNameInternet) {
+                    d.__haveCustomContexts = true;
+                }
+
+                if (name === "MMS") {
                     this.name = d.mCustomContextNameMms;
                     this.accessPointName = d.pendingCustomMmsData["accessPointName"];
                     this.username = d.pendingCustomMmsData["username"];
@@ -433,14 +422,7 @@
                 objectName: "customApnEdit"
                 text: i18n.tr("Custom Internet APN…")
                 width: parent.width - units.gu(4)
-                onClicked: {
-                    if (!d.__haveCustomContexts) {
-                        d.checkAndCreateCustomContexts();
-                        d.__haveCustomContexts = true;
-                    }
-
-                    d.openApnEditor("internet")
-                }
+                onClicked: d.openApnEditor("internet")
             }
 
             ListItem.ItemSelector {

=== added file 'plugins/cellular/apn.js'
--- plugins/cellular/apn.js	1970-01-01 00:00:00 +0000
+++ plugins/cellular/apn.js	2015-03-27 15:30:10 +0000
@@ -0,0 +1,61 @@
+var _CUSTOM_INTERNET_CONTEXT_NAME = '___ubuntu_custom_apn_internet';
+var _CUSTOM_MMS_CONTEXT_NAME = '___ubuntu_custom_apn_mms';
+
+/*
+Updates a context with new values.
+
+@param {QOfonoConnectionContext} context
+@param {Object} values dict with new values
+*/
+function updateContext (context, values) {
+    var messageProxy;
+    console.warn('updateContext', values.accessPointName, values.messageCenter,
+        values.messageProxy, values.port, values.useraccessPointName,
+        values.password, values.type);
+    if (typeof values.accessPointName !== 'undefined') {
+        context.accessPointName = values.accessPointName;
+    }
+    if (typeof values.messageCenter !== 'undefined') {
+        context.messageCenter = values.messageCenter;
+    }
+    if (typeof values.messageProxy !== 'undefined') {
+        messageProxy = values.messageProxy;
+        if (messageProxy !== '') {
+             messageProxy = messageProxy + ':' + values.port;
+        }
+        context.messageProxy = messageProxy;
+    }
+    if (typeof values.username !== 'undefined') {
+        context.username = values.username;
+    }
+    if (typeof values.password !== 'undefined') {
+        context.password = values.password;
+    }
+    if (typeof values.type !== 'undefined') {
+        context.type = values.type;
+    }
+
+    if (context.type === 'internet') {
+        context.name = CUSTOM_INTERNET_CONTEXT_NAME();
+    } else if (context.type === 'mms') {
+        context.name = CUSTOM_MMS_CONTEXT_NAME();
+    }
+}
+
+/*
+Exposes the custom internet context name to the world.
+
+@return {String} custom internet context name
+*/
+function CUSTOM_INTERNET_CONTEXT_NAME () {
+    return _CUSTOM_INTERNET_CONTEXT_NAME;
+}
+
+/*
+Exposes the custom mms context name to the world.
+
+@return {String} custom mms context name
+*/
+function CUSTOM_MMS_CONTEXT_NAME () {
+    return _CUSTOM_MMS_CONTEXT_NAME;
+}