refactor: only update favicon when tracker changes (#1336)

* refactor: make Favicon's Key a semi-opaque type and update a Torrent's icon only when the announce_url changes.

qt/FaviconCache.cc

@@ -74,14 +74,14 @@ void FaviconCache::ensureCacheDirHasBeenScanned()
 ****
 ***/
 
-QString FaviconCache::getDisplayName(QString const& key)
+QString FaviconCache::getDisplayName(Key const& key)
 {
     auto name = key;
     name[0] = name.at(0).toTitleCase();
     return name;
 }
 
-QString FaviconCache::getKey(QUrl const& url)
+FaviconCache::Key FaviconCache::getKey(QUrl const& url)
 {
     auto host = url.host();
 
@@ -94,7 +94,7 @@ QString FaviconCache::getKey(QUrl const& url)
     return pos < 0 ? host : host.remove(0, pos + 1);
 }
 
-QString FaviconCache::getKey(QString const& displayName)
+FaviconCache::Key FaviconCache::getKey(QString const& displayName)
 {
     return displayName.toLower();
 }
@@ -104,18 +104,18 @@ QSize FaviconCache::getIconSize()
     return QSize(16, 16);
 }
 
-QPixmap FaviconCache::find(QString const& key)
+QPixmap FaviconCache::find(Key const& key)
 {
     ensureCacheDirHasBeenScanned();
 
     return pixmaps_[key];
 }
 
-QString FaviconCache::add(QUrl const& url)
+FaviconCache::Key FaviconCache::add(QUrl const& url)
 {
     ensureCacheDirHasBeenScanned();
 
-    QString const key = getKey(url);
+    Key const key = getKey(url);
 
     if (pixmaps_.count(key) == 0)
     {

qt/FaviconCache.h

@@ -9,6 +9,7 @@
 #pragma once
 
 #include <unordered_map>
+#include <unordered_set>
 
 #include <QString>
 #include <QObject>
@@ -27,30 +28,32 @@ class FaviconCache : public QObject
 public:
     FaviconCache();
 
+    using Key = QString;
+    using Keys = std::unordered_set<Key>;
+
     // returns a cached pixmap, or a NULL pixmap if there's no match in the cache
-    QPixmap find(QString const& key);
+    QPixmap find(Key const& key);
     QPixmap find(QUrl const& url) { return find(getKey(url)); }
 
-    // This will emit a signal when (if) the icon becomes ready.
+    static Key getKey(QString const& display_name);
-    // Returns the key.
-    QString add(QUrl const& url);
 
-    static QString getDisplayName(QString const& key);
+    // This will emit a signal when (if) the icon becomes ready.
-    static QString getKey(QUrl const& url);
+    Key add(QUrl const& url);
-    static QString getKey(QString const& display_name);
+
+    static QString getDisplayName(Key const& key);
     static QSize getIconSize();
 
 signals:
-    void pixmapReady(QString const& key);
+    void pixmapReady(Key const& key);
-
-private:
-    QString getCacheDir();
-    void ensureCacheDirHasBeenScanned();
 
 private slots:
     void onRequestFinished(QNetworkReply* reply);
 
 private:
+    static Key getKey(QUrl const& url);
+    QString getCacheDir();
+    void ensureCacheDirHasBeenScanned();
+
     QNetworkAccessManager* nam_ = {};
-    std::unordered_map<QString, QPixmap> pixmaps_;
+    std::unordered_map<Key, QPixmap> pixmaps_;
 };

qt/FilterBar.cc

@@ -111,35 +111,38 @@ void FilterBar::refreshTrackers()
         ROW_TOTALS = 0, ROW_SEPARATOR, ROW_FIRST_TRACKER
     };
 
-    auto torrents_per_host = std::unordered_map<QString, int>{};
+    auto torrents_per_tracker = std::unordered_map<FaviconCache::Key, int>{};
     for (auto const& tor : torrents_.torrents())
     {
-        for (auto const& display_name : tor->trackerDisplayNames())
+        for (auto const& key : tor->trackerKeys())
         {
-            ++torrents_per_host[display_name];
+            ++torrents_per_tracker[key];
         }
     }
 
     // update the "All" row
