unity-webapps-qml

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

  1. handle-delayed-content-handler-creation
  2. Merge into trunk
Proposed by Alexandre Abreu
Status: Merged
Approved by: Alberto Mardegan
Approved revision: 158
Merged at revision: 158
Proposed branch: lp:~abreu-alexandre/unity-webapps-qml/handle-delayed-content-handler-creation
Merge into: lp:unity-webapps-qml
Diff against target: 150 lines (+98/-7)
1 file modified
src/Ubuntu/UnityWebApps/bindings/content-hub/backend/content-hub.js (+98/-7)
To merge this branch: bzr merge lp:~abreu-alexandre/unity-webapps-qml/handle-delayed-content-handler-creation
Reviewer Review Type Date Requested Status
Alberto Mardegan (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+293165@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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 updated
158. By Alexandre Abreu

tweak

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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)
Revision history for this message
Alberto Mardegan (mardy) wrote :

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
Revision history for this message
Alexandre Abreu (abreu-alexandre) wrote :

> 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).

Revision history for this message
Alberto Mardegan (mardy) wrote :

> 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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
4444
45 var _contenthub = ContentHubBridge.ContentHub;45 var _contenthub = ContentHubBridge.ContentHub;
4646
47 function ContentHubEventHandler() {
48 this._contentHub = _contenthub;
49
50 this._shareRequestedCallbacks = []
51 this._importRequestedCallbacks = []
52 this._exportRequestedCallbacks = []
53
54 this._currentShareTransfer = null
55 this._currentImportTranfer = null
56 this._currentExportTransfer = null
57
58 this.setup()
59 }
60 ContentHubEventHandler.prototype = {
61 setup: function() {
62 this._contentHub.exportRequested.connect(
63 this._exportRequested.bind(this))
64 this._contentHub.importRequested.connect(
65 this._importRequested.bind(this))
66 this._contentHub.shareRequested.connect(
67 this._shareRequested.bind(this))
68 },
69 _exportRequested: function(transfer) {
70 if (this._exportRequestedCallbacks.length === 0) {
71 if (this._currentExportTransfer) {
72 this._currentExportTransfer.state = ContentTransfer.Aborted
73 }
74 this._currentExportTransfer = transfer
75 } else {
76 this._runHandlers(this._exportRequestedCallbacks, transfer)
77 }
78 },
79 _importRequested: function(transfer) {
80 if (this._importRequestedCallbacks.length === 0) {
81 if (this._currentImportTransfer) {
82 this._currentImportTransfer.state = ContentTransfer.Aborted
83 }
84 this._currentImportTransfer = transfer
85 } else {
86 this._runHandlers(this._importRequestedCallbacks, transfer)
87 }
88 },
89 _shareRequested: function(transfer) {
90 if (this._shareRequestedCallbacks.length === 0) {
91 if (this._currentShareTransfer) {
92 this._currentShareTransfer.state = ContentTransfer.Aborted
93 }
94 this._currentShareTransfer = transfer
95 } else {
96 this._runHandlers(this._shareRequestedCallbacks, transfer)
97 }
98 },
99
100 _runHandlers: function(handlers, transfer) {
101 for (var i = 0;
102 i < handlers.length;
103 ++i) {
104 handlers[i](transfer)
105 }
106 },
107
108 onExportRequested: function(callback) {
109 this._exportRequestedCallbacks.push(callback)
110 if (this._currentExportTransfer
111 && this._currentExportTransfer.state !== ContentTransfer.Aborted
112 && this._currentExportTransfer.state !== ContentTransfer.Finalized) {
113 this._exportRequested(this._currentExportTransfer)
114 this._currentExportTransfer = null
115 }
116 },
117 onImportRequested: function(callback) {
118 this._importRequestedCallbacks.push(callback)
119 if (this._currentImportTransfer
120 && this._currentImportTransfer.state !== ContentTransfer.Aborted
121 && this._currentImportTransfer.state !== ContentTransfer.Finalized) {
122 this._importRequested(this._currentImportTransfer)
123 this._currentImportTransfer = null
124 }
125 },
126 onShareRequested: function(callback) {
127 this._shareRequestedCallbacks.push(callback)
128 if (this._currentShareTransfer
129 && this._currentShareTransfer.state !== ContentTransfer.Aborted
130 && this._currentShareTransfer.state !== ContentTransfer.Finalized) {
131 this._shareRequested(this._currentShareTransfer)
132 this._currentShareTransfer = null
133 }
134 },
135 };
136 var contentHubHandler = new ContentHubEventHandler();
137
47 // TODO find a better way138 // TODO find a better way
48 function _nameToContentType(name) {139 function _nameToContentType(name) {
49 var contentTypePerName = {140 var contentTypePerName = {
@@ -345,8 +436,8 @@
345 item.object.name = items[i].name;436 item.object.name = items[i].name;
346 item.object.url = items[i].url;437 item.object.url = items[i].url;
347 if (items[i].text) {438 if (items[i].text) {
348 item.object.text = items[i].text;439 item.object.text = items[i].text;
349 }440 }
350441
351 contentItems.push(item.object);442 contentItems.push(item.object);
352 }443 }
@@ -384,10 +475,10 @@
384 var items = [];475 var items = [];
385 for (var i = 0; i < self.items.length; ++i) {476 for (var i = 0; i < self.items.length; ++i) {
386 items.push({477 items.push({
387 name: self.items[i].name.toString(),478 name: self.items[i].name.toString(),
388 url: self.items[i].url.toString(),479 url: self.items[i].url.toString(),
389 text: self.items[i].text.toString()480 text: self.items[i].text.toString()
390 });481 });
391 }482 }
392 return items;483 return items;
393 }484 }
@@ -807,21 +898,21 @@
807 },898 },
808899
809 onExportRequested: function(callback) {900 onExportRequested: function(callback) {
810 _contenthub.exportRequested.connect(function(exportTransfer) {901 contentHubHandler.onExportRequested(function(exportTransfer) {
811 var wrapped = new ContentTransfer(exportTransfer);902 var wrapped = new ContentTransfer(exportTransfer);
812 callback(wrapped.serialize());903 callback(wrapped.serialize());
813 });904 });
814 },905 },
815906
816 onImportRequested: function(callback) {907 onImportRequested: function(callback) {
817 _contenthub.onImportRequested.connect(function(importTransfer) {908 contentHubHandler.onImportRequested(function(importTransfer) {
818 var wrapped = new ContentTransfer(importTransfer);909 var wrapped = new ContentTransfer(importTransfer);
819 callback(wrapped.serialize());910 callback(wrapped.serialize());
820 });911 });
821 },912 },
822913
823 onShareRequested: function(callback) {914 onShareRequested: function(callback) {
824 _contenthub.shareRequested.connect(function(shareTransfer) {915 contentHubHandler.onShareRequested(function(shareTransfer) {
825 var wrapped = new ContentTransfer(shareTransfer);916 var wrapped = new ContentTransfer(shareTransfer);
826 callback(wrapped.serialize());917 callback(wrapped.serialize());
827 });918 });

Subscribers

People subscribed via source and target branches

to all changes: