Storage Framework

Merge lp:~michihenning/storage-framework/use-qiodevice into lp:storage-framework/devel

use-qiodevice
Merge into devel

Proposed by Michi Henning on 2016-11-01

Status:	Merged
Approved by:	James Henstridge on 2016-11-04
Approved revision:	96
Merged at revision:	90
Proposed branch:	lp:~michihenning/storage-framework/use-qiodevice
Merge into:	lp:storage-framework/devel
Prerequisite:	lp:~michihenning/storage-framework/metadata-keys
Diff against target:	2011 lines (+568/-498) 16 files modified include/unity/storage/qt/Downloader.h (+14/-5) include/unity/storage/qt/Uploader.h (+15/-4) include/unity/storage/qt/client/internal/remote_client/HandlerBase.h (+1/-0) include/unity/storage/qt/internal/DownloaderImpl.h (+12/-1) include/unity/storage/qt/internal/Handler.h (+6/-0) include/unity/storage/qt/internal/HandlerBase.h (+0/-2) include/unity/storage/qt/internal/ListJobImplBase.h (+1/-0) include/unity/storage/qt/internal/UploaderImpl.h (+18/-1) include/unity/storage/qt/internal/VoidJobImpl.h (+1/-0) src/qt/Downloader.cpp (+48/-5) src/qt/Uploader.cpp (+48/-5) src/qt/internal/DownloaderImpl.cpp (+97/-41) src/qt/internal/HandlerBase.cpp (+1/-0) src/qt/internal/UploaderImpl.cpp (+171/-59) tests/remote-client/MockProvider.cpp (+4/-10) tests/remote-client/remote-client_test.cpp (+131/-365)
To merge this branch:	bzr merge lp:~michihenning/storage-framework/use-qiodevice
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
James Henstridge		2016-11-01	Approve on 2016-11-04
unity-api-1-bot	continuous-integration		Approve on 2016-11-04
Review via email: mp+309713@code.launchpad.net

Commit message

Changed Uploader and Downloader to derive from QIODevice.

Description of the change

Changed Uploader and Downloader to derive from QIODevice.

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-11-01:

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/162/rebuild

review: Approve (continuous-integration)

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-11-01:

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/164/rebuild

review: Approve (continuous-integration)

lp:~michihenning/storage-framework/use-qiodevice updated on 2016-11-02

90. By Michi Henning on 2016-11-02: Merged dependent branch.
91. By Michi Henning on 2016-11-02: Merged dependent branch.

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-11-02:

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/169/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-11-02:

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/172/rebuild

review: Approve (continuous-integration)

Revision history for this message

James Henstridge (jamesh) wrote on 2016-11-02:

I've noted a few small issues via inline comments

review: Needs Fixing

Revision history for this message

Michi Henning (michihenning) wrote on 2016-11-02:

I removed the stale file from the merge conflict and got rid of the redundant Q_INVOKABLE, thanks!

For the uploader, my thinking was that the caller should not write to the uploader until it enters the Ready state.

I need to check what happens if a write is done before then. I would expect the write to return an error seeing that underlying socket isn't connected at that point. (If that isn't the case, we can easily arrange for the write to return -1.)

I think forcing the caller to wait until the socket is ready is better than buffering the data. After all, the socket may not ever become ready due to an error (such as the server side going belly-up for some reason) and, if the client is allowed to write regardless, all the data just ends up piling up in memory.

lp:~michihenning/storage-framework/use-qiodevice updated on 2016-11-02

92. By Michi Henning on 2016-11-02: Got rid of merge conflict file and redundant Q_INVOKABLEs.

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-11-02:

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/176/rebuild

review: Approve (continuous-integration)

lp:~michihenning/storage-framework/use-qiodevice updated on 2016-11-03

93. By Michi Henning on 2016-11-03: Merged devel.

Revision history for this message

James Henstridge (jamesh) wrote on 2016-11-03:

The Uploader QIODevice is in its open state.

The expected semantics are that you can write to the device and those writes will be buffered, with the bytesWritten signal being used to notify when the write has completed.

While it is possible to signal an error, it seems that this is generally used for hard errors rather than simple "can't complete the write right now" errors. So if we throw away the early writes, we're effectively breaking the contract.

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-11-03:

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/180/rebuild

review: Approve (continuous-integration)

lp:~michihenning/storage-framework/use-qiodevice updated on 2016-11-04

94. By Michi Henning on 2016-11-04: Added code to deal with synchronous wait on uploader.
Not tested yet, because we need to make changes to the mock provider harness for this.

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-11-04:

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/186/rebuild

review: Approve (continuous-integration)

Revision history for this message

James Henstridge (jamesh) wrote on 2016-11-04:

This looks very close to ready. I've left a few inline comments. Some are just stylistic so could be ignored, so some could be left as is.

lp:~michihenning/storage-framework/use-qiodevice updated on 2016-11-04

95. By Michi Henning on 2016-11-04: Review comments from James.
96. By Michi Henning on 2016-11-04: Renamed finishUpload() and finishDownload() to close().

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-11-04:

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/187/rebuild

review: Approve (continuous-integration)

Revision history for this message

James Henstridge (jamesh) wrote on 2016-11-04:

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Michi Henning

 === modified file 'include/unity/storage/qt/Downloader.h'
 --- include/unity/storage/qt/Downloader.h	2016-10-31 02:01:05 +0000
 +++ include/unity/storage/qt/Downloader.h	2016-11-04 07:34:14 +0000
@@ -36,7 +36,7 @@
  }  // namespace internal
--class Q_DECL_EXPORT Downloader final : public QLocalSocket
++class Q_DECL_EXPORT Downloader final : public QIODevice
+ {
      Q_OBJECT
      Q_PROPERTY(bool isValid READ isValid NOTIFY statusChanged FINAL)
@@ -51,15 +51,21 @@
      Downloader();
      virtual ~Downloader();
--    bool isValid() const;        // Not nice, hides QLocalSocket::isValid()
++    bool isValid() const;
      Status status() const;
--    StorageError error() const;  // Not nice, hides QLocalSocket::error()
++    StorageError error() const;
      Item item() const;
--    Q_INVOKABLE void finishDownload();
      Q_INVOKABLE void cancel();
--    // TODO: will probably need QML invokable methods for reading and writing to/from QIODevice
++    // From QLocalSocket interface.
++    Q_INVOKABLE void close();
++    Q_INVOKABLE qint64 bytesAvailable() const override;
++    Q_INVOKABLE qint64 bytesToWrite() const override;
++    Q_INVOKABLE bool canReadLine() const override;
++    Q_INVOKABLE bool isSequential() const override;
++    Q_INVOKABLE bool waitForBytesWritten(int msecs = 30000) override;
++    Q_INVOKABLE bool waitForReadyRead(int msecs = 30000) override;
  Q_SIGNALS:
      void statusChanged(unity::storage::qt::Downloader::Status status) const;
@@ -67,6 +73,9 @@
  private:
      Downloader(std::unique_ptr<internal::DownloaderImpl> p);
++    qint64 readData(char* data, qint64 c);
++    qint64 writeData(char const* data, qint64 c);
++
      std::unique_ptr<internal::DownloaderImpl> p_;
      friend class internal::DownloaderImpl;
 === modified file 'include/unity/storage/qt/Uploader.h'
 --- include/unity/storage/qt/Uploader.h	2016-10-12 05:25:20 +0000
 +++ include/unity/storage/qt/Uploader.h	2016-11-04 07:34:14 +0000
@@ -39,7 +39,7 @@
  class Item;
  class StorageError;
--class Q_DECL_EXPORT Uploader final : public QLocalSocket
++class Q_DECL_EXPORT Uploader final : public QIODevice
+ {
      Q_OBJECT
      Q_PROPERTY(bool isValid READ isValid NOTIFY statusChanged FINAL)
@@ -56,22 +56,33 @@
      Uploader();
      virtual ~Uploader();
--    bool isValid() const;  // Not nice, hides QLocalSocket::isValid()
++    bool isValid() const;
      Status status() const;
--    StorageError error() const;  // Not nice, hides QLocalSocket::error()
++    StorageError error() const;
      Item::ConflictPolicy policy() const;
      qint64 sizeInBytes() const;
      Item item() const;
--    Q_INVOKABLE void finishUpload();
      Q_INVOKABLE void cancel();
++    // From QLocalSocket interface.
++    Q_INVOKABLE void close() override;
++    Q_INVOKABLE qint64 bytesAvailable() const override;
++    Q_INVOKABLE qint64 bytesToWrite() const override;
++    Q_INVOKABLE bool canReadLine() const override;
++    Q_INVOKABLE bool isSequential() const override;
++    Q_INVOKABLE bool waitForBytesWritten(int msecs = 30000) override;
++    Q_INVOKABLE bool waitForReadyRead(int msecs = 30000) override;
++
  Q_SIGNALS:
      void statusChanged(unity::storage::qt::Uploader::Status status) const;
  private:
      Uploader(std::unique_ptr<internal::UploaderImpl> p);
++    qint64 readData(char* data, qint64 c);
++    qint64 writeData(char const* data, qint64 c);
++
      std::unique_ptr<internal::UploaderImpl> p_;
      friend class internal::UploaderImpl;
 === modified file 'include/unity/storage/qt/client/internal/remote_client/HandlerBase.h'
 --- include/unity/storage/qt/client/internal/remote_client/HandlerBase.h	2016-07-13 05:12:47 +0000
 +++ include/unity/storage/qt/client/internal/remote_client/HandlerBase.h	2016-11-04 07:34:14 +0000
@@ -19,6 +19,7 @@
  #pragma once
  #pragma GCC diagnostic push
++#pragma GCC diagnostic ignored "-Wcast-align"
  #pragma GCC diagnostic ignored "-Wctor-dtor-privacy"
  #pragma GCC diagnostic ignored "-Wswitch-default"
  #include <QDBusPendingCallWatcher>
 === modified file 'include/unity/storage/qt/internal/DownloaderImpl.h'
 --- include/unity/storage/qt/internal/DownloaderImpl.h	2016-10-12 07:23:15 +0000
 +++ include/unity/storage/qt/internal/DownloaderImpl.h	2016-11-04 07:34:14 +0000
@@ -47,9 +47,19 @@
      StorageError error() const;
      Item item() const;
--    void finishDownload();
      void cancel();
++    // From QLocalSocket interface.
++    void close();
++    qint64 bytesAvailable() const;
++    qint64 bytesToWrite() const;
++    bool canReadLine() const;
++    bool isSequential() const;
++    bool waitForBytesWritten(int msecs);
++    bool waitForReadyRead(int msecs);
++    qint64 readData(char* data, qint64 c);
++    qint64 writeData(char const* data, qint64 c);
++
      static Downloader* make_job(std::shared_ptr<ItemImpl> const& item_impl,
                                  QString const& method,
                                  QDBusPendingReply<QString, QDBusUnixFileDescriptor>& reply);
@@ -62,6 +72,7 @@
      std::shared_ptr<ItemImpl> item_impl_;
      QString download_id_;
      QDBusUnixFileDescriptor fd_;
++    QLocalSocket socket_;
      bool finalizing_ = false;
  };
 === modified file 'include/unity/storage/qt/internal/Handler.h'
 --- include/unity/storage/qt/internal/Handler.h	2016-10-12 07:23:15 +0000
 +++ include/unity/storage/qt/internal/Handler.h	2016-11-04 07:34:14 +0000
@@ -102,6 +102,12 @@
                            })
+     {
+     }
++
++    void wait_and_process_now()
++    {
++        watcher_.waitForFinished();
++        finished(&watcher_);
++    }
  };
  }  // namespace internal
 === modified file 'include/unity/storage/qt/internal/HandlerBase.h'
 --- include/unity/storage/qt/internal/HandlerBase.h	2016-09-21 00:29:49 +0000
 +++ include/unity/storage/qt/internal/HandlerBase.h	2016-11-04 07:34:14 +0000
@@ -53,8 +53,6 @@
  protected:
      QDBusPendingCallWatcher watcher_;
--
--private:
      std::function<void(QDBusPendingCallWatcher&)> closure_;
  };
 === modified file 'include/unity/storage/qt/internal/ListJobImplBase.h'
 --- include/unity/storage/qt/internal/ListJobImplBase.h	2016-10-12 05:25:20 +0000
 +++ include/unity/storage/qt/internal/ListJobImplBase.h	2016-11-04 07:34:14 +0000
@@ -22,6 +22,7 @@
  #include <unity/storage/qt/StorageError.h>
  #pragma GCC diagnostic push
++#pragma GCC diagnostic ignored "-Wcast-align"
  #pragma GCC diagnostic ignored "-Wctor-dtor-privacy"
  #include <QDBusPendingReply>
  #pragma GCC diagnostic pop
 === modified file 'include/unity/storage/qt/internal/UploaderImpl.h'
 --- include/unity/storage/qt/internal/UploaderImpl.h	2016-11-03 03:41:24 +0000
 +++ include/unity/storage/qt/internal/UploaderImpl.h	2016-11-04 07:34:14 +0000
@@ -18,10 +18,12 @@
  #pragma once
++#include <unity/storage/qt/internal/Handler.h>
  #include <unity/storage/qt/Uploader.h>
  #include <QDBusPendingReply>
  #include <QDBusUnixFileDescriptor>
++#include <QPointer>
  namespace unity
+ {
@@ -59,9 +61,19 @@
      qint64 sizeInBytes() const;
      Item item() const;
--    void finishUpload();
      void cancel();
++    // From QLocalSocket interface.
++    void close();
++    qint64 bytesAvailable() const;
++    qint64 bytesToWrite() const;
++    bool canReadLine() const;
++    bool isSequential() const;
++    bool waitForBytesWritten(int msecs);
++    bool waitForReadyRead(int msecs);
++    qint64 readData(char* data, qint64 c);
++    qint64 writeData(char const* data, qint64 c);
++
      static Uploader* make_job(std::shared_ptr<ItemImpl> const& item_impl,
                                QString const& method,
                                QDBusPendingReply<QString, QDBusUnixFileDescriptor>& reply,
@@ -70,6 +82,8 @@
                                qint64 size_in_bytes);
      static Uploader* make_job(StorageError const& e);
++    qint64 flush_buffer();
++
  private:
      Uploader* public_instance_;
      Uploader::Status status_;
@@ -79,8 +93,11 @@
      std::function<void(storage::internal::ItemMetadata const&)> validate_;
      Item::ConflictPolicy policy_ = Item::ConflictPolicy::IgnoreConflict;
      qint64 size_in_bytes_ = 0;
++    QPointer<Handler<QDBusPendingReply<QString, QDBusUnixFileDescriptor>>> handler_;
      QString upload_id_;
      QDBusUnixFileDescriptor fd_;
++    QLocalSocket socket_;
++    QByteArray buffer_;
      bool finalizing_ = false;
  };
 === modified file 'include/unity/storage/qt/internal/VoidJobImpl.h'
 --- include/unity/storage/qt/internal/VoidJobImpl.h	2016-10-12 08:08:48 +0000
 +++ include/unity/storage/qt/internal/VoidJobImpl.h	2016-11-04 07:34:14 +0000
