webbrowser-app

Merge lp:~osomon/webbrowser-app/limit-thumbnail-cache-size into lp:webbrowser-app

  1. limit-thumbnail-cache-size
  2. Merge into trunk
Proposed by Olivier Tilloy
Status: Merged
Approved by: Günter Schwann
Approved revision: 286
Merged at revision: 280
Proposed branch: lp:~osomon/webbrowser-app/limit-thumbnail-cache-size
Merge into: lp:webbrowser-app
Diff against target: 275 lines (+117/-22)
7 files modified
src/Ubuntu/Components/Extras/Browser/plugin.cpp (+18/-0)
src/Ubuntu/Components/Extras/Browser/plugin.h (+8/-0)
src/Ubuntu/Components/Extras/Browser/webthumbnail-utils.cpp (+66/-0)
src/Ubuntu/Components/Extras/Browser/webthumbnail-utils.h (+19/-3)
src/Ubuntu/Components/Extras/Browser/webview-thumbnailer.cpp (+6/-17)
tests/unittests/history-domainlist-chronological-model/CMakeLists.txt (+0/-1)
tests/unittests/history-domainlist-model/CMakeLists.txt (+0/-1)
To merge this branch: bzr merge lp:~osomon/webbrowser-app/limit-thumbnail-cache-size
Reviewer Review Type Date Requested Status
Günter Schwann (community) Approve
PS Jenkins bot continuous-integration Approve
Review via email: mp+179247@code.launchpad.net

Commit message

Delete old thumbnails to ensure the cache never grows bigger than 5MB.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:282
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/276/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/2032/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-ci/159/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/159/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-ci/159/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy/2037/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/webbrowser-app-ci/276/rebuild

review: Needs Fixing (continuous-integration)
lp:~osomon/webbrowser-app/limit-thumbnail-cache-size updated
283. By Olivier Tilloy

Remove unused dependencies.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:283
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/277/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/2035
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-ci/160
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/160
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/160/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-ci/160
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy/2040
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy/2040/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-saucy/1735

Click here to trigger a rebuild:
http://s-jenkins:8080/job/webbrowser-app-ci/277/rebuild

review: Approve (continuous-integration)
Revision history for this message
Günter Schwann (schwann) wrote :

45 + m_thumbnailUtilsThread->deleteLater();
I'd call m_thumbnailUtilsThread->wait() and delete the thread directly instead

234 + QMetaObject::invokeMethod(&WebThumbnailUtils::instance(), "cacheThumbnail",
You could add the connection type Qt::QueuedConnection, to highlight (and make sure) that it's asynchronous.

148 + qint64 goal = MAX_CACHE_SIZE_IN_BYTES * 9 / 10;
So the max cache size is 90% of 5MB? Why?

lp:~osomon/webbrowser-app/limit-thumbnail-cache-size updated
284. By Olivier Tilloy

Ensure the thread terminates before deleting it.

285. By Olivier Tilloy

Make it (more) explicit that the method is called asynchronously.

286. By Olivier Tilloy

Do not expire cache entries if the total size doesn’t exceed the limit.

Revision history for this message
Olivier Tilloy (osomon) wrote :

> 45 + m_thumbnailUtilsThread->deleteLater();
> I'd call m_thumbnailUtilsThread->wait() and delete the thread directly instead

Done. Thanks for the suggestion.

> 234 + QMetaObject::invokeMethod(&WebThumbnailUtils::instance(),
> "cacheThumbnail",
> You could add the connection type Qt::QueuedConnection, to highlight (and make
> sure) that it's asynchronous.

Although not strictly necessary, done.

> 148 + qint64 goal = MAX_CACHE_SIZE_IN_BYTES * 9 / 10;
> So the max cache size is 90% of 5MB? Why?

That was a mistake on my part. This code is not triggered only if the total cache size exceeds the limit. The goal is 90% of the limit to have some buffer, so that we do not need to expire entries every time a new thumbnail is generated (this is inspired by the implementation of the QNetworkDiskCache class).

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:286
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/278/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/2049
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-ci/161
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/161
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/161/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-ci/161
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy/2054
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy/2054/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-saucy/1747

Click here to trigger a rebuild:
http://s-jenkins:8080/job/webbrowser-app-ci/278/rebuild

review: Approve (continuous-integration)
Revision history for this message
Günter Schwann (schwann) wrote :

Looks good

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/Ubuntu/Components/Extras/Browser/plugin.cpp'
--- src/Ubuntu/Components/Extras/Browser/plugin.cpp 2013-07-30 17:26:49 +0000
+++ src/Ubuntu/Components/Extras/Browser/plugin.cpp 2013-08-09 07:47:22 +0000
@@ -26,11 +26,13 @@
26#include "tabs-model.h"26#include "tabs-model.h"
27#include "bookmarks-model.h"27#include "bookmarks-model.h"
28#include "webthumbnail-provider.h"28#include "webthumbnail-provider.h"
29#include "webthumbnail-utils.h"
29#include "webview-thumbnailer.h"30#include "webview-thumbnailer.h"
3031
31// Qt32// Qt
32#include <QtCore/QDir>33#include <QtCore/QDir>
33#include <QtCore/QStandardPaths>34#include <QtCore/QStandardPaths>
35#include <QtCore/QThread>
34#include <QtQml>36#include <QtQml>
3537
36void UbuntuBrowserPlugin::initializeEngine(QQmlEngine* engine, const char* uri)38void UbuntuBrowserPlugin::initializeEngine(QQmlEngine* engine, const char* uri)
@@ -43,9 +45,18 @@
43 QQmlContext* context = engine->rootContext();45 QQmlContext* context = engine->rootContext();
44 context->setContextProperty("dataLocation", dataLocation.absolutePath());46 context->setContextProperty("dataLocation", dataLocation.absolutePath());
4547
48 // This singleton lives in its own thread to ensure that
49 // disk I/O is not performed in the UI thread.
50 WebThumbnailUtils& utils = WebThumbnailUtils::instance();
51 m_thumbnailUtilsThread = new QThread;
52 utils.moveToThread(m_thumbnailUtilsThread);
53 m_thumbnailUtilsThread->start();
54
46 WebThumbnailProvider* thumbnailer = new WebThumbnailProvider;55 WebThumbnailProvider* thumbnailer = new WebThumbnailProvider;
47 engine->addImageProvider(QLatin1String("webthumbnail"), thumbnailer);56 engine->addImageProvider(QLatin1String("webthumbnail"), thumbnailer);
48 context->setContextProperty("WebThumbnailer", thumbnailer);57 context->setContextProperty("WebThumbnailer", thumbnailer);
58
59 connect(engine, SIGNAL(destroyed()), SLOT(onEngineDestroyed()));
49}60}
5061
51void UbuntuBrowserPlugin::registerTypes(const char* uri)62void UbuntuBrowserPlugin::registerTypes(const char* uri)
@@ -61,3 +72,10 @@
61 qmlRegisterType<BookmarksModel>(uri, 0, 1, "BookmarksModel");72 qmlRegisterType<BookmarksModel>(uri, 0, 1, "BookmarksModel");
62 qmlRegisterType<WebviewThumbnailer>(uri, 0, 1, "WebviewThumbnailer");73 qmlRegisterType<WebviewThumbnailer>(uri, 0, 1, "WebviewThumbnailer");
63}74}
75
76void UbuntuBrowserPlugin::onEngineDestroyed()
77{
78 m_thumbnailUtilsThread->quit();
79 m_thumbnailUtilsThread->wait();
80 delete m_thumbnailUtilsThread;
81}
6482
=== modified file 'src/Ubuntu/Components/Extras/Browser/plugin.h'
--- src/Ubuntu/Components/Extras/Browser/plugin.h 2013-06-11 12:06:15 +0000
+++ src/Ubuntu/Components/Extras/Browser/plugin.h 2013-08-09 07:47:22 +0000
@@ -22,6 +22,8 @@
22// Qt22// Qt
23#include <QtQml/QQmlExtensionPlugin>23#include <QtQml/QQmlExtensionPlugin>
2424
25class QThread;
26
25class UbuntuBrowserPlugin : public QQmlExtensionPlugin27class UbuntuBrowserPlugin : public QQmlExtensionPlugin
26{28{
27 Q_OBJECT29 Q_OBJECT
@@ -30,6 +32,12 @@
30public:32public:
31 void initializeEngine(QQmlEngine* engine, const char* uri);33 void initializeEngine(QQmlEngine* engine, const char* uri);
32 void registerTypes(const char* uri);34 void registerTypes(const char* uri);
35
36private:
37 QThread* m_thumbnailUtilsThread;
38
39private Q_SLOTS:
40 void onEngineDestroyed();
33};41};
3442
35#endif // __PLUGIN_H__43#endif // __PLUGIN_H__
3644
=== modified file 'src/Ubuntu/Components/Extras/Browser/webthumbnail-utils.cpp'
--- src/Ubuntu/Components/Extras/Browser/webthumbnail-utils.cpp 2013-07-10 12:02:00 +0000
+++ src/Ubuntu/Components/Extras/Browser/webthumbnail-utils.cpp 2013-08-09 07:47:22 +0000
@@ -16,11 +16,32 @@
16 * along with this program. If not, see <http://www.gnu.org/licenses/>.16 * along with this program. If not, see <http://www.gnu.org/licenses/>.
17 */17 */
1818
19#include "domain-utils.h"
19#include "webthumbnail-utils.h"20#include "webthumbnail-utils.h"
2021
21// Qt22// Qt
22#include <QtCore/QCryptographicHash>23#include <QtCore/QCryptographicHash>
24#include <QtCore/QFile>
23#include <QtCore/QStandardPaths>25#include <QtCore/QStandardPaths>
26#include <QtCore/QUrl>
27#include <QtGui/QImage>
28
29#define MAX_CACHE_SIZE_IN_BYTES 5 * 1024 * 1024 // 5MB
30
31WebThumbnailUtils::WebThumbnailUtils(QObject* parent)
32 : QObject(parent)
33{
34}
35
36WebThumbnailUtils& WebThumbnailUtils::instance()
37{
38 static WebThumbnailUtils utils;
39 return utils;
40}
41
42WebThumbnailUtils::~WebThumbnailUtils()
43{
44}
2445
25QDir WebThumbnailUtils::cacheLocation()46QDir WebThumbnailUtils::cacheLocation()
26{47{
@@ -40,3 +61,48 @@
40 QString hash(QCryptographicHash::hash(url.toEncoded(), QCryptographicHash::Md5).toHex());61 QString hash(QCryptographicHash::hash(url.toEncoded(), QCryptographicHash::Md5).toHex());
41 return cacheLocation().absoluteFilePath(hash + ".png");62 return cacheLocation().absoluteFilePath(hash + ".png");
42}63}
64
65void WebThumbnailUtils::cacheThumbnail(const QUrl& url, const QImage& thumbnail) const
66{
67 ensureCacheLocation();
68 QFileInfo file = thumbnailFile(url);
69 bool saved = thumbnail.save(file.absoluteFilePath());
70
71 if (saved) {
72 // Make a link to the thumbnail file for the corresponding domain’s thumbnail.
73 QUrl domain(DomainUtils::extractTopLevelDomainName(url));
74 QString domainThumbnail = WebThumbnailUtils::thumbnailFile(domain).absoluteFilePath();
75 if (QFile::exists(domainThumbnail)) {
76 QFile::remove(domainThumbnail);
77 }
78 QFile::link(file.fileName(), domainThumbnail);
79 }
80
81 expireCache();
82}
83
84void WebThumbnailUtils::expireCache() const
85{
86 QDir cache = cacheLocation();
87 if (!cache.exists()) {
88 return;
89 }
90 QStringList nameFilters = QStringList() << "*.png";
91 QDir::Filters filters = QDir::Files | QDir::NoDotAndDotDot;
92 QDir::SortFlags sort = QDir::Time;
93 QFileInfoList entries = cache.entryInfoList(nameFilters, filters, sort);
94 qint64 currentSize = 0;
95 Q_FOREACH(const QFileInfo& entry, entries) {
96 currentSize += entry.size();
97 }
98 if (currentSize > MAX_CACHE_SIZE_IN_BYTES) {
99 qint64 goal = MAX_CACHE_SIZE_IN_BYTES * 9 / 10;
100 while (!entries.isEmpty() && (currentSize > goal)) {
101 QFileInfo entry = entries.takeLast();
102 qint64 size = entry.size();
103 if (QFile::remove(entry.absoluteFilePath())) {
104 currentSize -= size;
105 }
106 }
107 }
108}
43109
=== modified file 'src/Ubuntu/Components/Extras/Browser/webthumbnail-utils.h'
--- src/Ubuntu/Components/Extras/Browser/webthumbnail-utils.h 2013-07-10 12:02:00 +0000
+++ src/Ubuntu/Components/Extras/Browser/webthumbnail-utils.h 2013-08-09 07:47:22 +0000
@@ -22,14 +22,30 @@
22// Qt22// Qt
23#include <QtCore/QDir>23#include <QtCore/QDir>
24#include <QtCore/QFileInfo>24#include <QtCore/QFileInfo>
25#include <QtCore/QUrl>25#include <QtCore/QObject>
2626
27class WebThumbnailUtils27class QImage;
28class QUrl;
29
30class WebThumbnailUtils : public QObject
28{31{
32 Q_OBJECT
33
29public:34public:
35 static WebThumbnailUtils& instance();
36 ~WebThumbnailUtils();
37
30 static QDir cacheLocation();38 static QDir cacheLocation();
31 static void ensureCacheLocation();39 static void ensureCacheLocation();
32 static QFileInfo thumbnailFile(const QUrl& url);40 static QFileInfo thumbnailFile(const QUrl& url);
41
42public Q_SLOTS:
43 void cacheThumbnail(const QUrl& url, const QImage& thumbnail) const;
44
45private:
46 WebThumbnailUtils(QObject* parent=0);
47
48 void expireCache() const;
33};49};
3450
35#endif // __WEBTHUMBNAIL_UTILS_H__51#endif // __WEBTHUMBNAIL_UTILS_H__
3652
=== modified file 'src/Ubuntu/Components/Extras/Browser/webview-thumbnailer.cpp'
--- src/Ubuntu/Components/Extras/Browser/webview-thumbnailer.cpp 2013-08-06 10:16:28 +0000
+++ src/Ubuntu/Components/Extras/Browser/webview-thumbnailer.cpp 2013-08-09 07:47:22 +0000
@@ -16,12 +16,11 @@
16 * along with this program. If not, see <http://www.gnu.org/licenses/>.16 * along with this program. If not, see <http://www.gnu.org/licenses/>.
17 */17 */
1818
19#include "domain-utils.h"
20#include "webthumbnail-utils.h"19#include "webthumbnail-utils.h"
21#include "webview-thumbnailer.h"20#include "webview-thumbnailer.h"
2221
23// Qt22// Qt
24#include <QtCore/QFile>23#include <QtCore/QMetaObject>
25#include <QtCore/QTimer>24#include <QtCore/QTimer>
26#include <QtQuick/private/qsgrenderer_p.h>25#include <QtQuick/private/qsgrenderer_p.h>
27#include <QtWebKit/private/qquickwebpage_p.h>26#include <QtWebKit/private/qquickwebpage_p.h>
@@ -140,20 +139,12 @@
140139
141 fbo.release();140 fbo.release();
142141
142 const QUrl& url = m_webview->url();
143 QImage image = fbo.toImage().scaled(m_targetSize, Qt::KeepAspectRatioByExpanding, Qt::SmoothTransformation);143 QImage image = fbo.toImage().scaled(m_targetSize, Qt::KeepAspectRatioByExpanding, Qt::SmoothTransformation);
144144
145 WebThumbnailUtils::ensureCacheLocation();145 // Invoke the method asynchronously.
146 QUrl url = m_webview->url();146 QMetaObject::invokeMethod(&WebThumbnailUtils::instance(), "cacheThumbnail",
147 QFileInfo thumbnail = WebThumbnailUtils::thumbnailFile(url);147 Qt::QueuedConnection, Q_ARG(QUrl, url), Q_ARG(QImage, image));
148 bool saved = image.save(thumbnail.absoluteFilePath());
149
150 // Make a link to the thumbnail file for the corresponding domain’s thumbnail.
151 QUrl domain(DomainUtils::extractTopLevelDomainName(url));
152 QString domainThumbnail = WebThumbnailUtils::thumbnailFile(domain).absoluteFilePath();
153 if (QFile::exists(domainThumbnail)) {
154 QFile::remove(domainThumbnail);
155 }
156 QFile::link(thumbnail.fileName(), domainThumbnail);
157148
158 root.removeChildNode(node);149 root.removeChildNode(node);
159150
@@ -165,9 +156,7 @@
165 }156 }
166 }157 }
167158
168 if (saved) {159 Q_EMIT thumbnailRendered(url);
169 Q_EMIT thumbnailRendered(url);
170 }
171160
172 return oldNode;161 return oldNode;
173}162}
174163
=== modified file 'tests/unittests/history-domainlist-chronological-model/CMakeLists.txt'
--- tests/unittests/history-domainlist-chronological-model/CMakeLists.txt 2013-07-29 10:32:13 +0000
+++ tests/unittests/history-domainlist-chronological-model/CMakeLists.txt 2013-08-09 07:47:22 +0000
@@ -6,7 +6,6 @@
6 ${webbrowser-plugin_SOURCE_DIR}/history-domainlist-chronological-model.cpp6 ${webbrowser-plugin_SOURCE_DIR}/history-domainlist-chronological-model.cpp
7 ${webbrowser-plugin_SOURCE_DIR}/history-model.cpp7 ${webbrowser-plugin_SOURCE_DIR}/history-model.cpp
8 ${webbrowser-plugin_SOURCE_DIR}/history-timeframe-model.cpp8 ${webbrowser-plugin_SOURCE_DIR}/history-timeframe-model.cpp
9 ${webbrowser-plugin_SOURCE_DIR}/webthumbnail-utils.cpp
10 tst_HistoryDomainListChronologicalModelTests.cpp9 tst_HistoryDomainListChronologicalModelTests.cpp
11)10)
12add_executable(${TEST} ${SOURCES})11add_executable(${TEST} ${SOURCES})
1312
=== modified file 'tests/unittests/history-domainlist-model/CMakeLists.txt'
--- tests/unittests/history-domainlist-model/CMakeLists.txt 2013-07-29 10:32:13 +0000
+++ tests/unittests/history-domainlist-model/CMakeLists.txt 2013-08-09 07:47:22 +0000
@@ -5,7 +5,6 @@
5 ${webbrowser-plugin_SOURCE_DIR}/history-domainlist-model.cpp5 ${webbrowser-plugin_SOURCE_DIR}/history-domainlist-model.cpp
6 ${webbrowser-plugin_SOURCE_DIR}/history-model.cpp6 ${webbrowser-plugin_SOURCE_DIR}/history-model.cpp
7 ${webbrowser-plugin_SOURCE_DIR}/history-timeframe-model.cpp7 ${webbrowser-plugin_SOURCE_DIR}/history-timeframe-model.cpp
8 ${webbrowser-plugin_SOURCE_DIR}/webthumbnail-utils.cpp
9 tst_HistoryDomainListModelTests.cpp8 tst_HistoryDomainListModelTests.cpp
10)9)
11add_executable(${TEST} ${SOURCES})10add_executable(${TEST} ${SOURCES})

Subscribers

People subscribed via source and target branches

to status/vote changes: