webbrowser-app

Merge lp:~osomon/webbrowser-app/private-mode-favicons into lp:webbrowser-app

  1. private-mode-favicons
  2. Merge into trunk
Proposed by Olivier Tilloy
Status: Merged
Approved by: Olivier Tilloy
Approved revision: 1034
Merged at revision: 1043
Proposed branch: lp:~osomon/webbrowser-app/private-mode-favicons
Merge into: lp:webbrowser-app
Diff against target: 210 lines (+55/-8)
7 files modified
src/app/Favicon.qml (+2/-1)
src/app/favicon-fetcher.cpp (+25/-1)
src/app/favicon-fetcher.h (+7/-1)
src/app/webbrowser/AddressBar.qml (+2/-0)
src/app/webbrowser/Browser.qml (+1/-1)
src/app/webbrowser/Chrome.qml (+3/-3)
tests/unittests/favicon-fetcher/tst_FaviconFetcherTests.cpp (+15/-1)
To merge this branch: bzr merge lp:~osomon/webbrowser-app/private-mode-favicons
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing
Arthur Mello (community) Approve
Review via email: mp+260216@code.launchpad.net

Commit message

Do not cache favicons on disk when browsing in private mode.

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

FAILED: Continuous integration, rev:1034
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1814/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2953/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/571/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/571/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/571/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2951/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Arthur Mello (artmello) wrote :

The code looks good to me and so does the test. I am not sure why Jenkins is failing, but since all tests run fine locally I am approving the MR. Just triggered a rebuild to see if the issue happens again.

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

