Account/Credentials: Have identical lifetimes

The QNAM may continue to outlive both.

Rename Credentials::getQNAM() to createQNAM() while we're at it - it's
used to make a new QNAM that will subsequently be owned by the Account
object.

See d01065b9a12e69ca493a232f3a8e8f3d416fed52 for rationale.

Relates to
d40c56eda561e3a541bf1b23f70fa8d659d3037e
147cf798a6f13c9b53a9f1fb2db1ef26c8c63273

10 files changed, 20 insertions, 16 deletions

diff --git a/src/gui/creds/shibbolethcredentials.cpp b/src/gui/creds/shibbolethcredentials.cpp
index cc3d6fb68..b7fca6dd3 100644
--- a/src/gui/creds/shibbolethcredentials.cpp
+++ b/src/gui/creds/shibbolethcredentials.cpp
@@ -90,7 +90,7 @@ QString ShibbolethCredentials::user() const
     return _user;
 }
 
-QNetworkAccessManager *ShibbolethCredentials::getQNAM() const
+QNetworkAccessManager *ShibbolethCredentials::createQNAM() const
 {
     QNetworkAccessManager *qnam(new AccessManager);
     connect(qnam, SIGNAL(finished(QNetworkReply *)),
diff --git a/src/gui/creds/shibbolethcredentials.h b/src/gui/creds/shibbolethcredentials.h
index 46b6bc08d..a4909ed74 100644
--- a/src/gui/creds/shibbolethcredentials.h
+++ b/src/gui/creds/shibbolethcredentials.h
@@ -53,7 +53,7 @@ public:
     void setAccount(Account *account) Q_DECL_OVERRIDE;
     QString authType() const Q_DECL_OVERRIDE;
     QString user() const Q_DECL_OVERRIDE;
-    QNetworkAccessManager *getQNAM() const Q_DECL_OVERRIDE;
+    QNetworkAccessManager *createQNAM() const Q_DECL_OVERRIDE;
     bool ready() const Q_DECL_OVERRIDE;
     void fetchFromKeychain() Q_DECL_OVERRIDE;
     void askFromUser() Q_DECL_OVERRIDE;
diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp
index ee83e3dee..45168033f 100644
--- a/src/libsync/account.cpp
+++ b/src/libsync/account.cpp
@@ -136,11 +136,13 @@ void Account::setCredentials(AbstractCredentials *cred)
 
     // The order for these two is important! Reading the credential's
     // settings accesses the account as well as account->_credentials,
-    // so deleteLater must be used.
-    _credentials = QSharedPointer<AbstractCredentials>(cred, &QObject::deleteLater);
+    _credentials.reset(cred);
     cred->setAccount(this);
 
-    _am = QSharedPointer<QNetworkAccessManager>(_credentials->getQNAM(), &QObject::deleteLater);
+    // Note: This way the QNAM can outlive the Account and Credentials.
+    // This is necessary to avoid issues with the QNAM being deleted while
+    // processing slotHandleSslErrors().
+    _am = QSharedPointer<QNetworkAccessManager>(_credentials->createQNAM(), &QObject::deleteLater);
 
     if (jar) {
         _am->setCookieJar(jar);
@@ -205,7 +207,7 @@ void Account::resetNetworkAccessManager()
 
     // Use a QSharedPointer to allow locking the life of the QNAM on the stack.
     // Make it call deleteLater to make sure that we can return to any QNAM stack frames safely.
-    _am = QSharedPointer<QNetworkAccessManager>(_credentials->getQNAM(), &QObject::deleteLater);
+    _am = QSharedPointer<QNetworkAccessManager>(_credentials->createQNAM(), &QObject::deleteLater);
 
     _am->setCookieJar(jar); // takes ownership of the old cookie jar
     connect(_am.data(), SIGNAL(sslErrors(QNetworkReply *, QList<QSslError>)),
diff --git a/src/libsync/account.h b/src/libsync/account.h
index d48b27b15..b34342d45 100644
--- a/src/libsync/account.h
+++ b/src/libsync/account.h
@@ -246,7 +246,7 @@ private:
     QScopedPointer<AbstractSslErrorHandler> _sslErrorHandler;
     QuotaInfo *_quotaInfo;
     QSharedPointer<QNetworkAccessManager> _am;
-    QSharedPointer<AbstractCredentials> _credentials;
+    QScopedPointer<AbstractCredentials> _credentials;
 
     /// Certificates that were explicitly rejected by the user
     QList<QSslCertificate> _rejectedCertificates;
diff --git a/src/libsync/creds/abstractcredentials.h b/src/libsync/creds/abstractcredentials.h
index 09ba50e3e..9958b5666 100644
--- a/src/libsync/creds/abstractcredentials.h
+++ b/src/libsync/creds/abstractcredentials.h
@@ -43,7 +43,7 @@ public:
 
     virtual QString authType() const = 0;
     virtual QString user() const = 0;
-    virtual QNetworkAccessManager *getQNAM() const = 0;
+    virtual QNetworkAccessManager *createQNAM() const = 0;
 
     /** Whether there are credentials that can be used for a connection attempt. */
     virtual bool ready() const = 0;
diff --git a/src/libsync/creds/dummycredentials.cpp b/src/libsync/creds/dummycredentials.cpp
index c55e979ac..3c5609d35 100644
--- a/src/libsync/creds/dummycredentials.cpp
+++ b/src/libsync/creds/dummycredentials.cpp
@@ -27,7 +27,7 @@ QString DummyCredentials::user() const
     return _user;
 }
 
-QNetworkAccessManager *DummyCredentials::getQNAM() const
+QNetworkAccessManager *DummyCredentials::createQNAM() const
 {
     return new AccessManager;
 }
diff --git a/src/libsync/creds/dummycredentials.h b/src/libsync/creds/dummycredentials.h
index ad56a08dd..6729b2e5a 100644
--- a/src/libsync/creds/dummycredentials.h
+++ b/src/libsync/creds/dummycredentials.h
@@ -28,7 +28,7 @@ public:
     QString _password;
     QString authType() const Q_DECL_OVERRIDE;
     QString user() const Q_DECL_OVERRIDE;
-    QNetworkAccessManager *getQNAM() const Q_DECL_OVERRIDE;
+    QNetworkAccessManager *createQNAM() const Q_DECL_OVERRIDE;
     bool ready() const Q_DECL_OVERRIDE;
     bool stillValid(QNetworkReply *reply) Q_DECL_OVERRIDE;
     void fetchFromKeychain() Q_DECL_OVERRIDE;
diff --git a/src/libsync/creds/httpcredentials.cpp b/src/libsync/creds/httpcredentials.cpp
index 1670e0101..bd1cc7aef 100644
--- a/src/libsync/creds/httpcredentials.cpp
+++ b/src/libsync/creds/httpcredentials.cpp
@@ -58,7 +58,7 @@ protected:
     QNetworkReply *createRequest(Operation op, const QNetworkRequest &request, QIODevice *outgoingData) Q_DECL_OVERRIDE
     {
         QNetworkRequest req(request);
-        if (!_cred->password().isEmpty()) {
+        if (_cred && !_cred->password().isEmpty()) {
             if (_cred->isUsingOAuth()) {
                 req.setRawHeader("Authorization", "Bearer " + _cred->password().toUtf8());
             } else {
@@ -72,7 +72,7 @@ protected:
             req.setRawHeader("Authorization", "Basic " + credHash);
         }
 
-        if (!_cred->_clientSslKey.isNull() && !_cred->_clientSslCertificate.isNull()) {
+        if (_cred && !_cred->_clientSslKey.isNull() && !_cred->_clientSslCertificate.isNull()) {
             // SSL configuration
             QSslConfiguration sslConfiguration = req.sslConfiguration();
             sslConfiguration.setLocalCertificate(_cred->_clientSslCertificate);
@@ -85,7 +85,9 @@ protected:
     }
 
 private:
-    const HttpCredentials *_cred;
+    // The credentials object dies along with the account, while the QNAM might
+    // outlive both.
+    QPointer<const HttpCredentials> _cred;
 };
 
 
@@ -135,7 +137,7 @@ void HttpCredentials::setAccount(Account *account)
     }
 }
 
-QNetworkAccessManager *HttpCredentials::getQNAM() const
+QNetworkAccessManager *HttpCredentials::createQNAM() const
 {
     AccessManager *qnam = new HttpCredentialsAccessManager(this);
 
diff --git a/src/libsync/creds/httpcredentials.h b/src/libsync/creds/httpcredentials.h
index 03f3b8c70..85e6d092a 100644
--- a/src/libsync/creds/httpcredentials.h
+++ b/src/libsync/creds/httpcredentials.h
@@ -79,7 +79,7 @@ public:
     HttpCredentials(const QString &user, const QString &password, const QSslCertificate &certificate = QSslCertificate(), const QSslKey &key = QSslKey());
 
     QString authType() const Q_DECL_OVERRIDE;
-    QNetworkAccessManager *getQNAM() const Q_DECL_OVERRIDE;
+    QNetworkAccessManager *createQNAM() const Q_DECL_OVERRIDE;
     bool ready() const Q_DECL_OVERRIDE;
     void fetchFromKeychain() Q_DECL_OVERRIDE;
     bool stillValid(QNetworkReply *reply) Q_DECL_OVERRIDE;
diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h
index 997e5054e..b90ff480b 100644
--- a/test/syncenginetestutils.h
+++ b/test/syncenginetestutils.h
@@ -778,7 +778,7 @@ public:
     FakeCredentials(QNetworkAccessManager *qnam) : _qnam{qnam} { }
     virtual QString authType() const { return "test"; }
     virtual QString user() const { return "admin"; }
-    virtual QNetworkAccessManager* getQNAM() const { return _qnam; }
+    virtual QNetworkAccessManager *createQNAM() const { return _qnam; }
     virtual bool ready() const { return true; }
     virtual void fetchFromKeychain() { }
     virtual void askFromUser() { }