@@ -23,6 +23,7 @@
  #include <unity/storage/qt/StorageError.h>
  #pragma GCC diagnostic push
++#pragma GCC diagnostic ignored "-Wcast-align"
  #pragma GCC diagnostic ignored "-Wctor-dtor-privacy"
  #include <QDBusPendingReply>
  #pragma GCC diagnostic pop
 === modified file 'src/qt/Downloader.cpp'
 --- src/qt/Downloader.cpp	2016-10-11 08:12:32 +0000
 +++ src/qt/Downloader.cpp	2016-11-04 07:34:14 +0000
@@ -61,16 +61,59 @@
      return p_->item();
+ }
--void Downloader::finishDownload()
--{
--    p_->finishDownload();
--}
--
  void Downloader::cancel()
+ {
      p_->cancel();
+ }
++void Downloader::close()
++{
++    p_->close();
++}
++
++qint64 Downloader::bytesAvailable() const
++{
++    return p_->bytesAvailable();
++}
++
++qint64 Downloader::bytesToWrite() const
++{
++    return p_->bytesToWrite();
++}
++
++bool Downloader::canReadLine() const
++{
++    return p_->canReadLine();
++}
++
++bool Downloader::isSequential() const
++{
++    return p_->isSequential();
++}
++
++bool Downloader::waitForBytesWritten(int msecs)
++{
++    return p_->waitForBytesWritten(msecs);
++}
++
++bool Downloader::waitForReadyRead(int msecs)
++{
++    return p_->waitForReadyRead(msecs);
++}
++
++qint64 Downloader::readData(char* data, qint64 c)
++{
++    return p_->readData(data, c);
++}
++
++// LCOV_EXCL_START
++// Never called by QIODevice because device is opened read-only.
++qint64 Downloader::writeData(char const* data, qint64 c)
++{
++    return p_->writeData(data, c);
++}
++// LCOV_EXCL_STOP
++
  }  // namespace qt
  }  // namespace storage
  }  // namespace unity
 === modified file 'src/qt/Uploader.cpp'
 --- src/qt/Uploader.cpp	2016-10-12 05:25:20 +0000
 +++ src/qt/Uploader.cpp	2016-11-04 07:34:14 +0000
@@ -71,16 +71,59 @@
      return p_->item();
+ }
--void Uploader::finishUpload()
--{
--    p_->finishUpload();
--}
--
  void Uploader::cancel()
+ {
      p_->cancel();
+ }
++void Uploader::close()
++{
++    p_->close();
++}
++
++qint64 Uploader::bytesAvailable() const
++{
++    return p_->bytesAvailable();
++}
++
++qint64 Uploader::bytesToWrite() const
++{
++    return p_->bytesToWrite();
++}
++
++bool Uploader::canReadLine() const
++{
++    return p_->canReadLine();
++}
++
++bool Uploader::isSequential() const
++{
++    return p_->isSequential();
++}
++
++bool Uploader::waitForBytesWritten(int msecs)
++{
++    return p_->waitForBytesWritten(msecs);
++}
++
++bool Uploader::waitForReadyRead(int msecs)
++{
++    return p_->waitForReadyRead(msecs);
++}
++
++// LCOV_EXCL_START
++// Never called by QIODevice because device is opened write-only.
++qint64 Uploader::readData(char* data, qint64 c)
++{
++    return p_->readData(data, c);
++}
++// LCOV_EXCL_STOP
++
++qint64 Uploader::writeData(char const* data, qint64 c)
++{
++    return p_->writeData(data, c);
++}
++
  }  // namespace qt
  }  // namespace storage
  }  // namespace unity
 === modified file 'src/qt/internal/DownloaderImpl.cpp'
 --- src/qt/internal/DownloaderImpl.cpp	2016-10-13 06:48:11 +0000
 +++ src/qt/internal/DownloaderImpl.cpp	2016-11-04 07:34:14 +0000
@@ -58,7 +58,7 @@
+         {
              QString msg = method + ": Runtime was destroyed previously";
              error_ = StorageErrorImpl::runtime_destroyed_error(msg);
--            public_instance_->abort();
++            socket_.abort();
              public_instance_->setErrorString(msg);
              status_ = Downloader::Status::Error;
              Q_EMIT public_instance_->statusChanged(status_);
@@ -73,7 +73,7 @@
              QString msg = method + ": invalid file descriptor returned by provider";
              qCritical().noquote() << msg;
              error_ = StorageErrorImpl::local_comms_error(msg);
--            public_instance_->abort();
++            socket_.abort();
              public_instance_->setErrorString(msg);
              status_ = Downloader::Status::Error;
              Q_EMIT public_instance_->statusChanged(status_);
@@ -81,7 +81,18 @@
              // LCOV_EXCL_STOP
+         }
--        public_instance_->setSocketDescriptor(fd_.fileDescriptor(), QLocalSocket::ConnectedState, QIODevice::ReadOnly);
++        // We forward any QIODevice signals emitted by the socket to the public instance.
++        connect(&socket_, &QIODevice::aboutToClose, public_instance_, &QIODevice::aboutToClose);
++        connect(&socket_, &QIODevice::bytesWritten, public_instance_, &QIODevice::bytesWritten);
++        connect(&socket_, &QIODevice::readChannelFinished, public_instance_, &QIODevice::readChannelFinished);
++        connect(&socket_, &QIODevice::readyRead, public_instance_, &QIODevice::readyRead);
++
++#if QT_VERSION >= QT_VERSION_CHECK(5, 7, 0)
++        connect(&socket_, &QIODevice::channelBytesWritten, public_instance_, &QIODevice::channelBytesWritten);
++        connect(&socket_, &QIODevice::channelReadyRead, public_instance_, &QIODevice::channelReadyRead);
++#endif
++
++        socket_.setSocketDescriptor(fd_.fileDescriptor(), QLocalSocket::ConnectedState, QIODevice::ReadOnly);
          status_ = Downloader::Status::Ready;
          Q_EMIT public_instance_->statusChanged(status_);
      };
@@ -91,12 +102,12 @@
          // TODO: This does not set the method
          error_ = error;
          status_ = Downloader::Status::Error;
--        public_instance_->abort();
++        socket_.abort();
          public_instance_->setErrorString(error.errorString());
          Q_EMIT public_instance_->statusChanged(status_);
      };
--    new Handler<storage::internal::ItemMetadata>(this, reply, process_reply, process_error);
++    new Handler<QDBusPendingReply<QString, QDBusUnixFileDescriptor>>(this, reply, process_reply, process_error);
+ }
  DownloaderImpl::DownloaderImpl(StorageError const& e)
@@ -146,11 +157,40 @@
      return Item(item_impl_);
