Merge lp:~ahayzen/music-app/fix-canload-false-queueloaderworker into lp:music-app/remix

[Andrew Hayzen]

* Wait until all worker threads have stopped after calling canLoad=false

* Wait until all worker threads have stopped after calling canLoad=false

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:821
http://91.189.93.70:8080/job/music-app-ci/1269/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/898
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/898/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/121

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/music-app-ci/1269/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

I've managed to break the timing/race condition part of the fix.

Before a large queue is loaded I play an album containing "Song 2" by the Blur: http://i.imgur.com/9UvRrrw.png. However, when I switch to the queue I see that the queue contains the original large list: http://i.imgur.com/FFBJXR6.png. When I restart the app I get the correct queue (Song 2).

The above might have been a really tight race condition though. I haven't been able to reproduce it more than just once.

review: Needs Information

[lp:~ahayzen/music-app/fix-canload-false-queueloaderworker]

822. By Andrew Hayzen on 2015-02-01

* Wait until all worker threads have stopped after calling canLoad=false

823. By Andrew Hayzen on 2015-02-01

* Revert dropping of sync as we now wait for workers

824. By Andrew Hayzen on 2015-02-01

* Fix for indenting

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:824
http://91.189.93.70:8080/job/music-app-ci/1272/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/929
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/929/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/124

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/music-app-ci/1272/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

LGTM! Race condition doesn't appear any longer and this still allows the queue to function. External stimuli such as content-hub and the uri-handlers still function as intended.

review: Approve

=== modified file 'WorkerModelLoader.qml'
--- WorkerModelLoader.qml	2014-10-29 02:44:47 +0000
+++ WorkerModelLoader.qml	2015-02-01 03:35:19 +0000
@@ -29,6 +29,7 @@
      property var list
      property var model
      property bool preLoadComplete: false
+     property int processing: 0
      property int syncFactor: 5
 
      onCanLoadChanged: {
@@ -54,6 +55,8 @@
                  completed = true
              }
 
+             processing--;
+
              return;  // do not continue from a sync 'pong' only from a process/clear
          }
 
@@ -72,10 +75,13 @@
          if (i === 1) {  // sync after the first item to prevent empty states
              sync()
          }
+
+         processing--;  // minus at end to cause count to go 1->2->1 not 1->0->1
      }
 
      function clear() {
          if (list !== undefined) {
+             processing++
              sendMessage({'clear': true, 'model': model})
          }
      }
@@ -85,6 +91,7 @@
      {
          if (i < list.length) {
              console.log(JSON.stringify(list[i]));
+             processing++;
              sendMessage({'add': list[i], 'model': model});
              i++;
          } else {
@@ -100,6 +107,7 @@
 
      function sync()
      {
+         processing++;
          sendMessage({'sync': true, 'model': model});
      }
 }

=== modified file 'music-app.qml'
--- music-app.qml	2015-01-29 02:42:38 +0000
+++ music-app.qml	2015-02-01 03:35:19 +0000
@@ -202,6 +202,11 @@
             // Stop queue loading in the background
             queueLoaderWorker.canLoad = false
 
+            if (queueLoaderWorker.processing > 0) {
+                waitForWorker.workerStop(queueLoaderWorker, processFile, [uri, play])
+                return;
+            }
+
             uri = decodeURIComponent(uri);
 
             // Lookup track in songs model
@@ -541,6 +546,33 @@
         }
     }
 
+    // A timer to wait for a worker to stop
+    Timer {
+        id: waitForWorker
+        interval: 16
+        repeat: true
+
+        property var func
+        property var params  // don't use args/arguments as they are already defined/internal
+        property WorkerScript worker
+
+        onTriggered: {
+            if (worker.processing === 0) {
+                stop()
+                func.apply(this, params)
+            }
+        }
+
+        // Waits until the worker has stopped and then calls the func(*params)
+        function workerStop(worker, func, params)
+        {
+            waitForWorker.func = func
+            waitForWorker.params = params
+            waitForWorker.worker = worker
+            start()
+        }
+    }
+
     // Run on startup
     Component.onCompleted: {
         customdebug("Version "+appVersion) // print the curren version
@@ -654,6 +686,14 @@
     }
 
     function trackClicked(model, index, play, clear) {
+        // Stop queue loading in the background
+        queueLoaderWorker.canLoad = false
+
+        if (queueLoaderWorker.processing > 0) {
+            waitForWorker.workerStop(queueLoaderWorker, trackClicked, [model, index, play, clear])
+            return;
+        }
+
         // TODO: remove once playlists uses U1DB
         if (model.hasOwnProperty("linkLibraryListModel")) {
             model = model.linkLibraryListModel;