Load icons asynchronously in History and Bookmarks models

See #1679

src/lib/bookmarks/bookmarkitem.cpp

@@ -70,19 +70,12 @@ QList<BookmarkItem*> BookmarkItem::children() const
     return m_children;
 }
 
-QIcon BookmarkItem::icon()
+QIcon BookmarkItem::icon(bool load)
 {
-    // Cache icon for 20 seconds
-    const int iconCacheTime = 20 * 1000;
-
     switch (m_type) {
     case Url:
-        if (m_iconTime.isNull() || m_iconTime.elapsed() > iconCacheTime) {
-            m_icon = IconProvider::iconForUrl(m_url, true);
-            if (m_icon.isNull()) {
-                m_icon = IconProvider::emptyWebIcon();
-                m_iconTime.restart();
-            }
+        if (load && m_icon.isNull()) {
+            setIcon(IconProvider::iconForUrl(m_url));
         }
         return m_icon;
     case Folder:
@@ -92,6 +85,11 @@ QIcon BookmarkItem::icon()
     }
 }
 
+void BookmarkItem::setIcon(const QIcon &icon)
+{
+    m_icon = icon;
+}
+
 QString BookmarkItem::urlString() const
 {
     return QString::fromUtf8(m_url.toEncoded());

src/lib/bookmarks/bookmarkitem.h

@@ -50,7 +50,9 @@ public:
     BookmarkItem* parent() const;
     QList<BookmarkItem*> children() const;
 
-    QIcon icon();
+    QIcon icon(bool load = true);
+    void setIcon(const QIcon &icon);
+
     QString urlString() const;
 
     QUrl url() const;

src/lib/bookmarks/bookmarksmodel.cpp

@@ -131,7 +131,7 @@ QVariant BookmarksModel::data(const QModelIndex &index, int role) const
         }
     case Qt::DecorationRole:
         if (index.column() == 0) {
-            return itm->icon();
+            return itm->icon(false);
         }
         return QVariant();
     default:
@@ -139,6 +139,23 @@ QVariant BookmarksModel::data(const QModelIndex &index, int role) const
     }
 }
 
+bool BookmarksModel::setData(const QModelIndex &index, const QVariant &value, int role)
+{
+    BookmarkItem *itm = item(index);
+
+    if (!itm) {
+        return false;
+    }
+
+    if (role == IconRole) {
+        itm->setIcon(value.value<QIcon>());
+        emit dataChanged(index, index);
+        return true;
+    }
+
+    return false;
+}
+
 QVariant BookmarksModel::headerData(int section, Qt::Orientation orientation, int role) const
 {
     if (orientation == Qt::Horizontal && role == Qt::DisplayRole) {

src/lib/bookmarks/bookmarksmodel.h

@@ -38,11 +38,12 @@ public:
         UrlRole = Qt::UserRole + 2,
         UrlStringRole = Qt::UserRole + 3,
         TitleRole = Qt::UserRole + 4,
-        DescriptionRole = Qt::UserRole + 5,
-        KeywordRole = Qt::UserRole + 6,
-        VisitCountRole = Qt::UserRole + 7,
-        ExpandedRole = Qt::UserRole + 8,
-        SidebarExpandedRole = Qt::UserRole + 9,
+        IconRole = Qt::UserRole + 5,
+        DescriptionRole = Qt::UserRole + 6,
+        KeywordRole = Qt::UserRole + 7,
+        VisitCountRole = Qt::UserRole + 8,
+        ExpandedRole = Qt::UserRole + 9,
+        SidebarExpandedRole = Qt::UserRole + 10,
         MaxRole = SidebarExpandedRole
     };
 
@@ -53,6 +54,7 @@ public:
 
     Qt::ItemFlags flags(const QModelIndex &index) const;
     QVariant data(const QModelIndex &index, int role) const;
+    bool setData(const QModelIndex &index, const QVariant &value, int role);
     QVariant headerData(int section, Qt::Orientation orientation, int role) const;
     int rowCount(const QModelIndex &parent) const;
     int columnCount(const QModelIndex &parent) const;

src/lib/bookmarks/bookmarkstools.cpp

@@ -390,7 +390,7 @@ void BookmarksTools::addUrlToMenu(QObject* receiver, Menu* menu, BookmarkItem* b
     Action* act = new Action(menu);
     QString title = QFontMetrics(act->font()).elidedText(bookmark->title(), Qt::ElideRight, 250);
     act->setText(title);
-
+    act->setIcon(bookmark->icon(false));
     act->setData(QVariant::fromValue<void*>(static_cast<void*>(bookmark)));
     act->setIconVisibleInMenu(true);
 

src/lib/bookmarks/bookmarkstreeview.cpp

@@ -21,6 +21,7 @@
 #include "bookmarkitem.h"
 #include "bookmarks.h"
 #include "mainapplication.h"
+#include "iconprovider.h"
 
 #include <QHeaderView>
 #include <QMouseEvent>
@@ -288,3 +289,18 @@ void BookmarksTreeView::keyPressEvent(QKeyEvent* event)
         }
     }
 }
+
+void BookmarksTreeView::drawRow(QPainter *painter, const QStyleOptionViewItem &options, const QModelIndex &index) const
+{
+    bool itemIsUrl = BookmarkItem::Type(index.data(BookmarksModel::TypeRole).toInt()) == BookmarkItem::Url;
+    bool iconLoaded = !index.data(BookmarksModel::IconRole).value<QIcon>().isNull();
+
+    if (itemIsUrl && !iconLoaded) {
+        const QPersistentModelIndex idx = index;
+        IconProvider::imageForUrlAsync(index.data(BookmarksModel::UrlRole).toUrl(), this, [=](const QImage &img) {
+            model()->setData(idx, QIcon(QPixmap::fromImage(img)), BookmarksModel::IconRole);
+        });
+    }
+
+    QTreeView::drawRow(painter, options, index);
+}

src/lib/bookmarks/bookmarkstreeview.h

@@ -82,6 +82,8 @@ private:
     void mouseDoubleClickEvent(QMouseEvent* event);
     void keyPressEvent(QKeyEvent* event);
 
+    void drawRow(QPainter* painter, const QStyleOptionViewItem &options, const QModelIndex &index) const;
+
     Bookmarks* m_bookmarks;
     BookmarksModel* m_model;
     BookmarksFilterModel* m_filter;

src/lib/history/historyitem.cpp

@@ -21,7 +21,6 @@
 HistoryItem::HistoryItem(HistoryItem* parent)
     : canFetchMore(false)
     , m_parent(parent)
-    , m_iconLoaded(false)
     , m_startTimestamp(0)
     , m_endTimestamp(0)
 {
@@ -121,11 +120,6 @@ bool HistoryItem::isTopLevel() const
     return (m_startTimestamp != 0);
 }
 
-bool HistoryItem::iconLoaded() const
-{
-    return m_iconLoaded;
-}
-
 QIcon HistoryItem::icon() const
 {
     return m_icon;
@@ -134,12 +128,6 @@ QIcon HistoryItem::icon() const
 void HistoryItem::setIcon(const QIcon &icon)
 {
     m_icon = icon;
-    m_iconLoaded = true;
-}
-
-void HistoryItem::refreshIcon()
-{
-    m_iconLoaded = false;
 }
 
 void HistoryItem::setStartTimestamp(qint64 start)

src/lib/history/historyitem.h

@@ -46,11 +46,9 @@ public:
     int indexOfChild(HistoryItem* child);
 
     bool isTopLevel() const;
-    bool iconLoaded() const;
 
     QIcon icon() const;
     void setIcon(const QIcon &icon);
-    void refreshIcon();
 
     void setStartTimestamp(qint64 start);
     qint64 startTimestamp() const;
@@ -67,7 +65,6 @@ private:
     QList<HistoryItem*> m_children;
 
     QIcon m_icon;
-    bool m_iconLoaded;
 
     qint64 m_startTimestamp;
     qint64 m_endTimestamp;

src/lib/history/historymodel.cpp

@@ -105,8 +105,6 @@ QVariant HistoryModel::data(const QModelIndex &index, int role) const
         return entry.urlString;
     case IconRole:
         return item->icon();
-    case IconLoadedRole:
-        return item->iconLoaded();
     case IsTopLevelRole:
         return false;
     case TimestampStartRole:

src/lib/history/historymodel.h

@@ -39,7 +39,6 @@ public:
         UrlRole = Qt::UserRole + 3,
         UrlStringRole = Qt::UserRole + 4,
         IconRole = Qt::UserRole + 5,
-        IconLoadedRole = Qt::UserRole + 6,
         IsTopLevelRole = Qt::UserRole + 7,
         TimestampStartRole = Qt::UserRole + 8,
         TimestampEndRole = Qt::UserRole + 9,

src/lib/history/historytreeview.cpp

@@ -255,11 +255,13 @@ void HistoryTreeView::keyPressEvent(QKeyEvent* event)
 void HistoryTreeView::drawRow(QPainter* painter, const QStyleOptionViewItem &options, const QModelIndex &index) const
 {
     bool itemTopLevel = index.data(HistoryModel::IsTopLevelRole).toBool();
-    bool iconLoaded = index.data(HistoryModel::IconLoadedRole).toBool();
+    bool iconLoaded = !index.data(HistoryModel::IconRole).value<QIcon>().isNull();
 
     if (index.isValid() && !itemTopLevel && !iconLoaded) {
-        const QIcon icon = IconProvider::iconForUrl(index.data(HistoryModel::UrlRole).toUrl());
-        model()->setData(index, icon, HistoryModel::IconRole);
+        const QPersistentModelIndex idx = index;
+        IconProvider::imageForUrlAsync(index.data(HistoryModel::UrlRole).toUrl(), this, [=](const QImage &img) {
+            model()->setData(idx, QIcon(QPixmap::fromImage(img)), HistoryModel::IconRole);
+        });
     }
 
     QTreeView::drawRow(painter, options, index);

src/lib/tools/iconprovider.cpp

@@ -190,7 +190,7 @@ QImage IconProvider::imageForUrl(const QUrl &url, bool allowEmpty)
     return allowEmpty ? QImage() : IconProvider::emptyWebImage();
 }
 
-void IconProvider::imageForUrlAsync(const QUrl &url, QObject *receiver, std::function<void(const QImage &)> callback)
+void IconProvider::imageForUrlAsync(const QUrl &url, const QObject *receiver, std::function<void(const QImage &)> callback)
 {
     QFutureWatcher<QImage> *watcher = new QFutureWatcher<QImage>();
     connect(watcher, &QFutureWatcher<QImage>::finished, receiver, [=]() {

src/lib/tools/iconprovider.h

@@ -61,7 +61,7 @@ public:
     // Icon for url (only available for urls in history)
     static QIcon iconForUrl(const QUrl &url, bool allowEmpty = false);
     static QImage imageForUrl(const QUrl &url, bool allowEmpty = false);
-    static void imageForUrlAsync(const QUrl &url, QObject *receiver, std::function<void(const QImage&)> callback);
+    static void imageForUrlAsync(const QUrl &url, const QObject *receiver, std::function<void(const QImage&)> callback);
 
     // Icon for domain (only available for urls in history)
     static QIcon iconForDomain(const QUrl &url, bool allowEmpty = false);