+ }
--void DownloaderImpl::finishDownload()
--{
--    static QString const method = "Downloader::finishDownload()";
--
--    // If we encountered an error earlier or were cancelled, or if finishDownload() was
++void DownloaderImpl::cancel()
++{
++    static QString const method = "Downloader::cancel()";
++
++    // If we are in a final state already, ignore the call.
++    if (   status_ == Downloader::Status::Error
++        || status_ == Downloader::Status::Finished
++        || status_ == Downloader::Status::Cancelled)
++    {
++        return;
++    }
++    auto runtime = item_impl_->runtime_impl();
++    if (!runtime || !runtime->isValid())
++    {
++        QString msg = method + ": Runtime was destroyed previously";
++        error_ = StorageErrorImpl::runtime_destroyed_error(msg);
++        status_ = Downloader::Status::Error;
++        Q_EMIT public_instance_->statusChanged(status_);
++        return;
++    }
++
++    QString msg = method + ": download was cancelled";
++    error_ = StorageErrorImpl::cancelled_error(msg);
++    socket_.abort();
++    public_instance_->setErrorString(msg);
++    status_ = Downloader::Status::Cancelled;
++    Q_EMIT public_instance_->statusChanged(status_);
++}
++
++void DownloaderImpl::close()
++{
++    static QString const method = "Downloader::close()";
++
++    // If we encountered an error earlier or were cancelled, or if close() was
      // called already, we ignore the call.
      if (status_ == Downloader::Status::Error || status_ == Downloader::Status::Cancelled || finalizing_)
+     {
@@ -162,7 +202,7 @@
+     {
          QString msg = method + ": cannot finalize while Downloader is not in the Ready state";
          error_ = StorageErrorImpl::logic_error(msg);
--        public_instance_->abort();
++        socket_.abort();
          public_instance_->setErrorString(msg);
          status_ = Downloader::Status::Error;
          Q_EMIT public_instance_->statusChanged(status_);
@@ -194,7 +234,7 @@
+         {
              QString msg = method + ": Runtime was destroyed previously";
              error_ = StorageErrorImpl::runtime_destroyed_error(msg);
--            public_instance_->abort();
++            socket_.abort();
              public_instance_->setErrorString(msg);
              status_ = Downloader::Status::Error;
              Q_EMIT public_instance_->statusChanged(status_);
@@ -214,7 +254,7 @@
          // TODO: this doesn't set the method
          error_ = error;
--        public_instance_->abort();
++        socket_.abort();
          public_instance_->setErrorString(error.errorString());
          status_ = Downloader::Status::Error;
          Q_EMIT public_instance_->statusChanged(status_);
@@ -223,34 +263,48 @@
      new Handler<void>(this, reply, process_reply, process_error);
+ }
--void DownloaderImpl::cancel()
--{
--    static QString const method = "Downloader::cancel()";
--
--    // If we are in a final state already, ignore the call.
--    if (   status_ == Downloader::Status::Error
--        || status_ == Downloader::Status::Finished
--        || status_ == Downloader::Status::Cancelled)
--    {
--        return;
--    }
--    auto runtime = item_impl_->runtime_impl();
--    if (!runtime || !runtime->isValid())
--    {
--        QString msg = method + ": Runtime was destroyed previously";
--        error_ = StorageErrorImpl::runtime_destroyed_error(msg);
--        status_ = Downloader::Status::Error;
--        Q_EMIT public_instance_->statusChanged(status_);
--        return;
--    }
--
--    QString msg = method + ": download was cancelled";
--    error_ = StorageErrorImpl::cancelled_error(msg);
--    public_instance_->abort();
--    public_instance_->setErrorString(msg);
--    status_ = Downloader::Status::Cancelled;
--    Q_EMIT public_instance_->statusChanged(status_);
--}
++qint64 DownloaderImpl::bytesAvailable() const
++{
++    return socket_.bytesAvailable();
++}
++
++qint64 DownloaderImpl::bytesToWrite() const
++{
++    return socket_.bytesToWrite();
++}
++
++bool DownloaderImpl::canReadLine() const
++{
++    return socket_.canReadLine();
++}
++
++bool DownloaderImpl::isSequential() const
++{
++    return socket_.isSequential();
++}
++
++bool DownloaderImpl::waitForBytesWritten(int msecs)
++{
++    return socket_.waitForBytesWritten(msecs);
++}
++
++bool DownloaderImpl::waitForReadyRead(int msecs)
++{
++    return socket_.waitForReadyRead(msecs);
++}
++
++qint64 DownloaderImpl::readData(char* data, qint64 c)
++{
++    return socket_.read(data, c);
++}
++
++// LCOV_EXCL_START
++// Never called by QIODevice because device is opened read-only.
++qint64 DownloaderImpl::writeData(char const* data, qint64 c)
++{
++    return socket_.write(data, c);
++}
++// LCOV_EXCL_STOP
  Downloader* DownloaderImpl::make_job(shared_ptr<ItemImpl> const& item_impl,
                                       QString const& method,
@@ -258,6 +312,7 @@
+ {
      unique_ptr<DownloaderImpl> impl(new DownloaderImpl(item_impl, method, reply));
      auto downloader = new Downloader(move(impl));
++    downloader->open(QIODevice::ReadOnly);
      downloader->p_->public_instance_ = downloader;
      return downloader;
+ }
@@ -266,6 +321,7 @@
+ {
      unique_ptr<DownloaderImpl> impl(new DownloaderImpl(e));
      auto downloader = new Downloader(move(impl));
++    downloader->open(QIODevice::ReadOnly);
      downloader->p_->public_instance_ = downloader;
      QMetaObject::invokeMethod(downloader,
                                "statusChanged",
 === modified file 'src/qt/internal/HandlerBase.cpp'
 --- src/qt/internal/HandlerBase.cpp	2016-09-20 04:53:49 +0000
 +++ src/qt/internal/HandlerBase.cpp	2016-11-04 07:34:14 +0000
@@ -50,6 +50,7 @@
  void HandlerBase::finished(QDBusPendingCallWatcher* call)
+ {
      deleteLater();
++    disconnect(&watcher_, &QDBusPendingCallWatcher::finished, this, &HandlerBase::finished);
      closure_(*call);
+ }
 === modified file 'src/qt/internal/UploaderImpl.cpp'
 --- src/qt/internal/UploaderImpl.cpp	2016-10-13 07:59:12 +0000
 +++ src/qt/internal/UploaderImpl.cpp	2016-11-04 07:34:14 +0000
@@ -19,7 +19,7 @@
  #include <unity/storage/qt/internal/UploaderImpl.h>
  #include "ProviderInterface.h"
--#include <unity/storage/qt/internal/Handler.h>
++//#include <unity/storage/qt/internal/Handler.h>
  #include <unity/storage/qt/internal/ItemImpl.h>
  #include <unity/storage/qt/internal/VoidJobImpl.h>
  #include <unity/storage/qt/ItemJob.h>
@@ -55,7 +55,7 @@
      assert(!method.isEmpty());
      assert(size_in_bytes >= 0);
--    auto process_reply = [this, method](decltype(reply)& r)
++    auto process_reply = [this, method](QDBusPendingReply<QString, QDBusUnixFileDescriptor>& r)
+     {
          if (status_ != Uploader::Status::Loading)
+         {
@@ -67,7 +67,7 @@
+         {
              QString msg = method + ": Runtime was destroyed previously";
              error_ = StorageErrorImpl::runtime_destroyed_error(msg);
--            public_instance_->abort();
++            socket_.abort();
              public_instance_->setErrorString(msg);
              status_ = Uploader::Status::Error;
              Q_EMIT public_instance_->statusChanged(status_);
@@ -82,7 +82,7 @@
              QString msg = method + ": invalid file descriptor returned by provider";
              qCritical().noquote() << msg;
              error_ = StorageErrorImpl::local_comms_error(msg);
--            public_instance_->abort();
++            socket_.abort();
              public_instance_->setErrorString(msg);
              status_ = Uploader::Status::Error;
              Q_EMIT public_instance_->statusChanged(status_);
@@ -90,7 +90,19 @@
              // LCOV_EXCL_STOP
+         }
--        public_instance_->setSocketDescriptor(fd_.fileDescriptor(), QLocalSocket::ConnectedState, QIODevice::WriteOnly);
++        // We forward any QIODevice signals emitted by the socket to the public instance.
++        connect(&socket_, &QIODevice::aboutToClose, public_instance_, &QIODevice::aboutToClose);
++        connect(&socket_, &QIODevice::bytesWritten, public_instance_, &QIODevice::bytesWritten);
++        connect(&socket_, &QIODevice::readChannelFinished, public_instance_, &QIODevice::readChannelFinished);
++        connect(&socket_, &QIODevice::readyRead, public_instance_, &QIODevice::readyRead);
++
++#if QT_VERSION >= QT_VERSION_CHECK(5, 7, 0)
++        connect(&socket_, &QIODevice::channelBytesWritten, public_instance_, &QIODevice::channelBytesWritten);
++        connect(&socket_, &QIODevice::channelReadyRead, public_instance_, &QIODevice::channelReadyRead);
++#endif
++
++        socket_.setSocketDescriptor(fd_.fileDescriptor(), QLocalSocket::ConnectedState, QIODevice::WriteOnly);
++        flush_buffer();
          status_ = Uploader::Status::Ready;
          Q_EMIT public_instance_->statusChanged(status_);
      };
@@ -100,12 +112,12 @@
          // TODO: This does not set the method
          error_ = error;
          status_ = Uploader::Status::Error;
--        public_instance_->abort();
++        socket_.abort();
          public_instance_->setErrorString(error.errorString());
          Q_EMIT public_instance_->statusChanged(status_);
      };
--    new Handler<storage::internal::ItemMetadata>(this, reply, process_reply, process_error);
++    handler_ = new Handler<QDBusPendingReply<QString, QDBusUnixFileDescriptor>>(this, reply, process_reply, process_error);
+ }
  UploaderImpl::UploaderImpl(StorageError const& e)
@@ -165,11 +177,56 @@
      return Item(item_impl_);
+ }
--void UploaderImpl::finishUpload()
--{
--    static QString const method = "Uploader::finishUpload()";
--
--    // If we encountered an error earlier or were cancelled, or if finishUpload() was
++void UploaderImpl::cancel()
++{
++    static QString const method = "Uploader::cancel()";
++
++    // If we are in a final state already, ignore the call.
++    if (   status_ == Uploader::Status::Error
++        || status_ == Uploader::Status::Finished
++        || status_ == Uploader::Status::Cancelled)
++    {
++        return;
++    }
++    auto runtime = item_impl_->runtime_impl();
++    if (!runtime || !runtime->isValid())
++    {
++        QString msg = method + ": Runtime was destroyed previously";
++        error_ = StorageErrorImpl::runtime_destroyed_error(msg);
++        status_ = Uploader::Status::Error;
++        Q_EMIT public_instance_->statusChanged(status_);
++        return;
++    }
++
++    if (!upload_id_.isEmpty())
++    {
++        // We just send the cancel and ignore any reply because it is best-effort only.
++        auto reply = item_impl_->account_impl()->provider()->CancelUpload(upload_id_);
++
++        auto process_reply = [](decltype(reply)&)
++        {
++        };
++
++        auto process_error = [](StorageError const&)
++        {
++        };
++
++        new Handler<void>(this, reply, process_reply, process_error);
++    }
++
++    QString msg = method + ": upload was cancelled";
++    error_ = StorageErrorImpl::cancelled_error(msg);
++    socket_.abort();
++    public_instance_->setErrorString(msg);
++    status_ = Uploader::Status::Cancelled;
++    Q_EMIT public_instance_->statusChanged(status_);
++}
++
++void UploaderImpl::close()
++{
++    static QString const method = "Uploader::close()";
++
++    // If we encountered an error earlier or were cancelled, or if close() was
      // called already, we ignore the call.
      if (status_ == Uploader::Status::Error || status_ == Uploader::Status::Cancelled || finalizing_)
+     {
@@ -181,7 +238,7 @@
+     {
          QString msg = method + ": cannot finalize while Uploader is not in the Ready state";
          error_ = StorageErrorImpl::logic_error(msg);
--        public_instance_->abort();
++        socket_.abort();
          public_instance_->setErrorString(msg);
          status_ = Uploader::Status::Error;
          Q_EMIT public_instance_->statusChanged(status_);
@@ -199,7 +256,8 @@
+     }
      finalizing_ = true;
--    public_instance_->disconnectFromServer();
++    flush_buffer();
++    socket_.disconnectFromServer();
      auto reply = item_impl_->account_impl()->provider()->FinishUpload(upload_id_);
      auto process_reply = [this](decltype(reply)& r)
@@ -214,7 +272,7 @@
+         {
              QString msg = method + ": Runtime was destroyed previously";
              error_ = StorageErrorImpl::runtime_destroyed_error(msg);
--            public_instance_->abort();
++            socket_.abort();
              public_instance_->setErrorString(msg);
              status_ = Uploader::Status::Error;
              Q_EMIT public_instance_->statusChanged(status_);
@@ -247,7 +305,7 @@
          // TODO: this doesn't set the method
          error_ = error;
--        public_instance_->abort();
++        socket_.abort();
          public_instance_->setErrorString(error.errorString());
          status_ = Uploader::Status::Error;
          Q_EMIT public_instance_->statusChanged(status_);
@@ -256,49 +314,85 @@
      new Handler<void>(this, reply, process_reply, process_error);
+ }
--void UploaderImpl::cancel()
--{
--    static QString const method = "Uploader::cancel()";
--
--    // If we are in a final state already, ignore the call.
--    if (   status_ == Uploader::Status::Error
--        || status_ == Uploader::Status::Finished
--        || status_ == Uploader::Status::Cancelled)
--    {
--        return;
--    }
--    auto runtime = item_impl_->runtime_impl();
--    if (!runtime || !runtime->isValid())
--    {
--        QString msg = method + ": Runtime was destroyed previously";
--        error_ = StorageErrorImpl::runtime_destroyed_error(msg);
--        status_ = Uploader::Status::Error;
--        Q_EMIT public_instance_->statusChanged(status_);
--        return;
--    }
--
--    if (!upload_id_.isEmpty())
--    {
--        // We just send the cancel and ignore any reply because it is best-effort only.
--        auto reply = item_impl_->account_impl()->provider()->CancelUpload(upload_id_);
--
--        auto process_reply = [](decltype(reply)&)
--        {
--        };
--
--        auto process_error = [](StorageError const&)
--        {
--        };
--
--        new Handler<void>(this, reply, process_reply, process_error);
--    }
--
--    QString msg = method + ": upload was cancelled";
--    error_ = StorageErrorImpl::cancelled_error(msg);
--    public_instance_->abort();
--    public_instance_->setErrorString(msg);
--    status_ = Uploader::Status::Cancelled;
--    Q_EMIT public_instance_->statusChanged(status_);
++qint64 UploaderImpl::bytesAvailable() const
++{
++    return socket_.bytesAvailable();
++}
++
++qint64 UploaderImpl::bytesToWrite() const
++{
++    return socket_.bytesToWrite();
++}
++
++bool UploaderImpl::canReadLine() const
++{
++    return socket_.canReadLine();
++}
++
++bool UploaderImpl::isSequential() const
++{
++    return socket_.isSequential();
++}
++
++bool UploaderImpl::waitForBytesWritten(int msecs)
++{
++    if (status_ == Uploader::Status::Loading)
++    {
++        // Unfortunately, QDBusPendingReply::waitForFinished() does not accept a timeout.
++        // The next-best thing we can do is to simply wait without a timeout. The DBus
++        // method will finish eventually, even though it might take a lot longer than msecs.
++        handler_->wait_and_process_now();
++    }
++    if (flush_buffer() == -1)
++    {
++        return false;
++    }
++    return socket_.waitForBytesWritten(msecs);
++}
++
++bool UploaderImpl::waitForReadyRead(int msecs)
++{
++    return socket_.waitForReadyRead(msecs);
++}
++
++// LCOV_EXCL_START
++// Never called by QIODevice because device is opened write-only.
++qint64 UploaderImpl::readData(char* data, qint64 c)
++{
++    return socket_.read(data, c);
++}
++// LCOV_EXCL_STOP
++
++qint64 UploaderImpl::writeData(char const* data, qint64 c)
++{
++    switch (status_)
++    {
++        case Uploader::Status::Loading:
++        {
++            // Client is writing before we have received the file descriptor from the provider.
++            buffer_.append(data, c);
++            return c;
++        }
++        case Uploader::Status::Ready:
++        {
++            if (flush_buffer() == -1)
++            {
++                return -1;
++            }
++            return socket_.write(data, c);
++        }
++        case Uploader::Status::Cancelled:
++        case Uploader::Status::Finished:
++        case Uploader::Status::Error:
++        {
++            return -1;  // Can't write to an already-finalized uploader.
++        }
++        default:
++        {
++            abort();  // Impossible  // LCOV_EXCL_LINE
++        }
++    }
++    // NOTREACHED
+ }
  Uploader* UploaderImpl::make_job(shared_ptr<ItemImpl> const& item_impl,
@@ -310,6 +404,7 @@
+ {
      unique_ptr<UploaderImpl> impl(new UploaderImpl(item_impl, method, reply, validate, policy, size_in_bytes));
      auto uploader = new Uploader(move(impl));
++    uploader->open(QIODevice::WriteOnly);
      uploader->p_->public_instance_ = uploader;
      return uploader;
+ }
@@ -318,6 +413,7 @@
+ {
      unique_ptr<UploaderImpl> impl(new UploaderImpl(e));
      auto uploader = new Uploader(move(impl));
++    uploader->open(QIODevice::WriteOnly);
      uploader->p_->public_instance_ = uploader;
      QMetaObject::invokeMethod(uploader,
                                "statusChanged",
@@ -326,6 +422,22 @@
      return uploader;
+ }
++qint64 UploaderImpl::flush_buffer()
++{
++    qint64 bytes_written = 0;
++    auto bytes_to_write = buffer_.size();
++    if (bytes_to_write > 0)
++    {
++        auto bytes_written = socket_.write(buffer_.data(), bytes_to_write);
++        if (bytes_written != bytes_to_write)
++        {
++            return -1;  // Not exactly detailed, but that's the best we can do.  // LCOV_EXCL_LINE
++        }
++        buffer_.resize(0);
++    }
++    return bytes_written;
++}
++
  }  // namespace internal
  }  // namespace qt
  }  // namespace storage
 === modified file 'tests/remote-client/MockProvider.cpp'
 --- tests/remote-client/MockProvider.cpp	2016-11-03 03:41:24 +0000
 +++ tests/remote-client/MockProvider.cpp	2016-11-04 07:34:14 +0000
@@ -194,9 +194,9 @@
          ++num_calls;
          switch (num_calls)
+         {
++            case 2:
++                return make_exceptional_future<Item>(ResourceException("metadata(): weird error", 42));
              case 3:
--                return make_exceptional_future<Item>(ResourceException("metadata(): weird error", 42));
--            case 4:
                  num_calls = 0;
                  return make_exceptional_future<Item>(RemoteCommsException("metadata(): HTTP broken"));
              default:
@@ -207,14 +207,8 @@
+     {
          if (cmd_ == "bad_parent_metadata_from_child")
+         {
--            ++num_calls;
--            if (num_calls == 2)
--            {
--                num_calls = 0;
--                // On second call, we return type file for the root.
--                Item metadata{"root_id", {}, "Root", "etag", ItemType::file, {}};
--                return make_ready_future<Item>(metadata);
--            }
++            Item metadata{"root_id", {}, "Root", "etag", ItemType::file, {}};
++            return make_ready_future<Item>(metadata);
+         }
          if (cmd_ == "root_with_parent")
+         {
 === modified file 'tests/remote-client/remote-client_test.cpp'
 --- tests/remote-client/remote-client_test.cpp	2016-11-03 07:33:40 +0000
 +++ tests/remote-client/remote-client_test.cpp	2016-11-04 07:34:14 +0000
@@ -1310,14 +1310,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("two_parents")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -1364,14 +1356,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("two_parents_throw")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -1455,14 +1439,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("slow_metadata")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -1489,14 +1465,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("bad_parent_metadata_from_child")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -1600,14 +1568,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -1731,14 +1691,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -2014,14 +1966,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -2102,14 +2046,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -2320,14 +2256,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -2434,14 +2362,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -2468,10 +2388,17 @@
              read_spy.wait(SIGNAL_WAIT_TIME);
+         }
+     }
++
++    EXPECT_EQ(11, downloader->bytesAvailable());
++    EXPECT_EQ(0, downloader->bytesToWrite());
++    EXPECT_EQ(-1, downloader->write("a", 1));
++    EXPECT_FALSE(downloader->waitForBytesWritten(1));
++    EXPECT_FALSE(downloader->waitForReadyRead(1));
++
      auto data = downloader->readAll();
      EXPECT_EQ(QByteArray("Hello world", -1), data);
--    downloader->finishDownload();
++    downloader->close();
      ASSERT_TRUE(status_spy.wait(SIGNAL_WAIT_TIME));
      auto arg = status_spy.takeFirst();
      EXPECT_EQ(Downloader::Status::Finished, qvariant_cast<Downloader::Status>(arg.at(0)));
@@ -2496,14 +2423,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -2521,20 +2440,14 @@
          auto arg = spy.takeFirst();
          EXPECT_EQ(Downloader::Status::Ready, qvariant_cast<Downloader::Status>(arg.at(0)));
+     }
++
++    EXPECT_TRUE(downloader->waitForReadyRead(SIGNAL_WAIT_TIME));
+ }
  TEST_F(DownloadTest, runtime_destroyed)
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -2566,14 +2479,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("download_slow")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -2607,14 +2512,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("download_error")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -2641,10 +2538,10 @@
      EXPECT_EQ(42, downloader->error().errorCode());
      EXPECT_EQ(Item(), downloader->item());
--    // For coverage: call finishDownload() while in the Error state.
++    // For coverage: call close() while in the Error state.
+     {
          QSignalSpy spy(downloader.get(), &Downloader::statusChanged);
--        downloader->finishDownload();
++        downloader->close();
          EXPECT_FALSE(spy.wait(1000));
+     }
+ }
@@ -2653,14 +2550,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -2674,7 +2563,7 @@
      QSignalSpy spy(downloader.get(), &Downloader::statusChanged);
--    downloader->finishDownload();
++    downloader->close();
      ASSERT_EQ(1, spy.count());
      auto arg = spy.takeFirst();
@@ -2683,7 +2572,7 @@
      EXPECT_FALSE(downloader->isValid());
      EXPECT_EQ(Downloader::Status::Error, downloader->status());
      EXPECT_EQ(StorageError::LogicError, downloader->error().type());
--    EXPECT_EQ("LogicError: Downloader::finishDownload(): cannot finalize while Downloader is not in the Ready state",
++    EXPECT_EQ("LogicError: Downloader::close(): cannot finalize while Downloader is not in the Ready state",
                downloader->error().errorString());
+ }
@@ -2691,14 +2580,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -2721,7 +2602,7 @@
      EXPECT_EQ(StorageError::Type::NoError, runtime_->shutdown().type());  // Destroy runtime
--    downloader->finishDownload();
++    downloader->close();
      ASSERT_EQ(1, spy.count());
      auto arg = spy.takeFirst();
@@ -2730,21 +2611,13 @@
      EXPECT_FALSE(downloader->isValid());
      EXPECT_EQ(Downloader::Status::Error, downloader->status());
      EXPECT_EQ(StorageError::RuntimeDestroyed, downloader->error().type());
--    EXPECT_EQ("Downloader::finishDownload(): Runtime was destroyed previously", downloader->error().message());
++    EXPECT_EQ("Downloader::close(): Runtime was destroyed previously", downloader->error().message());
+ }
  TEST_F(DownloadTest, finish_runtime_destroyed_while_reply_outstanding)
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("finish_download_slow")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -2765,7 +2638,7 @@
      QSignalSpy spy(downloader.get(), &Downloader::statusChanged);
--    downloader->finishDownload();
++    downloader->close();
      EXPECT_EQ(StorageError::Type::NoError, runtime_->shutdown().type());  // Destroy runtime, provider still sleeping
@@ -2776,21 +2649,13 @@
      EXPECT_FALSE(downloader->isValid());
      EXPECT_EQ(Downloader::Status::Error, downloader->status());
      EXPECT_EQ(StorageError::RuntimeDestroyed, downloader->error().type());
--    EXPECT_EQ("Downloader::finishDownload(): Runtime was destroyed previously", downloader->error().message());
++    EXPECT_EQ("Downloader::close(): Runtime was destroyed previously", downloader->error().message());
+ }
  TEST_F(DownloadTest, finish_twice)
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -2811,8 +2676,8 @@
      QSignalSpy spy(downloader.get(), &Downloader::statusChanged);
--    downloader->finishDownload();
--    downloader->finishDownload();
++    downloader->close();
++    downloader->close();
      ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
      auto arg = spy.takeFirst();
@@ -2825,14 +2690,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("finish_download_error")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -2853,7 +2710,7 @@
      QSignalSpy spy(downloader.get(), &Downloader::statusChanged);
--    downloader->finishDownload();
++    downloader->close();
      ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
      auto arg = spy.takeFirst();
@@ -2925,14 +2782,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("finish_download_slow_error")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -2953,12 +2802,12 @@
      QSignalSpy spy(downloader.get(), &Downloader::statusChanged);
--    downloader->finishDownload();
++    downloader->close();
      downloader->cancel();
      downloader->cancel();  // Second time for coverage
--    downloader->finishDownload();  // Second time for coverage
++    downloader->close();  // Second time for coverage
      EXPECT_EQ(1, spy.count());
      auto arg = spy.takeFirst();
@@ -2968,7 +2817,7 @@
      EXPECT_EQ(StorageError::Type::Cancelled, downloader->error().type());
      EXPECT_EQ("Downloader::cancel(): download was cancelled", downloader->error().message());
--    // We wait here to get coverage for when the reply to a finishDownload() call
++    // We wait here to get coverage for when the reply to a FinishDownload() call
      // finds the downloader in a final state.
      EXPECT_FALSE(spy.wait(2000));
+ }
@@ -2977,14 +2826,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("finish_download_slow_error")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3025,14 +2866,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3057,10 +2890,92 @@
          EXPECT_EQ(Uploader::Status::Ready, qvariant_cast<Uploader::Status>(arg.at(0)));
+     }
--    EXPECT_EQ(contents.size(), uploader->write(contents));
--
--    QSignalSpy spy(uploader.get(), &Uploader::statusChanged);
--    uploader->finishUpload();
++    EXPECT_EQ(0, uploader->bytesAvailable());
++    EXPECT_EQ(0, uploader->bytesToWrite());
++    char buf;
++    EXPECT_EQ(-1, uploader->read(&buf, 1));
++    EXPECT_FALSE(uploader->waitForReadyRead(1));
++
++    EXPECT_EQ(contents.size(), uploader->write(contents));
++    EXPECT_TRUE(uploader->waitForBytesWritten(contents.size()));
++
++    QSignalSpy spy(uploader.get(), &Uploader::statusChanged);
++    uploader->close();
++    ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++    auto arg = spy.takeFirst();
++    EXPECT_EQ(Uploader::Status::Finished, qvariant_cast<Uploader::Status>(arg.at(0)));
++
++    EXPECT_EQ(Uploader::Status::Finished, uploader->status());
++    EXPECT_EQ(child, uploader->item());
++}
++
++#if 0
++// TODO: This test is currently disabled because a synchronous wait in the client
++//       blocks the single event loop that is shared by the client and the mock provider.
++//       We need to change the test harness to run a separate event loop for the provider.
++//
++TEST_F(UploadTest, write_before_ready_and_wait)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("upload_slow")));
++
++    Item child;
++    {
++        unique_ptr<ItemJob> j(acc_.get("child_id"));
++        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++        spy.wait(SIGNAL_WAIT_TIME);
++        child = j->item();
++    }
++
++    QByteArray contents("Hello world", -1);
++    unique_ptr<Uploader> uploader(child.createUploader(Item::ConflictPolicy::IgnoreConflict, contents.size()));
++    EXPECT_TRUE(uploader->isValid());
++
++    // Don't wait for ready state.
++
++    EXPECT_EQ(contents.size(), uploader->write(contents));
++    EXPECT_TRUE(uploader->waitForBytesWritten(1000));
++
++    QSignalSpy spy(uploader.get(), &Uploader::statusChanged);
++    uploader->close();
++    ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++    auto arg = spy.takeFirst();
++    EXPECT_EQ(Uploader::Status::Finished, qvariant_cast<Uploader::Status>(arg.at(0)));
++
++    EXPECT_EQ(Uploader::Status::Finished, uploader->status());
++    EXPECT_EQ(child, uploader->item());
++}
++#endif
++
++TEST_F(UploadTest, write_before_ready)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("upload_slow")));
++
++    Item child;
++    {
++        unique_ptr<ItemJob> j(acc_.get("child_id"));
++        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++        spy.wait(SIGNAL_WAIT_TIME);
++        child = j->item();
++    }
++
++    QByteArray contents("Hello world", -1);
++    unique_ptr<Uploader> uploader(child.createUploader(Item::ConflictPolicy::IgnoreConflict, contents.size()));
++    EXPECT_TRUE(uploader->isValid());
++
++    // Don't wait for ready state.
++
++    EXPECT_EQ(contents.size(), uploader->write(contents));
++
++    // Wait until we get confirmation that the contents were written.
++    {
++        QSignalSpy spy(uploader.get(), &Uploader::bytesWritten);
++        ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++        auto arg = spy.takeFirst();
++        EXPECT_TRUE(qvariant_cast<bool>(arg.at(0)));
++    }
++
++    QSignalSpy spy(uploader.get(), &Uploader::statusChanged);
++    uploader->close();
      ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
      auto arg = spy.takeFirst();
      EXPECT_EQ(Uploader::Status::Finished, qvariant_cast<Uploader::Status>(arg.at(0)));
@@ -3073,14 +2988,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3098,20 +3005,15 @@
          auto arg = spy.takeFirst();
          EXPECT_EQ(Uploader::Status::Ready, qvariant_cast<Uploader::Status>(arg.at(0)));
+     }
++
++    EXPECT_EQ(1, uploader->write("a", 1));
++    EXPECT_TRUE(uploader->waitForBytesWritten(SIGNAL_WAIT_TIME));
+ }
  TEST_F(UploadTest, runtime_destroyed)
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3143,14 +3045,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("upload_slow")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3184,14 +3078,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("upload_error")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3217,10 +3103,10 @@
      EXPECT_EQ("Conflict: version mismatch", uploader->error().errorString());
      EXPECT_EQ(Item(), uploader->item());
--    // For coverage: call finishUpload() while in the Error state.
++    // For coverage: call close() while in the Error state.
+     {
          QSignalSpy spy(uploader.get(), &Uploader::statusChanged);
--        uploader->finishUpload();
++        uploader->close();
          EXPECT_FALSE(spy.wait(1000));
+     }
+ }
@@ -3229,14 +3115,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3250,7 +3128,7 @@
      QSignalSpy spy(uploader.get(), &Uploader::statusChanged);
--    uploader->finishUpload();
++    uploader->close();
      ASSERT_EQ(1, spy.count());
      auto arg = spy.takeFirst();
@@ -3259,7 +3137,7 @@
      EXPECT_FALSE(uploader->isValid());
      EXPECT_EQ(Uploader::Status::Error, uploader->status());
      EXPECT_EQ(StorageError::LogicError, uploader->error().type());
--    EXPECT_EQ("LogicError: Uploader::finishUpload(): cannot finalize while Uploader is not in the Ready state",
++    EXPECT_EQ("LogicError: Uploader::close(): cannot finalize while Uploader is not in the Ready state",
                uploader->error().errorString());
+ }
@@ -3267,14 +3145,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3297,7 +3167,7 @@
      EXPECT_EQ(StorageError::Type::NoError, runtime_->shutdown().type());  // Destroy runtime
--    uploader->finishUpload();
++    uploader->close();
      ASSERT_EQ(1, spy.count());
      auto arg = spy.takeFirst();
@@ -3306,21 +3176,13 @@
      EXPECT_FALSE(uploader->isValid());
      EXPECT_EQ(Uploader::Status::Error, uploader->status());
      EXPECT_EQ(StorageError::RuntimeDestroyed, uploader->error().type());
--    EXPECT_EQ("Uploader::finishUpload(): Runtime was destroyed previously", uploader->error().message());
++    EXPECT_EQ("Uploader::close(): Runtime was destroyed previously", uploader->error().message());
+ }
  TEST_F(UploadTest, finish_runtime_destroyed_while_reply_outstanding)
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("finish_upload_slow")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3341,7 +3203,7 @@
      QSignalSpy spy(uploader.get(), &Uploader::statusChanged);
--    uploader->finishUpload();
++    uploader->close();
      EXPECT_EQ(StorageError::Type::NoError, runtime_->shutdown().type());  // Destroy runtime, provider still sleeping
@@ -3352,21 +3214,13 @@
      EXPECT_FALSE(uploader->isValid());
      EXPECT_EQ(Uploader::Status::Error, uploader->status());
      EXPECT_EQ(StorageError::RuntimeDestroyed, uploader->error().type());
--    EXPECT_EQ("Uploader::finishUpload(): Runtime was destroyed previously", uploader->error().message());
++    EXPECT_EQ("Uploader::close(): Runtime was destroyed previously", uploader->error().message());
+ }
  TEST_F(UploadTest, finish_twice)
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3387,8 +3241,8 @@
      QSignalSpy spy(uploader.get(), &Uploader::statusChanged);
--    uploader->finishUpload();
--    uploader->finishUpload();
++    uploader->close();
++    uploader->close();
      ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
      auto arg = spy.takeFirst();
@@ -3401,14 +3255,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("finish_upload_error")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3429,7 +3275,7 @@
      QSignalSpy spy(uploader.get(), &Uploader::statusChanged);
--    uploader->finishUpload();
++    uploader->close();
      ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
      auto arg = spy.takeFirst();
@@ -3472,14 +3318,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3506,14 +3344,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("upload_returns_dir")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3533,7 +3363,7 @@
+     }
      QSignalSpy spy(uploader.get(), &Uploader::statusChanged);
--    uploader->finishUpload();
++    uploader->close();
      ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
      auto arg = spy.takeFirst();
      EXPECT_EQ(Uploader::Status::Error, qvariant_cast<Uploader::Status>(arg.at(0)));
@@ -3547,14 +3377,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3592,14 +3414,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("finish_upload_slow_error")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3620,12 +3434,12 @@
      QSignalSpy spy(uploader.get(), &Uploader::statusChanged);
--    uploader->finishUpload();
++    uploader->close();
      uploader->cancel();
      uploader->cancel();  // Second time for coverage
--    uploader->finishUpload();  // Second time for coverage
++    uploader->close();  // Second time for coverage
      EXPECT_EQ(1, spy.count());
      auto arg = spy.takeFirst();
@@ -3635,7 +3449,7 @@
      EXPECT_EQ(StorageError::Type::Cancelled, uploader->error().type());
      EXPECT_EQ("Uploader::cancel(): upload was cancelled", uploader->error().message());
--    // We wait here to get coverage for when the reply to a finishUpload() call
++    // We wait here to get coverage for when the reply to a FinishUpload() call
      // finds the uploader in a final state.
      EXPECT_FALSE(spy.wait(2000));
+ }
@@ -3644,14 +3458,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("finish_upload_slow_error")));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3730,7 +3536,7 @@
      EXPECT_EQ(contents.size(), uploader->write(contents));
      QSignalSpy spy(uploader.get(), &Uploader::statusChanged);
--    uploader->finishUpload();
++    uploader->close();
      ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
      auto arg = spy.takeFirst();
      EXPECT_EQ(Uploader::Status::Finished, qvariant_cast<Uploader::Status>(arg.at(0)));
@@ -3751,14 +3557,6 @@
          root = j->item();
+     }
--    Item child;
--    {
--        unique_ptr<ItemJob> j(acc_.get("child_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        child = j->item();
--    }
--
      EXPECT_EQ(StorageError::Type::NoError, runtime_->shutdown().type());  // Destroy runtime
      QByteArray contents("Hello world", -1);
@@ -3783,14 +3581,6 @@
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
--    Item root;
--    {
--        unique_ptr<ItemJob> j(acc_.get("root_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        root = j->item();
--    }
--
      Item child;
+     {
          unique_ptr<ItemJob> j(acc_.get("child_id"));
@@ -3829,14 +3619,6 @@
          root = j->item();
+     }
--    Item child;
--    {
--        unique_ptr<ItemJob> j(acc_.get("child_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        child = j->item();
--    }
--
      QByteArray contents("Hello world", -1);
      unique_ptr<Uploader> uploader(root.createFile("",
                                                     Item::ConflictPolicy::IgnoreConflict,
@@ -3867,14 +3649,6 @@
          root = j->item();
+     }
--    Item child;
--    {
--        unique_ptr<ItemJob> j(acc_.get("child_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        child = j->item();
--    }
--
      QByteArray contents("Hello world", -1);
      unique_ptr<Uploader> uploader(root.createFile("some_file",
                                                     Item::ConflictPolicy::IgnoreConflict,
@@ -3905,14 +3679,6 @@
          root = j->item();
+     }
--    Item child;
--    {
--        unique_ptr<ItemJob> j(acc_.get("child_id"));
--        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
--        spy.wait(SIGNAL_WAIT_TIME);
--        child = j->item();
--    }
--
      QByteArray contents("Hello world", -1);
      unique_ptr<Uploader> uploader(root.createFile("some_file",
                                                     Item::ConflictPolicy::IgnoreConflict,
@@ -3930,7 +3696,7 @@
      EXPECT_EQ(contents.size(), uploader->write(contents));
      QSignalSpy spy(uploader.get(), &Uploader::statusChanged);
--    uploader->finishUpload();
++    uploader->close();
      ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
      auto arg = spy.takeFirst();
      EXPECT_EQ(Uploader::Status::Error, qvariant_cast<Uploader::Status>(arg.at(0)));
@@ -3968,7 +3734,7 @@
      EXPECT_EQ(contents.size(), uploader->write(contents));
      QSignalSpy spy(uploader.get(), &Uploader::statusChanged);
--    uploader->finishUpload();
++    uploader->close();
      ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
      auto arg = spy.takeFirst();
      EXPECT_EQ(Uploader::Status::Error, qvariant_cast<Uploader::Status>(arg.at(0)));