Merge lp:~abreu-alexandre/unity-webapps-qml/handle-delayed-content-handler-creation into lp:unity-webapps-qml

[Alexandre Abreu]

Handle delayed creation of content hub share/import/export handler that could miss transfer requests

Handle delayed creation of content hub share/import/export handler that could miss transfer requests

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:157
http://jenkins.qa.ubuntu.com/job/unity-webapps-qml-ci/182/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-webapps-qml-wily-amd64-ci/4
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-webapps-qml-wily-armhf-ci/4

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-webapps-qml-ci/182/rebuild

review: Approve (continuous-integration)

[lp:~abreu-alexandre/unity-webapps-qml/handle-delayed-content-handler-creation]

158. By Alexandre Abreu on 2016-04-27

tweak

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:158
http://jenkins.qa.ubuntu.com/job/unity-webapps-qml-ci/183/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-webapps-qml-wily-amd64-ci/5
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-webapps-qml-wily-armhf-ci/5

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-webapps-qml-ci/183/rebuild

review: Approve (continuous-integration)

[Alberto Mardegan (mardy)]

Can you clarify a bit how this works? If I look at this code, it seems that the _#action#Requested() functions are only called after pushing a callback into the array, so it seems that the first branch of the if() can never be executed:

       _exportRequested: function(transfer) {
           if (this._exportRequestedCallbacks.length === 0) { // <- never true
               if (this._currentExportTransfer) {
                   this._currentExportTransfer.state = ContentTransfer.Aborted
               }
               this._currentExportTransfer = transfer
           } else {
               this._runHandlers(this._exportRequestedCallbacks, transfer)
           }

review: Needs Information

[Alexandre Abreu (abreu-alexandre)]

> Can you clarify a bit how this works? If I look at this code, it seems that
> the _#action#Requested() functions are only called after pushing a callback
> into the array, so it seems that the first branch of the if() can never be
> executed:
>
> _exportRequested: function(transfer) {
> if (this._exportRequestedCallbacks.length === 0) { // <- never true
> if (this._currentExportTransfer) {
> this._currentExportTransfer.state = ContentTransfer.Aborted
> }
> this._currentExportTransfer = transfer
> } else {
> this._runHandlers(this._exportRequestedCallbacks, transfer)
> }

actually the rationale behind this is this one:

at the moment (and I am thinking about possibly revamping it), the content hub bits found in the JS bindings (used by html5 and webapps) are triggered by the JS. Meaning that as the application is being started, it is not until the webview has been triggered, and the content loaded that the JS code scripts are being injected to potentially add e.g. share handler for the content hub.

This delay creates a clear race and the content hub sharing transfer is currently being dropped since at the time that all the QML app tree is complete, there are no share requested signal handlers.

When the JS finally gets executed and binds some share handler callbacks the transfer requested has been popped.

So the idea is for those signals that can wake the app, always define handlers, and acknowledge the fact that only until we have one callback defined the app could have been woken up by a missed share/import/export request.

In this context, the expression this._exportRequestedCallbacks.length evaluates to 0 when this happens (when the app is being woken up by a share request and the JS hasnt run yet).

[Alberto Mardegan (mardy)]

> So the idea is for those signals that can wake the app, always define
> handlers, and acknowledge the fact that only until we have one callback
> defined the app could have been woken up by a missed share/import/export
> request.

Now I got it, thanks! I missed the connections made in the setup() method; now it's clear.

review: Approve

=== modified file 'src/Ubuntu/UnityWebApps/bindings/content-hub/backend/content-hub.js'
--- src/Ubuntu/UnityWebApps/bindings/content-hub/backend/content-hub.js	2015-05-06 16:38:34 +0000
+++ src/Ubuntu/UnityWebApps/bindings/content-hub/backend/content-hub.js	2016-04-27 20:15:04 +0000
@@ -44,6 +44,97 @@
 
     var _contenthub = ContentHubBridge.ContentHub;
 
+    function ContentHubEventHandler() {
+        this._contentHub = _contenthub;
+
+        this._shareRequestedCallbacks = []
+        this._importRequestedCallbacks = []
+        this._exportRequestedCallbacks = []
+
+        this._currentShareTransfer = null
+        this._currentImportTranfer = null
+        this._currentExportTransfer = null
+
+        this.setup()
+    }
+    ContentHubEventHandler.prototype = {
+        setup: function() {
+            this._contentHub.exportRequested.connect(
+                     this._exportRequested.bind(this))
+            this._contentHub.importRequested.connect(
+                     this._importRequested.bind(this))
+            this._contentHub.shareRequested.connect(
+                     this._shareRequested.bind(this))
+        },
+        _exportRequested: function(transfer) {
+            if (this._exportRequestedCallbacks.length === 0) {
+                if (this._currentExportTransfer) {
+                    this._currentExportTransfer.state = ContentTransfer.Aborted
+                }
+                this._currentExportTransfer = transfer
+            } else {
+                this._runHandlers(this._exportRequestedCallbacks, transfer)
+            }
+        },
+        _importRequested: function(transfer) {
+            if (this._importRequestedCallbacks.length === 0) {
+                if (this._currentImportTransfer) {
+                    this._currentImportTransfer.state = ContentTransfer.Aborted
+                }
+                this._currentImportTransfer = transfer
+            } else {
+                this._runHandlers(this._importRequestedCallbacks, transfer)
+            }
+        },
+        _shareRequested: function(transfer) {
+            if (this._shareRequestedCallbacks.length === 0) {
+                if (this._currentShareTransfer) {
+                    this._currentShareTransfer.state = ContentTransfer.Aborted
+                }
+                this._currentShareTransfer = transfer
+            } else {
+                this._runHandlers(this._shareRequestedCallbacks, transfer)
+            }
+        },
+
+        _runHandlers: function(handlers, transfer) {
+            for (var i = 0;
+                 i < handlers.length;
+                 ++i) {
+                handlers[i](transfer)
+            }
+        },
+
+        onExportRequested: function(callback) {
+            this._exportRequestedCallbacks.push(callback)
+            if (this._currentExportTransfer
+                    && this._currentExportTransfer.state !== ContentTransfer.Aborted
+                    && this._currentExportTransfer.state !== ContentTransfer.Finalized) {
+                this._exportRequested(this._currentExportTransfer)
+                this._currentExportTransfer = null
+            }
+        },
+        onImportRequested: function(callback) {
+            this._importRequestedCallbacks.push(callback)
+            if (this._currentImportTransfer
+                    && this._currentImportTransfer.state !== ContentTransfer.Aborted
+                    && this._currentImportTransfer.state !== ContentTransfer.Finalized) {
+                this._importRequested(this._currentImportTransfer)
+                this._currentImportTransfer = null
+            }
+        },
+        onShareRequested: function(callback) {
+            this._shareRequestedCallbacks.push(callback)
+            if (this._currentShareTransfer
+                    && this._currentShareTransfer.state !== ContentTransfer.Aborted
+                    && this._currentShareTransfer.state !== ContentTransfer.Finalized) {
+                this._shareRequested(this._currentShareTransfer)
+                this._currentShareTransfer = null
+            }
+        },
+    };
+    var contentHubHandler = new ContentHubEventHandler();
+
     // TODO find a better way
     function _nameToContentType(name) {
         var contentTypePerName = {
@@ -345,8 +436,8 @@
                 item.object.name = items[i].name;
                 item.object.url = items[i].url;
                 if (items[i].text) {
-		    item.object.text = items[i].text;
-		}
+                    item.object.text = items[i].text;
+                }
 
                 contentItems.push(item.object);
             }
@@ -384,10 +475,10 @@
                 var items = [];
                 for (var i = 0; i < self.items.length; ++i) {
                     items.push({
-			name: self.items[i].name.toString(),
+                        name: self.items[i].name.toString(),
                         url: self.items[i].url.toString(),
                         text: self.items[i].text.toString()
-		    });
+                    });
                 }
                 return items;
             }
@@ -807,21 +898,21 @@
         },
 
         onExportRequested: function(callback) {
-            _contenthub.exportRequested.connect(function(exportTransfer) {
+            contentHubHandler.onExportRequested(function(exportTransfer) {
                 var wrapped = new ContentTransfer(exportTransfer);
                 callback(wrapped.serialize());
             });
         },
 
         onImportRequested: function(callback) {
-            _contenthub.onImportRequested.connect(function(importTransfer) {
+            contentHubHandler.onImportRequested(function(importTransfer) {
                 var wrapped = new ContentTransfer(importTransfer);
                 callback(wrapped.serialize());
             });
         },
 
         onShareRequested: function(callback) {
-            _contenthub.shareRequested.connect(function(shareTransfer) {
+            contentHubHandler.onShareRequested(function(shareTransfer) {
                 var wrapped = new ContentTransfer(shareTransfer);
                 callback(wrapped.serialize());
             });