Cache avatar sending, fix image file preview

- Add AvatarBroadcaster, in charge of making sure our friends have our avatar without spamming file transfers

- Fix file sending code not closing the file after transfer, which prevented file previews, and make the QFile a shared_ptr to fix the obvious memory leak

Some small additions to Core to support AvatarBroadcaster

qtox.pro

@@ -430,7 +430,8 @@ SOURCES += \
     src/core/coreencryption.cpp \
     src/core/corefile.cpp \
     src/core/corestructs.cpp \
-    src/profilelocker.cpp
+    src/profilelocker.cpp \
+    src/avatarbroadcaster.cpp
 
 HEADERS += \
     src/audio.h \
@@ -455,4 +456,5 @@ HEADERS += \
     src/widget/gui.h \
     src/toxme.h \
     src/misc/qrwidget.h \
-    src/profilelocker.h
+    src/profilelocker.h \
+    src/avatarbroadcaster.h

src/avatarbroadcaster.cpp

@@ -0,0 +1,41 @@
+#include "avatarbroadcaster.h"
+#include "src/core/core.h"
+#include <QObject>
+#include <QDebug>
+
+QByteArray AvatarBroadcaster::avatarData;
+QMap<uint32_t, bool> AvatarBroadcaster::friendsSentTo;
+
+static QMetaObject::Connection autoBroadcastConn;
+static auto autoBroadcast = [](uint32_t friendId, Status)
+{
+    AvatarBroadcaster::sendAvatarTo(friendId);
+};
+
+void AvatarBroadcaster::setAvatar(QByteArray data)
+{
+    avatarData = data;
+    friendsSentTo.clear();
+
+    QVector<uint32_t> friends = Core::getInstance()->getFriendList();
+    for (uint32_t friendId : friends)
+        sendAvatarTo(friendId);
+}
+
+void AvatarBroadcaster::sendAvatarTo(uint32_t friendId)
+{
+    if (friendsSentTo.contains(friendId) && friendsSentTo[friendId])
+        return;
+    if (!Core::getInstance()->isFriendOnline(friendId))
+        return;
+    qDebug() << "AvatarBroadcaster: Sending avatar to "<<friendId;
+    Core::getInstance()->sendAvatarFile(friendId, avatarData);
+    friendsSentTo[friendId] = true;
+}
+
+void AvatarBroadcaster::enableAutoBroadcast(bool state)
+{
+    QObject::disconnect(autoBroadcastConn);
+    if (state)
+        autoBroadcastConn = QObject::connect(Core::getInstance(), &Core::friendStatusChanged, autoBroadcast);
+}

src/avatarbroadcaster.h

@@ -0,0 +1,28 @@
+#ifndef AVATARBROADCASTER_H
+#define AVATARBROADCASTER_H
+
+#include <QByteArray>
+#include <QMap>
+
+/// Takes care of broadcasting avatar changes to our friends in a smart way
+/// Cache a copy of our current avatar and friends who have received it
+/// so we don't spam avatar transfers to a friend who already has it.
+class AvatarBroadcaster
+{
+private:
+    AvatarBroadcaster()=delete;
+
+public:
+    /// Set our current avatar
+    static void setAvatar(QByteArray data);
+    /// Send our current avatar to this friend, if not already sent
+    static void sendAvatarTo(uint32_t friendId);
+    /// If true, we automatically broadcast our avatar to friends when they come online
+    static void enableAutoBroadcast(bool state = true);
+
+private:
+    static QByteArray avatarData;
+    static QMap<uint32_t, bool> friendsSentTo;
+};
+
+#endif // AVATARBROADCASTER_H

src/core/core.cpp

@@ -23,6 +23,7 @@
 #include "src/historykeeper.h"
 #include "src/audio.h"
 #include "src/profilelocker.h"
+#include "src/avatarbroadcaster.h"
 #include "corefile.h"
 
 #include <tox/tox.h>
@@ -479,22 +480,10 @@ void Core::onUserStatusChanged(Tox*/* tox*/, uint32_t friendId, TOX_USER_STATUS
 
 void Core::onConnectionStatusChanged(Tox*/* tox*/, uint32_t friendId, TOX_CONNECTION status, void* core)
 {
-    Status friendStatus = status ? Status::Online : Status::Offline;
+    Status friendStatus = status != TOX_CONNECTION_NONE ? Status::Online : Status::Offline;
     emit static_cast<Core*>(core)->friendStatusChanged(friendId, friendStatus);
     if (friendStatus == Status::Offline)
-    {
         static_cast<Core*>(core)->checkLastOnline(friendId);
-    }
-    else
-    {
-        QPixmap pic = Settings::getInstance().getSavedAvatar(static_cast<Core*>(core)->getSelfId().toString());
-        QByteArray bytes;
-        QBuffer buffer(&bytes);
-        buffer.open(QIODevice::WriteOnly);
-        pic.save(&buffer, "PNG");
-        buffer.close();
-        CoreFile::sendAvatarFile(static_cast<Core*>(core), friendId, bytes);
-    }
 }
 
 void Core::onGroupAction(Tox*, int groupnumber, int peernumber, const uint8_t *action, uint16_t length, void* _core)
@@ -664,6 +653,11 @@ void Core::sendFile(uint32_t friendId, QString Filename, QString FilePath, long
     CoreFile::sendFile(this, friendId, Filename, FilePath, filesize);
 }
 
+void Core::sendAvatarFile(uint32_t friendId, const QByteArray& data)
+{
+    CoreFile::sendAvatarFile(this, friendId, data);
+}
+
 void Core::pauseResumeFileSend(uint32_t friendId, uint32_t fileNum)
 {
     CoreFile::pauseResumeFileSend(this, friendId, fileNum);
@@ -754,11 +748,8 @@ void Core::setAvatar(const QByteArray& data)
     Settings::getInstance().saveAvatar(pic, getSelfId().toString());
     emit selfAvatarChanged(pic);
     
-    // Broadcast our new avatar!
-    // according to tox.h, we need not broadcast this ourselves, but initial testing indicated elsewise
-    const uint32_t friendCount = tox_self_get_friend_list_size(tox);
-    for (unsigned i=0; i<friendCount; i++)
-        CoreFile::sendAvatarFile(this, i, data);
+    AvatarBroadcaster::setAvatar(data);
+    AvatarBroadcaster::enableAutoBroadcast();
 }
 
 ToxID Core::getSelfId() const
@@ -1044,6 +1035,14 @@ void Core::checkLastOnline(uint32_t friendId) {
         emit friendLastSeenChanged(friendId, QDateTime::fromTime_t(lastOnline));
 }
 
+QVector<uint32_t> Core::getFriendList() const
+{
+    QVector<uint32_t> friends;
+    friends.resize(tox_self_get_friend_list_size(tox));
+    tox_self_get_friend_list(tox, friends.data());
+    return friends;
+}
+
 int Core::getGroupNumberPeers(int groupId) const
 {
     return tox_group_number_peers(tox, groupId);
@@ -1153,6 +1152,11 @@ bool Core::isGroupAvEnabled(int groupId)
     return tox_group_get_type(tox, groupId) == TOX_GROUPCHAT_TYPE_AV;
 }
 
+bool Core::isFriendOnline(uint32_t friendId) const
+{
+    return tox_friend_get_connection_status(tox, friendId, nullptr) != TOX_CONNECTION_NONE;
+}
+
 bool Core::hasFriendWithAddress(const QString &addr) const
 {
     // Valid length check

src/core/core.h

@@ -57,15 +57,17 @@ public:
 
     QString getPeerName(const ToxID& id) const;
 
+    QVector<uint32_t> getFriendList() const; ///< Returns the list of friendIds in our friendlist, an empty list on error
     int getGroupNumberPeers(int groupId) const; ///< Return the number of peers in the group chat on success, or -1 on failure
     QString getGroupPeerName(int groupId, int peerId) const; ///< Get the name of a peer of a group
     ToxID getGroupPeerToxID(int groupId, int peerId) const; ///< Get the ToxID of a peer of a group
     QList<QString> getGroupPeerNames(int groupId) const; ///< Get the names of the peers of a group
-    QString getFriendAddress(uint32_t friendNumber) const; ///< Get the full address if known, or Tox ID of a friend
-    QString getFriendUsername(uint32_t friendNumber) const; ///< Get the username of a friend
+    QString getFriendAddress(uint32_t friendId) const; ///< Get the full address if known, or Tox ID of a friend
+    QString getFriendUsername(uint32_t friendId) const; ///< Get the username of a friend
+    bool isFriendOnline(uint32_t friendId) const; ///< Check if a friend is online. Unknown friends are considered offline.
     bool hasFriendWithAddress(const QString &addr) const; ///< Check if we have a friend by address
     bool hasFriendWithPublicKey(const QString &pubkey) const; ///< Check if we have a friend by public key
-    int joinGroupchat(int32_t friendNumber, uint8_t type, const uint8_t* pubkey,uint16_t length) const; ///< Accept a groupchat invite
+    int joinGroupchat(int32_t friendId, uint8_t type, const uint8_t* pubkey,uint16_t length) const; ///< Accept a groupchat invite
     void quitGroupChat(int groupId) const; ///< Quit a groupchat
 
     QString getIDString() const; ///< Get the 12 first characters of our Tox ID
@@ -113,6 +115,7 @@ public slots:
     void sendTyping(uint32_t friendId, bool typing);
 
     void sendFile(uint32_t friendId, QString Filename, QString FilePath, long long filesize);
+    void sendAvatarFile(uint32_t friendId, const QByteArray& data);
     void cancelFileSend(uint32_t friendId, uint32_t fileNum);
     void cancelFileRecv(uint32_t friendId, uint32_t fileNum);
     void rejectFileRecvRequest(uint32_t friendId, uint32_t fileNum);
@@ -170,7 +173,7 @@ signals:
     void friendLastSeenChanged(uint32_t friendId, const QDateTime& dateTime);
 
     void emptyGroupCreated(int groupnumber);
-    void groupInviteReceived(uint32_t friendNumber, uint8_t type, QByteArray publicKey);
+    void groupInviteReceived(uint32_t friendId, uint8_t type, QByteArray publicKey);
     void groupMessageReceived(int groupnumber, int peernumber, const QString& message, bool isAction);
     void groupNamelistChanged(int groupnumber, int peernumber, uint8_t change);
     void groupTitleChanged(int groupnumber, const QString& author, const QString& title);
@@ -241,14 +244,14 @@ private:
     static void onConnectionStatusChanged(Tox* tox, uint32_t friendId, TOX_CONNECTION status, void* core);
     static void onGroupAction(Tox* tox, int groupnumber, int peernumber, const uint8_t * action,
                               uint16_t length, void* core);
-    static void onGroupInvite(Tox *tox, int32_t friendNumber, uint8_t type, const uint8_t *data,
+    static void onGroupInvite(Tox *tox, int32_t friendId, uint8_t type, const uint8_t *data,
                               uint16_t length, void *userdata);
     static void onGroupMessage(Tox *tox, int groupnumber, int friendgroupnumber,
                                const uint8_t * message, uint16_t length, void *userdata);
     static void onGroupNamelistChange(Tox *tox, int groupId, int peerId, uint8_t change, void *core);
     static void onGroupTitleChange(Tox*, int groupnumber, int peernumber,
                                    const uint8_t* title, uint8_t len, void* _core);
-    static void onReadReceiptCallback(Tox *tox, uint32_t friendnumber, uint32_t receipt, void *core);
+    static void onReadReceiptCallback(Tox *tox, uint32_t friendId, uint32_t receipt, void *core);
 
     static void onAvInvite(void* toxav, int32_t call_index, void* core);
     static void onAvStart(void* toxav, int32_t call_index, void* core);

src/core/corefile.cpp

@@ -178,7 +178,7 @@ ToxFile* CoreFile::findFile(uint32_t friendId, uint32_t fileId)
     uint64_t key = ((uint64_t)friendId<<32) + (uint64_t)fileId;
     if (!fileMap.contains(key))
     {
-        qWarning() << "CoreFile::addFile: File transfer with ID "<<friendId<<':'<<fileId<<" doesn't exist";
+        qWarning() << "CoreFile::findFile: File transfer with ID "<<friendId<<':'<<fileId<<" doesn't exist";
         return nullptr;
     }
     else
@@ -196,8 +196,13 @@ void CoreFile::addFile(uint32_t friendId, uint32_t fileId, const ToxFile& file)
 void CoreFile::removeFile(uint32_t friendId, uint32_t fileId)
 {
     uint64_t key = ((uint64_t)friendId<<32) + (uint64_t)fileId;
-    if (!fileMap.remove(key))
+    if (!fileMap.contains(key))
+    {
         qWarning() << "CoreFile::removeFile: No such file in queue";
+        return;
+    }
+    fileMap[key].file->close();
+    fileMap.remove(key);
 }
 
 void CoreFile::onFileReceiveCallback(Tox*, uint32_t friendId, uint32_t fileId, uint32_t kind,
@@ -331,7 +336,8 @@ void CoreFile::onFileDataCallback(Tox *tox, uint32_t friendId, uint32_t fileId,
 void CoreFile::onFileRecvChunkCallback(Tox *tox, uint32_t friendId, uint32_t fileId, uint64_t position,
                                     const uint8_t *data, size_t length, void *core)
 {
-    //qDebug() << QString("CoreFile: Received file chunk for request %1:%2").arg(friendId).arg(fileId);
+    qDebug() << QString("CoreFile: Received chunk for %1:%2 pos %3 size %4")
+                        .arg(friendId).arg(fileId).arg(position).arg(length);
 
     ToxFile* file = findFile(friendId, fileId);
     if (!file)
@@ -352,13 +358,7 @@ void CoreFile::onFileRecvChunkCallback(Tox *tox, uint32_t friendId, uint32_t fil
         return;
     }
 
-    if (file->fileKind == TOX_FILE_KIND_AVATAR)
-        file->avatarData.append((char*)data, length);
-    else
-        file->file->write((char*)data,length);
-    file->bytesSent += length;
-
-    if (file->bytesSent == file->filesize)
+    if (!length)
     {
         if (file->fileKind == TOX_FILE_KIND_AVATAR)
         {
@@ -376,9 +376,16 @@ void CoreFile::onFileRecvChunkCallback(Tox *tox, uint32_t friendId, uint32_t fil
         {
             emit static_cast<Core*>(core)->fileTransferFinished(*file);
         }
+        removeFile(friendId, fileId);
+        return;
     }
-    else if (file->fileKind != TOX_FILE_KIND_AVATAR)
-    {
+
+    if (file->fileKind == TOX_FILE_KIND_AVATAR)
+        file->avatarData.append((char*)data, length);
+    else
+        file->file->write((char*)data,length);
+    file->bytesSent += length;
+
+    if (file->fileKind != TOX_FILE_KIND_AVATAR)
         emit static_cast<Core*>(core)->fileTransferInfo(*file);
-    }
 }

src/core/corestructs.h

@@ -5,6 +5,7 @@
 // They should include this file directly instead to reduce compilation times
 
 #include <QString>
+#include <memory>
 class QFile;
 class QTimer;
 
@@ -72,7 +73,7 @@ struct ToxFile
     uint32_t friendId;
     QByteArray fileName;
     QString filePath;
-    QFile* file;
+    std::shared_ptr<QFile> file;
     quint64 bytesSent;
     quint64 filesize;
     FileStatus status;