-    auto const num_trackers = torrents_per_host.size();
+    auto const num_trackers = torrents_per_tracker.size();
     auto* item = tracker_model_->item(ROW_TOTALS);
     item->setData(int(num_trackers), FilterBarComboBox::CountRole);
     item->setData(getCountString(num_trackers), FilterBarComboBox::CountStringRole);
 
     auto update_tracker_item = [](QStandardItem* i, auto const& it)
         {
-            auto const& display_name = it->first;
+            auto const& key = it->first;
+            auto const& display_name = FaviconCache::getDisplayName(key);
             auto const& count = it->second;
-            auto const icon = qApp->faviconCache().find(FaviconCache::getKey(display_name));
+            auto const icon = qApp->faviconCache().find(key);
+
             i->setData(display_name, Qt::DisplayRole);
             i->setData(display_name, TRACKER_ROLE);
             i->setData(getCountString(count), FilterBarComboBox::CountStringRole);
             i->setData(icon, Qt::DecorationRole);
             i->setData(int(count), FilterBarComboBox::CountRole);
+
             return i;
         };
 
-    auto new_trackers = std::map<QString, int>(torrents_per_host.begin(), torrents_per_host.end());
+    auto new_trackers = std::map<FaviconCache::Key, int>(torrents_per_tracker.begin(), torrents_per_tracker.end());
     auto old_it = tracker_counts_.cbegin();
     auto new_it = new_trackers.cbegin();
     auto const old_end = tracker_counts_.cend();
@@ -331,25 +334,8 @@ void FilterBar::onTrackerIndexChanged(int i)
 {
     if (!is_bootstrapping_)
     {
-        QString str;
+        auto const display_name = tracker_combo_->itemData(i, TRACKER_ROLE).toString();
-        bool const is_tracker = !tracker_combo_->itemData(i, TRACKER_ROLE).toString().isEmpty();
+        prefs_.set(Prefs::FILTER_TRACKERS, display_name);
-
-        if (!is_tracker)
-        {
-            // show all
-        }
-        else
-        {
-            str = tracker_combo_->itemData(i, TRACKER_ROLE).toString();
-            int const pos = str.lastIndexOf(QLatin1Char('.'));
-
-            if (pos >= 0)
-            {
-                str.truncate(pos + 1);
-            }
-        }
-
-        prefs_.set(Prefs::FILTER_TRACKERS, str);
     }
 }
 

qt/FilterBar.h

@@ -14,6 +14,7 @@
 #include <QString>
 #include <QWidget>
 
+#include "FaviconCache.h"
 #include "Torrent.h"
 #include "Typedefs.h"
 
@@ -56,7 +57,7 @@ private:
     TorrentModel const& torrents_;
     TorrentFilter const& filter_;
 
-    std::map<QString, int> tracker_counts_;
+    std::map<FaviconCache::Key, int> tracker_counts_;
     FilterBarComboBox* activity_combo_ = {};
     FilterBarComboBox* tracker_combo_ = {};
     QLabel* count_label_ = {};

qt/Torrent.cc

@@ -6,8 +6,9 @@
  *
  */
 
+#include <algorithm>
 #include <cassert>
-#include <iostream>
+#include <iterator>
 
 #include <QApplication>
 #include <QString>
@@ -284,10 +285,12 @@ bool change(TrackerStat& setme, tr_variant const* value)
     tr_variant* child;
     while (tr_variantDictChild(const_cast<tr_variant*>(value), pos++, &key, &child))
     {
+        bool field_changed = false;
+
         switch (key)
         {
 #define HANDLE_KEY(key, field) case TR_KEY_ ## key: \
-    changed = change(setme.field, child) || changed; break;
+    field_changed = change(setme.field, child); break;
             HANDLE_KEY(announce, announce)
             HANDLE_KEY(announceState, announce_state)
             HANDLE_KEY(downloadCount, download_count)
@@ -318,6 +321,16 @@ bool change(TrackerStat& setme, tr_variant const* value)
         default:
             break;
         }
+
+        if (field_changed)
+        {
+            if (key == TR_KEY_announce)
+            {
+                setme.favicon_key = qApp->faviconCache().add(QUrl(setme.announce));
+            }
+
+            changed = true;
+        }
     }
 
     return changed;
@@ -376,17 +389,9 @@ bool Torrent::getSeedRatio(double& setmeRatio) const
     return is_limited;
 }
 
-bool Torrent::hasTrackerSubstring(QString const& substr) const
+bool Torrent::includesTracker(FaviconCache::Key const& key) const
 {
-    for (auto const& s : trackers())
+    return tracker_keys_.count(key) != 0;
-    {
-        if (s.contains(substr, Qt::CaseInsensitive))
-        {
-            return true;
-        }
-    }
-
-    return false;
 }
 
 int Torrent::compareSeedRatio(Torrent const& that) const
@@ -586,10 +591,6 @@ Torrent::fields_t Torrent::update(tr_quark const* keys, tr_variant const* const*
         {
             switch (key)
             {
-            case TR_KEY_editDate:
-                // FIXME
-                break;
-
             case TR_KEY_name:
                 {
                     updateMimeIcon();
@@ -609,28 +610,11 @@ Torrent::fields_t Torrent::update(tr_quark const* keys, tr_variant const* const*
 
             case TR_KEY_trackers:
                 {
-                    // rebuild trackers_
+                    FaviconCache::Keys keys;
-                    QStringList urls;
+                    std::transform(std::cbegin(tracker_stats_), std::cend(tracker_stats_),
-                    urls.reserve(tracker_stats_.size());
+                        std::inserter(keys, std::end(keys)),
-                    for (auto const& t : tracker_stats_)
+                        [](auto const& ts) { return ts.favicon_key; });
-                    {
+                    std::swap(tracker_keys_, keys);
-                        urls.append(t.announce);
-                    }
-
-                    trackers_.swap(urls);
-
-                    // rebuild trackerDisplayNames
-                    QStringList display_names;
-                    display_names.reserve(trackers_.size());
-                    for (auto const& tracker : trackers_)
-                    {
-                        auto const url = QUrl(tracker);
-                        auto const key = qApp->faviconCache().add(url);
-                        display_names.append(FaviconCache::getDisplayName(key));
-                    }
-
-                    display_names.removeDuplicates();
-                    tracker_display_names_.swap(display_names);
                     break;
                 }
             }
@@ -706,5 +690,5 @@ QString Torrent::getError() const
 
 QPixmap TrackerStat::getFavicon() const
 {
-    return qApp->faviconCache().find(QUrl(announce));
+    return qApp->faviconCache().find(favicon_key);
 }

qt/Torrent.h

@@ -15,11 +15,11 @@
 #include <QMetaType>
 #include <QObject>
 #include <QString>
-#include <QStringList>
 
 #include <libtransmission/transmission.h>
 #include <libtransmission/quark.h>
 
+#include "FaviconCache.h"
 #include "Speed.h"
 
 #ifdef ERROR
@@ -81,6 +81,7 @@ struct TrackerStat
     int scrape_state;
     int seeder_count;
     int tier;
+    FaviconCache::Key favicon_key;
     QString announce;
     QString host;
     QString last_announce_result;
@@ -356,7 +357,12 @@ public:
         return recheck_progress_;
     }
 
-    bool hasTrackerSubstring(QString const& substr) const;
+    bool includesTracker(FaviconCache::Key const& key) const;
+
+    FaviconCache::Keys const& trackerKeys() const
+    {
+        return tracker_keys_;
+    }
 
     Speed uploadLimit() const
     {
@@ -413,16 +419,6 @@ public:
         return tracker_stats_;
     }
 
-    QStringList const& trackers() const
-    {
-        return trackers_;
-    }
-
-    QStringList const& trackerDisplayNames() const
-    {
-        return tracker_display_names_;
-    }
-
     PeerList const& peers() const
     {
         return peers_;
@@ -639,8 +635,7 @@ private:
     PeerList peers_;
     FileList files_;
 
-    QStringList trackers_;
+    FaviconCache::Keys tracker_keys_;
-    QStringList tracker_display_names_;
     TrackerStatsList tracker_stats_;
 
     Speed upload_speed_;

qt/TorrentFilter.cc

@@ -245,8 +245,9 @@ bool TorrentFilter::filterAcceptsRow(int source_row, QModelIndex const& source_p
 
     if (accepts)
     {
-        auto const name = prefs_.getString(Prefs::FILTER_TRACKERS);
+        auto const display_name = prefs_.getString(Prefs::FILTER_TRACKERS);
-        accepts = name.isEmpty() || tor.hasTrackerSubstring(name);
+        auto const key = FaviconCache::getKey(display_name);
+        accepts = key.isEmpty() || tor.includesTracker(key);
     }
 
     if (accepts)