FAILED: Continuous integration, rev:1034
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1815/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2954/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/572/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/572/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/572/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2952/console

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

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/app/Favicon.qml'
2--- src/app/Favicon.qml 2014-11-26 11:44:50 +0000
3+++ src/app/Favicon.qml 2015-05-26 21:48:22 +0000
4@@ -1,5 +1,5 @@
5 /*
6- * Copyright 2014 Canonical Ltd.
7+ * Copyright 2014-2015 Canonical Ltd.
8 *
9 * This file is part of webbrowser-app.
10 *
11@@ -23,6 +23,7 @@
12 Item {
13 property alias source: fetcher.url
14 property bool fallbackIcon: true
15+ property alias shouldCache: fetcher.shouldCache
16
17 width: units.dp(16)
18 height: units.dp(16)
19
20=== modified file 'src/app/favicon-fetcher.cpp'
21--- src/app/favicon-fetcher.cpp 2015-03-23 07:51:17 +0000
22+++ src/app/favicon-fetcher.cpp 2015-05-26 21:48:22 +0000
23@@ -19,6 +19,8 @@
24 #include "favicon-fetcher.h"
25
26 // Qt
27+#include <QtCore/QBuffer>
28+#include <QtCore/QByteArray>
29 #include <QtCore/QCryptographicHash>
30 #include <QtCore/QDebug>
31 #include <QtCore/QDir>
32@@ -35,6 +37,7 @@
33
34 FaviconFetcher::FaviconFetcher(QObject* parent)
35 : QObject(parent)
36+ , m_shouldCache(true)
37 , m_reply(0)
38 , m_redirections(0)
39 {
40@@ -114,6 +117,19 @@
41 }
42 }
43
44+bool FaviconFetcher::shouldCache() const
45+{
46+ return m_shouldCache;
47+}
48+
49+void FaviconFetcher::setShouldCache(bool shouldCache)
50+{
51+ if (shouldCache != m_shouldCache) {
52+ m_shouldCache = shouldCache;
53+ Q_EMIT shouldCacheChanged();
54+ }
55+}
56+
57 const QString& FaviconFetcher::cacheLocation() const
58 {
59 return m_cacheLocation;
60@@ -144,8 +160,16 @@
61 if (url.isEmpty()) {
62 if (reply->error() == QNetworkReply::NoError) {
63 QByteArray data = reply->readAll();
64- if (QImage::fromData(data).save(m_filepath)) {
65+ QImage image = QImage::fromData(data);
66+ if (m_shouldCache && image.save(m_filepath)) {
67 setLocalUrl(QUrl::fromLocalFile(m_filepath));
68+ } else {
69+ QByteArray ba;
70+ QBuffer buffer(&ba);
71+ buffer.open(QIODevice::WriteOnly);
72+ if (image.save(&buffer, "PNG")) {
73+ setLocalUrl(QUrl("data:image/png;base64," + ba.toBase64()));
74+ }
75 }
76 } else {
77 qWarning() << "Failed to download" << reply->url()
78
79=== modified file 'src/app/favicon-fetcher.h'
80--- src/app/favicon-fetcher.h 2015-02-04 10:56:38 +0000
81+++ src/app/favicon-fetcher.h 2015-05-26 21:48:22 +0000
82@@ -1,5 +1,5 @@
83 /*
84- * Copyright 2014 Canonical Ltd.
85+ * Copyright 2014-2015 Canonical Ltd.
86 *
87 * This file is part of webbrowser-app.
88 *
89@@ -35,6 +35,7 @@
90
91 Q_PROPERTY(QUrl url READ url WRITE setUrl NOTIFY urlChanged)
92 Q_PROPERTY(QUrl localUrl READ localUrl NOTIFY localUrlChanged)
93+ Q_PROPERTY(bool shouldCache READ shouldCache WRITE setShouldCache NOTIFY shouldCacheChanged)
94
95 public:
96 FaviconFetcher(QObject* parent=0);
97@@ -45,11 +46,15 @@
98
99 const QUrl& localUrl() const;
100
101+ bool shouldCache() const;
102+ void setShouldCache(bool shouldCache);
103+
104 const QString& cacheLocation() const;
105
106 Q_SIGNALS:
107 void urlChanged() const;
108 void localUrlChanged() const;
109+ void shouldCacheChanged() const;
110
111 private Q_SLOTS:
112 void download(const QUrl& url);
113@@ -58,6 +63,7 @@
114 private:
115 void setLocalUrl(const QUrl& url);
116
117+ bool m_shouldCache;
118 QString m_cacheLocation;
119 QScopedPointer<QNetworkAccessManager> m_manager;
120 QNetworkReply* m_reply;
121
122=== modified file 'src/app/webbrowser/AddressBar.qml'
123--- src/app/webbrowser/AddressBar.qml 2015-05-18 14:29:29 +0000
124+++ src/app/webbrowser/AddressBar.qml 2015-05-26 21:48:22 +0000
125@@ -27,6 +27,7 @@
126 id: addressbar
127
128 property alias icon: favicon.source
129+ property bool incognito: false
130 property alias text: textField.text
131 property bool bookmarked: false
132 property url requestedUrl
133@@ -71,6 +72,7 @@
134
135 Favicon {
136 id: favicon
137+ shouldCache: !addressbar.incognito
138 anchors.verticalCenter: parent.verticalCenter
139 visible: internal.idle && addressbar.actualUrl.toString() &&
140 !internal.securityWarning && !internal.securityError
141
142=== modified file 'src/app/webbrowser/Browser.qml'
143--- src/app/webbrowser/Browser.qml 2015-05-22 19:21:10 +0000
144+++ src/app/webbrowser/Browser.qml 2015-05-26 21:48:22 +0000
145@@ -199,7 +199,7 @@
146 webview: browser.currentWebview
147 searchUrl: currentSearchEngine.urlTemplate
148
149- useDarkTheme: browser.incognito
150+ incognito: browser.incognito
151
152 y: webview ? webview.locationBarController.offset : 0
153
154
155=== modified file 'src/app/webbrowser/Chrome.qml'
156--- src/app/webbrowser/Chrome.qml 2015-05-19 19:22:17 +0000
157+++ src/app/webbrowser/Chrome.qml 2015-05-26 21:48:22 +0000
158@@ -29,9 +29,9 @@
159 property list<Action> drawerActions
160 readonly property bool drawerOpen: internal.openDrawer
161 property alias requestedUrl: addressbar.requestedUrl
162- property bool useDarkTheme: false
163+ property alias incognito: addressbar.incognito
164
165- backgroundColor: useDarkTheme ? UbuntuColors.darkGrey : Theme.palette.normal.background
166+ backgroundColor: incognito ? UbuntuColors.darkGrey : Theme.palette.normal.background
167
168 FocusScope {
169 anchors {
170@@ -155,7 +155,7 @@
171 QtObject {
172 id: internal
173 property var openDrawer: null
174- readonly property color iconColor: chrome.useDarkTheme ? "white" : "grey"
175+ readonly property color iconColor: chrome.incognito ? "white" : "grey"
176 }
177
178 onWebviewChanged: {
179
180=== modified file 'tests/unittests/favicon-fetcher/tst_FaviconFetcherTests.cpp'
181--- tests/unittests/favicon-fetcher/tst_FaviconFetcherTests.cpp 2014-11-27 09:45:44 +0000
182+++ tests/unittests/favicon-fetcher/tst_FaviconFetcherTests.cpp 2015-05-26 21:48:22 +0000
183@@ -1,5 +1,5 @@
184 /*
185- * Copyright 2014 Canonical Ltd.
186+ * Copyright 2014-2015 Canonical Ltd.
187 *
188 * This file is part of webbrowser-app.
189 *
190@@ -220,6 +220,20 @@
191 QVERIFY(serverSpy->isEmpty());
192 }
193
194+ void shouldNotCacheIcon()
195+ {
196+ fetcher->setShouldCache(false);
197+ QUrl url(server->baseURL() + "/favicon1.ico");
198+ fetcher->setUrl(url);
199+ QCOMPARE(fetcher->url(), url);
200+ QVERIFY(fetcherSpy->wait());
201+ QCOMPARE(serverSpy->count(), 1);
202+ QUrl icon = fetcher->localUrl();
203+ QVERIFY(!icon.isLocalFile());
204+ QCOMPARE(icon.scheme(), QString("data"));
205+ QVERIFY(icon.path().startsWith("image/png;base64,"));
206+ }
207+
208 void shouldHandleRedirections()
209 {
210 QUrl url(server->baseURL() + "/redirect/3/favicon1.ico");

Subscribers

People subscribed via source and target branches

to status/vote changes: