Merge lp:~michihenning/thumbnailer/request-cancellation into lp:thumbnailer/devel
- request-cancellation
- Merge into devel
Proposed by
Michi Henning
| Status: | Merged |
|---|---|
| Approved by: | James Henstridge |
| Approved revision: | 319 |
| Merged at revision: | 292 |
| Proposed branch: | lp:~michihenning/thumbnailer/request-cancellation |
| Merge into: | lp:thumbnailer/devel |
| Diff against target: |
1495 lines (+570/-158) 23 files modified
data/com.canonical.Unity.Thumbnailer.gschema.xml (+8/-0) debian/changelog (+2/-0) debian/libthumbnailer-qt1.0.symbols (+1/-0) include/internal/thumbnailer.h (+1/-24) include/ratelimiter.h (+8/-3) include/settings.h (+2/-0) include/unity/thumbnailer/qt/thumbnailer-qt.h (+12/-6) man/thumbnailer-settings.5 (+25/-12) plugins/Ubuntu/Thumbnailer.0.1/thumbnailerimageresponse.cpp (+1/-1) src/libthumbnailer-qt/libthumbnailer-qt.cpp (+99/-46) src/ratelimiter.cpp (+23/-11) src/service/dbusinterface.cpp (+3/-3) src/service/dbusinterface.h (+2/-2) src/service/handler.cpp (+19/-9) src/service/handler.h (+1/-1) src/settings.cpp (+14/-0) src/thumbnailer.cpp (+24/-0) src/vs-thumb/thumbnailextractor.cpp (+1/-1) tests/libthumbnailer-qt/CMakeLists.txt (+2/-0) tests/libthumbnailer-qt/libthumbnailer-qt_test.cpp (+189/-16) tests/stress/stress_test.cpp (+109/-23) tests/thumbnailer/thumbnailer_test.cpp (+19/-0) tools/parse-settings.py (+5/-0) |
| To merge this branch: | bzr merge lp:~michihenning/thumbnailer/request-cancellation |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| James Henstridge | Approve | ||
| PS Jenkins bot (community) | continuous-integration | Approve | |
|
Review via email:
|
|||
Commit message
Fixed broken request cancellation. Improved test coverage.
Description of the change
Fixed broken request cancellation. Improved test coverage.
To post a comment you must log in.
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote : | # |
review:
Needs Fixing
(continuous-integration)
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:291
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
review:
Needs Fixing
(continuous-integration)
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:292
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
review:
Approve
(continuous-integration)
Revision history for this message
| James Henstridge (jamesh) wrote : | # |
The new logic inside waitForFinished() looks suspect. See the inline comment for details.
review:
Needs Fixing
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:293
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
review:
Approve
(continuous-integration)
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:294
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
review:
Approve
(continuous-integration)
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:297
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
review:
Approve
(continuous-integration)
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:302
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
review:
Approve
(continuous-integration)
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:304
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
review:
Approve
(continuous-integration)
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:305
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
review:
Approve
(continuous-integration)
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:310
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
review:
Approve
(continuous-integration)
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:311
http://
Executed test runs:
None: http://
None: http://
None: http://
None: http://
None: http://
None: http://
Click here to trigger a rebuild:
http://
review:
Needs Fixing
(continuous-integration)
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:312
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
review:
Approve
(continuous-integration)
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:318
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
review:
Approve
(continuous-integration)
- 319. By Michi Henning
-
Changed no-extension test to use a real file instead of a symbolic link, otherwise we pick
up the thumbnail from the previously-cached version. This ensures that we correctly
test content type determination for non-empty files without a file extensions.
Fixed incorrect coverage suppression for glib >2.21.
Revision history for this message
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:319
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
review:
Approve
(continuous-integration)
Revision history for this message
| James Henstridge (jamesh) wrote : | # |
The logic in waitForFinished() looks about right now.
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
| 1 | === modified file 'data/com.canonical.Unity.Thumbnailer.gschema.xml' |
| 2 | --- data/com.canonical.Unity.Thumbnailer.gschema.xml 2015-09-22 03:02:07 +0000 |
| 3 | +++ data/com.canonical.Unity.Thumbnailer.gschema.xml 2015-11-02 00:48:23 +0000 |
| 4 | @@ -80,5 +80,13 @@ |
| 5 | This parameter limits the number of pending DBus requests to the thumbnailer service. If there are more requests than this currently in progress, additional requests are queued and sent once the backlog drops below the limit. |
| 6 | </description> |
| 7 | </key> |
| 8 | + |
| 9 | + <key type="b" name="trace-client"> |
| 10 | + <default>false</default> |
| 11 | + <summary>Enable client-side tracing.</summary> |
| 12 | + <description> |
| 13 | + If set to true, this parameter enables request tracing via qDebug() on the client side. |
| 14 | + </description> |
| 15 | + </key> |
| 16 | </schema> |
| 17 | </schemalist> |
| 18 | |
| 19 | === modified file 'debian/changelog' |
| 20 | --- debian/changelog 2015-09-30 07:15:41 +0000 |
| 21 | +++ debian/changelog 2015-11-02 00:48:23 +0000 |
| 22 | @@ -1,6 +1,8 @@ |
| 23 | thumbnailer (2.3+15.10.20150915.1-0ubuntu2) UNRELEASED; urgency=medium |
| 24 | [ Michi Henning ] |
| 25 | * "thumbnailer-admin clear f" now also resets the network retry timeout. |
| 26 | + * Added cancelled() accessor to Request class. |
| 27 | + * Added trace-client key to gsettings. |
| 28 | |
| 29 | -- Michi Henning <michi.henning@canonical.com> Sun, 20 Sep 2015 20:06:32 +1000 |
| 30 | |
| 31 | |
| 32 | === modified file 'debian/libthumbnailer-qt1.0.symbols' |
| 33 | --- debian/libthumbnailer-qt1.0.symbols 2015-09-15 11:04:14 +0000 |
| 34 | +++ debian/libthumbnailer-qt1.0.symbols 2015-11-02 00:48:23 +0000 |
| 35 | @@ -5,6 +5,7 @@ |
| 36 | (c++)"unity::thumbnailer::qt::Request::errorMessage() const@Base" 2.3+15.10.20150915.1 |
| 37 | (c++)"unity::thumbnailer::qt::Request::finished()@Base" 2.3+15.10.20150915.1 |
| 38 | (c++)"unity::thumbnailer::qt::Request::image() const@Base" 2.3+15.10.20150915.1 |
| 39 | + (c++)"unity::thumbnailer::qt::Request::isCancelled() const@Base" 0replaceme |
| 40 | (c++)"unity::thumbnailer::qt::Request::isFinished() const@Base" 2.3+15.10.20150915.1 |
| 41 | (c++)"unity::thumbnailer::qt::Request::isValid() const@Base" 2.3+15.10.20150915.1 |
| 42 | (c++)"unity::thumbnailer::qt::Request::metaObject() const@Base" 2.3+15.10.20150915.1 |
| 43 | |
| 44 | === modified file 'include/internal/thumbnailer.h' |
| 45 | --- include/internal/thumbnailer.h 2015-09-10 10:01:34 +0000 |
| 46 | +++ include/internal/thumbnailer.h 2015-11-02 00:48:23 +0000 |
| 47 | @@ -37,8 +37,6 @@ |
| 48 | namespace internal |
| 49 | { |
| 50 | |
| 51 | -class RequestBase; |
| 52 | - |
| 53 | class ThumbnailRequest : public QObject |
| 54 | { |
| 55 | Q_OBJECT |
| 56 | @@ -80,16 +78,7 @@ |
| 57 | void downloadFinished(); |
| 58 | }; |
| 59 | |
| 60 | -/** |
| 61 | - * This class provides a way to generate and access |
| 62 | - * thumbnails of video, audio and image files. |
| 63 | - * |
| 64 | - * All methods are blocking. |
| 65 | - * |
| 66 | - * All methods are thread safe. |
| 67 | - * |
| 68 | - * Errors are reported as exceptions. |
| 69 | - */ |
| 70 | +class RequestBase; |
| 71 | |
| 72 | class Thumbnailer |
| 73 | { |
| 74 | @@ -100,25 +89,13 @@ |
| 75 | Thumbnailer(Thumbnailer const&) = delete; |
| 76 | Thumbnailer& operator=(Thumbnailer const&) = delete; |
| 77 | |
| 78 | - /** |
| 79 | - * Gets a thumbnail of the given input file in the requested size. |
| 80 | - * |
| 81 | - * Return value is the thumbnail image as a string. |
| 82 | - * If the thumbnail could not be generated, an empty string is returned. |
| 83 | - */ |
| 84 | std::unique_ptr<ThumbnailRequest> get_thumbnail(std::string const& filename, |
| 85 | QSize const& requested_size); |
| 86 | |
| 87 | - /** |
| 88 | - * Gets album art for the given artist an album. |
| 89 | - */ |
| 90 | std::unique_ptr<ThumbnailRequest> get_album_art(std::string const& artist, |
| 91 | std::string const& album, |
| 92 | QSize const& requested_size); |
| 93 | |
| 94 | - /** |
| 95 | - * Gets artist art for the given artist and album. |
| 96 | - */ |
| 97 | std::unique_ptr<ThumbnailRequest> get_artist_art(std::string const& artist, |
| 98 | std::string const& album, |
| 99 | QSize const& requested_size); |
| 100 | |
| 101 | === modified file 'include/ratelimiter.h' |
| 102 | --- include/ratelimiter.h 2015-09-17 06:23:52 +0000 |
| 103 | +++ include/ratelimiter.h 2015-11-02 00:48:23 +0000 |
| 104 | @@ -42,19 +42,24 @@ |
| 105 | RateLimiter(RateLimiter const&) = delete; |
| 106 | RateLimiter& operator=(RateLimiter const&) = delete; |
| 107 | |
| 108 | + typedef std::function<void() noexcept> CancelFunc; |
| 109 | + |
| 110 | // Schedule a job to run. If the concurrency limit has not been |
| 111 | // reached, the job will be run immediately. Otherwise it will be |
| 112 | // added to the queue. Return value is a function that, when |
| 113 | // called, cancels the job in the queue (if it's still in the queue). |
| 114 | - std::function<void() noexcept> schedule(std::function<void()> job); |
| 115 | + CancelFunc schedule(std::function<void()> job); |
| 116 | + |
| 117 | + // Schedule a job to run immediately, regardless of the concurrency limit. |
| 118 | + CancelFunc schedule_now(std::function<void()> job); |
| 119 | |
| 120 | // Notify that a job has completed. If there are queued jobs, |
| 121 | // start the one at the head of the queue. |
| 122 | void done(); |
| 123 | |
| 124 | private: |
| 125 | - int const concurrency_; |
| 126 | - int running_; |
| 127 | + int const concurrency_; // Max number of outstanding requests. |
| 128 | + int running_; // Actual number of outstanding requests. |
| 129 | // We store a shared_ptr so we can detect on cancellation |
| 130 | // whether a job completed before it was cancelled. |
| 131 | std::queue<std::shared_ptr<std::function<void()>>> queue_; |
| 132 | |
| 133 | === modified file 'include/settings.h' |
| 134 | --- include/settings.h 2015-08-03 03:35:38 +0000 |
| 135 | +++ include/settings.h 2015-11-02 00:48:23 +0000 |
| 136 | @@ -51,12 +51,14 @@ |
| 137 | int max_extractions() const; |
| 138 | int extraction_timeout() const; // In seconds |
| 139 | int max_backlog() const; |
| 140 | + bool trace_client() const; |
| 141 | |
| 142 | private: |
| 143 | std::string get_string(char const* key, std::string const& default_value) const; |
| 144 | int get_positive_int(char const* key, int default_value) const; |
| 145 | int get_positive_or_zero_int(char const* key, int default_value) const; |
| 146 | int get_int(char const* key, int default_value) const; |
| 147 | + bool get_bool(char const* key, bool default_value) const; |
| 148 | |
| 149 | std::unique_ptr<GSettingsSchema, void(*)(GSettingsSchema*)> schema_; |
| 150 | std::string schema_name_; |
| 151 | |
| 152 | === modified file 'include/unity/thumbnailer/qt/thumbnailer-qt.h' |
| 153 | --- include/unity/thumbnailer/qt/thumbnailer-qt.h 2015-09-21 05:50:54 +0000 |
| 154 | +++ include/unity/thumbnailer/qt/thumbnailer-qt.h 2015-11-02 00:48:23 +0000 |
| 155 | @@ -76,7 +76,7 @@ |
| 156 | /** |
| 157 | \brief Returns whether the request completed successfully. |
| 158 | \return `true` if the request completed successfully. Otherwise, if the request is still |
| 159 | - in progress or has failed, the return value is `false`. |
| 160 | + in progress, has failed, or was cancelled, the return value is `false`. |
| 161 | */ |
| 162 | bool isValid() const; |
| 163 | |
| 164 | @@ -88,22 +88,28 @@ |
| 165 | |
| 166 | \warning Calling this function from the main (GUI) thread might cause your user interface to freeze. |
| 167 | |
| 168 | - \warning Calling waitForFinished() may cause the request to be scheduled out of order. This means |
| 169 | + \warning Calling waitForFinished() causes the request to be scheduled out of order. This means |
| 170 | that, if you send requests for thumbnails A, B, and C (in that order) and then call waitForFinished() |
| 171 | - on C, you _cannot_ assume that A and B have also finished once waitForFinsished() returns. |
| 172 | + on C, you _cannot_ assume that A and B have also finished once waitForFinished() returns. |
| 173 | */ |
| 174 | void waitForFinished(); |
| 175 | |
| 176 | /** |
| 177 | \brief Cancel the thumbnail request. |
| 178 | |
| 179 | - The finished signal will be emitted and the request will be |
| 180 | - considered to be in an invalid state with an error message set. |
| 181 | - |
| 182 | + Cancels the request if it has not completed yet and emits the finished() signal. |
| 183 | Calling cancel() more than once or on a request that has already completed does nothing. |
| 184 | */ |
| 185 | void cancel(); |
| 186 | |
| 187 | + /** |
| 188 | + \brief Returns whether the request was cancelled. |
| 189 | + \return `true` if the request was cancelled and `false`, otherwise. |
| 190 | + \note Depending on the time at which cancel() is called, |
| 191 | + the request may complete successfully despite having been cancelled. |
| 192 | + */ |
| 193 | + bool isCancelled() const; |
| 194 | + |
| 195 | Q_SIGNALS: |
| 196 | /** |
| 197 | \brief This signal is emitted when the request completes. |
| 198 | |
| 199 | === modified file 'man/thumbnailer-settings.5' |
| 200 | --- man/thumbnailer-settings.5 2015-09-30 10:08:20 +0000 |
| 201 | +++ man/thumbnailer-settings.5 2015-11-02 00:48:23 +0000 |
| 202 | @@ -13,31 +13,38 @@ |
| 203 | The API key for the remote artwork service. |
| 204 | .TP |
| 205 | .B full\-size\-cache\-size \fR(int)\fP |
| 206 | -The size (in megabytes) of the image cache that stores original artwork. The default is 50 MB. |
| 207 | -.TP |
| 208 | -.B thumbnail\-size\-cache\-size \fR(int)\fP |
| 209 | -The size (in megabytes) of the thumbnail cache that stores scaled thumbnails. The default is 100 MB. |
| 210 | -.TP |
| 211 | -.B thumbnail\-size\-cache\-size \fR(int)\fP |
| 212 | -The size (in megabytes) of the failure cache that stores media keys for media without an image. The default is 2 MB. |
| 213 | +The size (in megabytes) of the image cache that stores original artwork. |
| 214 | +The default is 50 MB. |
| 215 | +.TP |
| 216 | +.B thumbnail\-size\-cache\-size \fR(int)\fP |
| 217 | +The size (in megabytes) of the thumbnail cache that stores scaled thumbnails. |
| 218 | +The default is 100 MB. |
| 219 | +.TP |
| 220 | +.B thumbnail\-size\-cache\-size \fR(int)\fP |
| 221 | +The size (in megabytes) of the failure cache that stores media keys for media without an image. |
| 222 | +The default is 2 MB. |
| 223 | .TP |
| 224 | .B max\-thumbnail\-size \fR(int)\fP |
| 225 | Requests for thumbnails larger than this will automatically reduce the thumbnail to \fBmax\-thumbnail\-size\fP |
| 226 | (in pixels) in the larger dimension. Requests for thumbnails with size zero are interpreted as requests |
| 227 | -for \fBmax\-thumbnail\-size\fP. The default is 1920 pixels. |
| 228 | +for \fBmax\-thumbnail\-size\fP. |
| 229 | +The default is 1920 pixels. |
| 230 | .TP |
| 231 | .B retry\-not\-found\-hours \fR(int)\fP |
| 232 | If artwork cannot be retrieved because the remote server authoritavely confirmed that no artwork exists for |
| 233 | an artist or album, this parameter defines how long the thumbnailer waits before trying to download |
| 234 | -the same image again. The default is 168 hours (one week). |
| 235 | +the same image again. |
| 236 | +The default is 168 hours (one week). |
| 237 | .TP |
| 238 | .B retry\-error\-hours \fR(int)\fP |
| 239 | If artwork cannot be retrieved from the remote server due to an unexpected error (such as the server not |
| 240 | responding), this parameter defines how long the thumbnailer waits before trying to contact the server |
| 241 | -again (for any image). The default is two hours. |
| 242 | +again (for any image). |
| 243 | +The default is two hours. |
| 244 | .TP |
| 245 | .B max\-downloads \fR(int)\fP |
| 246 | -Controls the maximum number of concurrent downloads for remote artwork. The default value is 2. |
| 247 | +Controls the maximum number of concurrent downloads for remote artwork. |
| 248 | +The default value is 2. |
| 249 | .TP |
| 250 | .B max\-extractions \fR(int)\fP |
| 251 | Controls the maximum number of concurrent image extractions from local media files. |
| 252 | @@ -45,11 +52,17 @@ |
| 253 | .TP |
| 254 | .B max\-extraction\-timeout \fR(int)\fP |
| 255 | Sets the amount of time (in seconds) to wait for a remote image download or |
| 256 | -a thumbnail extraction before giving up. The default is 10 seconds. |
| 257 | +a thumbnail extraction before giving up. |
| 258 | +The default is 10 seconds. |
| 259 | .TP |
| 260 | .B max\-backlog \fR(int)\fP |
| 261 | Controls the number of DBus requests that will be sent before queueing the requests internally. |
| 262 | The default is 20 requests. |
| 263 | +.TP |
| 264 | +.B trace\-client \fR(bool)\fP |
| 265 | +If true, thumbnail and cancel requests are logged. Log messages are written to the calling application's log |
| 266 | +via \fBqdebug\fP(). |
| 267 | +The default value is false. |
| 268 | |
| 269 | .SH FILES |
| 270 | /usr/share/glib\-2.0/schemas/com.canonical.Unity.Thumbnailer.gschema.xml |
| 271 | |
| 272 | === modified file 'plugins/Ubuntu/Thumbnailer.0.1/thumbnailerimageresponse.cpp' |
| 273 | --- plugins/Ubuntu/Thumbnailer.0.1/thumbnailerimageresponse.cpp 2015-09-30 10:08:20 +0000 |
| 274 | +++ plugins/Ubuntu/Thumbnailer.0.1/thumbnailerimageresponse.cpp 2015-11-02 00:48:23 +0000 |
| 275 | @@ -88,7 +88,7 @@ |
| 276 | |
| 277 | void ThumbnailerImageResponse::requestFinished() |
| 278 | { |
| 279 | - if (!request_->isValid()) |
| 280 | + if (!request_->isValid() && !request_->isCancelled()) |
| 281 | { |
| 282 | qWarning() << "ThumbnailerImageResponse::dbusCallFinished(): D-Bus error: " << request_->errorMessage(); |
| 283 | } |
| 284 | |
| 285 | === modified file 'src/libthumbnailer-qt/libthumbnailer-qt.cpp' |
| 286 | --- src/libthumbnailer-qt/libthumbnailer-qt.cpp 2015-10-14 04:27:03 +0000 |
| 287 | +++ src/libthumbnailer-qt/libthumbnailer-qt.cpp 2015-11-02 00:48:23 +0000 |
| 288 | @@ -25,10 +25,10 @@ |
| 289 | #include <utils/artgeneratorcommon.h> |
| 290 | #include <service/dbus_names.h> |
| 291 | |
| 292 | +#include <QSharedPointer> |
| 293 | + |
| 294 | #include <memory> |
| 295 | |
| 296 | -#include <QSharedPointer> |
| 297 | - |
| 298 | namespace unity |
| 299 | { |
| 300 | |
| 301 | @@ -45,7 +45,8 @@ |
| 302 | { |
| 303 | Q_OBJECT |
| 304 | public: |
| 305 | - RequestImpl(QSize const& requested_size, |
| 306 | + RequestImpl(QString const& details, |
| 307 | + QSize const& requested_size, |
| 308 | RateLimiter* limiter, |
| 309 | std::function<QDBusPendingReply<QDBusUnixFileDescriptor>()> const& job); |
| 310 | |
| 311 | @@ -73,7 +74,7 @@ |
| 312 | |
| 313 | void waitForFinished() |
| 314 | { |
| 315 | - if (finished_) |
| 316 | + if (finished_ || cancelled_) |
| 317 | { |
| 318 | return; |
| 319 | } |
| 320 | @@ -83,11 +84,11 @@ |
| 321 | // In that case we send the request right here after removing it |
| 322 | // from the limiter queue. This guarantees that we always have |
| 323 | // a watcher to wait on. |
| 324 | - if (!watcher_) |
| 325 | + if (!sent_) |
| 326 | { |
| 327 | + Q_ASSERT(!watcher_); |
| 328 | cancel_func_(); |
| 329 | - watcher_.reset(new QDBusPendingCallWatcher(job_())); |
| 330 | - connect(watcher_.get(), &QDBusPendingCallWatcher::finished, this, &RequestImpl::dbusCallFinished); |
| 331 | + limiter_->schedule_now(send_request_); |
| 332 | } |
| 333 | watcher_->waitForFinished(); |
| 334 | } |
| 335 | @@ -99,21 +100,30 @@ |
| 336 | |
| 337 | void cancel(); |
| 338 | |
| 339 | + bool isCancelled() const |
| 340 | + { |
| 341 | + return cancelled_; |
| 342 | + } |
| 343 | + |
| 344 | private Q_SLOTS: |
| 345 | void dbusCallFinished(); |
| 346 | |
| 347 | private: |
| 348 | void finishWithError(QString const& errorMessage); |
| 349 | |
| 350 | + QString details_; |
| 351 | QSize requested_size_; |
| 352 | RateLimiter* limiter_; |
| 353 | std::function<QDBusPendingReply<QDBusUnixFileDescriptor>()> job_; |
| 354 | + std::function<void()> send_request_; |
| 355 | + |
| 356 | std::unique_ptr<QDBusPendingCallWatcher> watcher_; |
| 357 | - std::function<void()> cancel_func_; |
| 358 | + RateLimiter::CancelFunc cancel_func_; |
| 359 | QString error_message_; |
| 360 | bool finished_; |
| 361 | bool is_valid_; |
| 362 | - bool sent_via_limiter_; // Becomes true once rate limiter has given the request to DBus. |
| 363 | + bool cancelled_; |
| 364 | + bool sent_; // Becomes true once rate limiter has given the request to DBus. |
| 365 | QImage image_; |
| 366 | unity::thumbnailer::qt::Request* public_request_; |
| 367 | }; |
| 368 | @@ -130,42 +140,51 @@ |
| 369 | QSharedPointer<Request> getThumbnail(QString const& filename, QSize const& requestedSize); |
| 370 | |
| 371 | private: |
| 372 | - QSharedPointer<Request> createRequest(QSize const& requested_size, |
| 373 | + QSharedPointer<Request> createRequest(QString const& details, |
| 374 | + QSize const& requested_size, |
| 375 | std::function<QDBusPendingReply<QDBusUnixFileDescriptor>()> const& job); |
| 376 | std::unique_ptr<ThumbnailerInterface> iface_; |
| 377 | RateLimiter limiter_; |
| 378 | + bool trace_client_; |
| 379 | }; |
| 380 | |
| 381 | -RequestImpl::RequestImpl(QSize const& requested_size, |
| 382 | +RequestImpl::RequestImpl(QString const& details, |
| 383 | + QSize const& requested_size, |
| 384 | RateLimiter* limiter, |
| 385 | std::function<QDBusPendingReply<QDBusUnixFileDescriptor>()> const& job) |
| 386 | - : requested_size_(requested_size) |
| 387 | + : details_(details) |
| 388 | + , requested_size_(requested_size) |
| 389 | , limiter_(limiter) |
| 390 | , job_(job) |
| 391 | , finished_(false) |
| 392 | , is_valid_(false) |
| 393 | - , sent_via_limiter_(false) |
| 394 | + , cancelled_(false) |
| 395 | + , sent_(false) |
| 396 | , public_request_(nullptr) |
| 397 | { |
| 398 | - // The limiter does not call send_request until the request can be sent |
| 399 | + // The limiter does not call send_request_ until the request can be sent |
| 400 | // without exceeding max_backlog(). |
| 401 | - auto send_request = [this, job] |
| 402 | + send_request_ = [this] |
| 403 | { |
| 404 | - watcher_.reset(new QDBusPendingCallWatcher(job())); |
| 405 | + watcher_.reset(new QDBusPendingCallWatcher(job_())); |
| 406 | connect(watcher_.get(), &QDBusPendingCallWatcher::finished, this, &RequestImpl::dbusCallFinished); |
| 407 | - sent_via_limiter_ = true; |
| 408 | + sent_ = true; |
| 409 | }; |
| 410 | - cancel_func_ = limiter_->schedule(send_request); |
| 411 | + cancel_func_ = limiter_->schedule(send_request_); |
| 412 | } |
| 413 | |
| 414 | void RequestImpl::dbusCallFinished() |
| 415 | { |
| 416 | Q_ASSERT(watcher_); |
| 417 | + Q_ASSERT(sent_); |
| 418 | + Q_ASSERT(!finished_); |
| 419 | + |
| 420 | + limiter_->done(); |
| 421 | |
| 422 | QDBusPendingReply<QDBusUnixFileDescriptor> reply = *watcher_.get(); |
| 423 | if (!reply.isValid()) |
| 424 | { |
| 425 | - finishWithError("ThumbnailerRequestImpl::dbusCallFinished(): D-Bus error: " + reply.error().message()); |
| 426 | + finishWithError("Thumbnailer: RequestImpl::dbusCallFinished(): D-Bus error: " + reply.error().message()); |
| 427 | return; |
| 428 | } |
| 429 | |
| 430 | @@ -179,24 +198,21 @@ |
| 431 | watcher_.reset(); |
| 432 | Q_ASSERT(public_request_); |
| 433 | Q_EMIT public_request_->finished(); |
| 434 | - if (sent_via_limiter_) |
| 435 | + if (!details_.isEmpty()) |
| 436 | { |
| 437 | - limiter_->done(); |
| 438 | + qDebug().noquote() << "completed:" << details_; |
| 439 | } |
| 440 | - return; |
| 441 | } |
| 442 | // LCOV_EXCL_START |
| 443 | catch (const std::exception& e) |
| 444 | { |
| 445 | - finishWithError("ThumbnailerRequestImpl::dbusCallFinished(): thumbnailer failed: " + |
| 446 | + finishWithError("Thumbnailer: RequestImpl::dbusCallFinished(): thumbnailer failed: " + |
| 447 | QString::fromStdString(e.what())); |
| 448 | } |
| 449 | catch (...) |
| 450 | { |
| 451 | - finishWithError(QStringLiteral("ThumbnailerRequestImpl::dbusCallFinished(): unknown exception")); |
| 452 | + finishWithError(QStringLiteral("Thumbnailer: RequestImpl::dbusCallFinished(): unknown exception")); |
| 453 | } |
| 454 | - |
| 455 | - finishWithError(QStringLiteral("ThumbnailerRequestImpl::dbusCallFinished(): unknown error")); |
| 456 | // LCOV_EXCL_STOP |
| 457 | } |
| 458 | |
| 459 | @@ -206,34 +222,43 @@ |
| 460 | finished_ = true; |
| 461 | is_valid_ = false; |
| 462 | image_ = QImage(); |
| 463 | - qWarning() << error_message_; |
| 464 | + if (!cancelled_) |
| 465 | + { |
| 466 | + qWarning().noquote() << error_message_; // Cancellation is an expected outcome, no warning for that. |
| 467 | + } |
| 468 | + else if (!details_.isEmpty()) |
| 469 | + { |
| 470 | + qDebug().noquote() << "cancelled:" << details_; |
| 471 | + } |
| 472 | watcher_.reset(); |
| 473 | Q_ASSERT(public_request_); |
| 474 | Q_EMIT public_request_->finished(); |
| 475 | - if (sent_via_limiter_) |
| 476 | - { |
| 477 | - limiter_->done(); |
| 478 | - } |
| 479 | } |
| 480 | |
| 481 | void RequestImpl::cancel() |
| 482 | { |
| 483 | + if (!details_.isEmpty()) |
| 484 | + { |
| 485 | + qDebug().noquote() << "cancelling:" << details_; |
| 486 | + } |
| 487 | + |
| 488 | + if (finished_ || cancelled_) |
| 489 | + { |
| 490 | + return; // Too late, do nothing. |
| 491 | + } |
| 492 | + |
| 493 | cancel_func_(); |
| 494 | - |
| 495 | - if (!finished_) |
| 496 | + cancelled_ = true; |
| 497 | + if (sent_) |
| 498 | { |
| 499 | - // Deleting the pending call watcher (which should hold the only |
| 500 | - // reference to the pending call at this point) tells Qt that we |
| 501 | - // are no longer interested in the reply. The destruction will |
| 502 | - // also clear up the signal connections. |
| 503 | - watcher_.reset(); |
| 504 | - |
| 505 | - finishWithError(QStringLiteral("Request cancelled")); |
| 506 | + limiter_->done(); // Pump the limiter because finishWithError deletes the watcher. |
| 507 | } |
| 508 | + finishWithError("Request cancelled"); |
| 509 | } |
| 510 | |
| 511 | ThumbnailerImpl::ThumbnailerImpl(QDBusConnection const& connection) |
| 512 | : limiter_(Settings().max_backlog()) |
| 513 | + , trace_client_(Settings().trace_client()) |
| 514 | { |
| 515 | iface_.reset(new ThumbnailerInterface(service::BUS_NAME, service::THUMBNAILER_BUS_PATH, connection)); |
| 516 | } |
| 517 | @@ -242,38 +267,61 @@ |
| 518 | QString const& album, |
| 519 | QSize const& requestedSize) |
| 520 | { |
| 521 | + QString details; |
| 522 | + if (trace_client_) |
| 523 | + { |
| 524 | + QTextStream s(&details, QIODevice::WriteOnly); |
| 525 | + s << "getAlbumArt: (" << requestedSize.width() << "," << requestedSize.height() |
| 526 | + << ") \"" << artist << "\", \"" << album << "\""; |
| 527 | + qDebug().noquote() << details; |
| 528 | + } |
| 529 | auto job = [this, artist, album, requestedSize] |
| 530 | { |
| 531 | return iface_->GetAlbumArt(artist, album, requestedSize); |
| 532 | }; |
| 533 | - return createRequest(requestedSize, job); |
| 534 | + return createRequest(details, requestedSize, job); |
| 535 | } |
| 536 | |
| 537 | QSharedPointer<Request> ThumbnailerImpl::getArtistArt(QString const& artist, |
| 538 | QString const& album, |
| 539 | QSize const& requestedSize) |
| 540 | { |
| 541 | + QString details; |
| 542 | + if (trace_client_) |
| 543 | + { |
| 544 | + QTextStream s(&details, QIODevice::WriteOnly); |
| 545 | + s << "getArtistArt: (" << requestedSize.width() << "," << requestedSize.height() |
| 546 | + << ") \"" << artist << "\", \"" << album << "\""; |
| 547 | + qDebug().noquote() << details; |
| 548 | + } |
| 549 | auto job = [this, artist, album, requestedSize] |
| 550 | { |
| 551 | return iface_->GetArtistArt(artist, album, requestedSize); |
| 552 | }; |
| 553 | - return createRequest(requestedSize, job); |
| 554 | + return createRequest(details, requestedSize, job); |
| 555 | } |
| 556 | |
| 557 | QSharedPointer<Request> ThumbnailerImpl::getThumbnail(QString const& filename, QSize const& requestedSize) |
| 558 | { |
| 559 | + QString details; |
| 560 | + if (trace_client_) |
| 561 | + { |
| 562 | + QTextStream s(&details, QIODevice::WriteOnly); |
| 563 | + s << "getThumbnail: (" << requestedSize.width() << "," << requestedSize.height() << ") " << filename; |
| 564 | + qDebug().noquote() << details; |
| 565 | + } |
| 566 | auto job = [this, filename, requestedSize] |
| 567 | { |
| 568 | return iface_->GetThumbnail(filename, requestedSize); |
| 569 | }; |
| 570 | - return createRequest(requestedSize, job); |
| 571 | + return createRequest(details, requestedSize, job); |
| 572 | } |
| 573 | |
| 574 | -QSharedPointer<Request> ThumbnailerImpl::createRequest(QSize const& requested_size, |
| 575 | +QSharedPointer<Request> ThumbnailerImpl::createRequest(QString const& details, |
| 576 | + QSize const& requested_size, |
| 577 | std::function<QDBusPendingReply<QDBusUnixFileDescriptor>()> const& job) |
| 578 | { |
| 579 | - //std::unique_ptr<QDBusPendingCallWatcher> watcher(new QDBusPendingCallWatcher(reply)); |
| 580 | - auto request_impl = new RequestImpl(requested_size, &limiter_, job); |
| 581 | + auto request_impl = new RequestImpl(details, requested_size, &limiter_, job); |
| 582 | auto request = QSharedPointer<Request>(new Request(request_impl)); |
| 583 | request_impl->setRequest(request.data()); |
| 584 | return request; |
| 585 | @@ -318,6 +366,11 @@ |
| 586 | p_->cancel(); |
| 587 | } |
| 588 | |
| 589 | +bool Request::isCancelled() const |
| 590 | +{ |
| 591 | + return p_->isCancelled(); |
| 592 | +} |
| 593 | + |
| 594 | // LCOV_EXCL_START |
| 595 | Thumbnailer::Thumbnailer() |
| 596 | : Thumbnailer(QDBusConnection::sessionBus()) |
| 597 | |
| 598 | === modified file 'src/ratelimiter.cpp' |
| 599 | --- src/ratelimiter.cpp 2015-09-30 11:32:36 +0000 |
| 600 | +++ src/ratelimiter.cpp 2015-11-02 00:48:23 +0000 |
| 601 | @@ -38,27 +38,29 @@ |
| 602 | |
| 603 | RateLimiter::~RateLimiter() |
| 604 | { |
| 605 | - assert(running_ == 0); |
| 606 | + // No assert here because this code is linked by the calling application. |
| 607 | + // If the application terminates without waiting for outstanding requests |
| 608 | + // to finish, we don't want to cause a core dump. |
| 609 | + // assert(running_ == 0); |
| 610 | } |
| 611 | |
| 612 | -function<void() noexcept> RateLimiter::schedule(function<void()> job) |
| 613 | +RateLimiter::CancelFunc RateLimiter::schedule(function<void()> job) |
| 614 | { |
| 615 | assert(job); |
| 616 | + assert (running_ >= 0); |
| 617 | + assert (running_ <= concurrency_); |
| 618 | |
| 619 | if (running_ < concurrency_) |
| 620 | { |
| 621 | - running_++; |
| 622 | - job(); |
| 623 | - return []{}; // Wasn't queued, so cancel does nothing. |
| 624 | + return schedule_now(job); |
| 625 | } |
| 626 | |
| 627 | - shared_ptr<function<void()>> job_p(new function<void()>(move(job))); |
| 628 | - queue_.emplace(job_p); |
| 629 | + queue_.emplace(make_shared<function<void()>>(move(job))); |
| 630 | |
| 631 | - // Returned function clears job when called, provided the job is still in the queue. |
| 632 | + // Returned function clears the job when called, provided the job is still in the queue. |
| 633 | // done() removes any cleared jobs from the queue without calling them. |
| 634 | - weak_ptr<function<void()>> weak_p(job_p); |
| 635 | - return [weak_p]() noexcept |
| 636 | + weak_ptr<function<void()>> weak_p(queue_.back()); |
| 637 | + return [this, weak_p]() noexcept |
| 638 | { |
| 639 | auto job_p = weak_p.lock(); |
| 640 | if (job_p) |
| 641 | @@ -68,6 +70,15 @@ |
| 642 | }; |
| 643 | } |
| 644 | |
| 645 | +RateLimiter::CancelFunc RateLimiter::schedule_now(function<void()> job) |
| 646 | +{ |
| 647 | + assert(job); |
| 648 | + |
| 649 | + running_++; |
| 650 | + job(); |
| 651 | + return []{}; // Wasn't queued, so cancel does nothing. |
| 652 | +} |
| 653 | + |
| 654 | void RateLimiter::done() |
| 655 | { |
| 656 | // Find the next job, discarding any cancelled jobs. |
| 657 | @@ -75,8 +86,9 @@ |
| 658 | while (!queue_.empty()) |
| 659 | { |
| 660 | job_p = queue_.front(); |
| 661 | + assert(job_p); |
| 662 | queue_.pop(); |
| 663 | - if (*job_p) |
| 664 | + if (*job_p != nullptr) |
| 665 | { |
| 666 | break; |
| 667 | } |
| 668 | |
| 669 | === modified file 'src/service/dbusinterface.cpp' |
| 670 | --- src/service/dbusinterface.cpp 2015-09-22 02:29:52 +0000 |
| 671 | +++ src/service/dbusinterface.cpp 2015-11-02 00:48:23 +0000 |
| 672 | @@ -135,7 +135,7 @@ |
| 673 | , inactivity_handler_(inactivity_handler) |
| 674 | , check_thread_pool_(make_shared<QThreadPool>()) |
| 675 | , create_thread_pool_(make_shared<QThreadPool>()) |
| 676 | - , download_limiter_(settings_.max_downloads()) |
| 677 | + , download_limiter_(make_shared<RateLimiter>(settings_.max_downloads())) |
| 678 | { |
| 679 | auto limit = settings_.max_extractions(); |
| 680 | |
| 681 | @@ -154,7 +154,7 @@ |
| 682 | } |
| 683 | } |
| 684 | |
| 685 | - extraction_limiter_.reset(new RateLimiter(limit)); |
| 686 | + extraction_limiter_ = make_shared<RateLimiter>(limit); |
| 687 | } |
| 688 | |
| 689 | DBusInterface::~DBusInterface() |
| 690 | @@ -236,7 +236,7 @@ |
| 691 | auto request = thumbnailer_->get_thumbnail(filename.toStdString(), requestedSize); |
| 692 | queueRequest(new Handler(connection(), message(), |
| 693 | check_thread_pool_, create_thread_pool_, |
| 694 | - *extraction_limiter_, credentials(), inactivity_handler_, |
| 695 | + extraction_limiter_, credentials(), inactivity_handler_, |
| 696 | std::move(request), details)); |
| 697 | } |
| 698 | catch (exception const& e) |
| 699 | |
| 700 | === modified file 'src/service/dbusinterface.h' |
| 701 | --- src/service/dbusinterface.h 2015-09-15 00:47:25 +0000 |
| 702 | +++ src/service/dbusinterface.h 2015-11-02 00:48:23 +0000 |
| 703 | @@ -75,8 +75,8 @@ |
| 704 | std::map<Handler*, std::unique_ptr<Handler>> requests_; |
| 705 | std::map<std::string, std::vector<Handler*>> request_keys_; |
| 706 | unity::thumbnailer::Settings settings_; |
| 707 | - RateLimiter download_limiter_; |
| 708 | - std::unique_ptr<RateLimiter> extraction_limiter_; |
| 709 | + std::shared_ptr<RateLimiter> download_limiter_; |
| 710 | + std::shared_ptr<RateLimiter> extraction_limiter_; |
| 711 | }; |
| 712 | |
| 713 | } // namespace service |
| 714 | |
| 715 | === modified file 'src/service/handler.cpp' |
| 716 | --- src/service/handler.cpp 2015-09-29 14:05:53 +0000 |
| 717 | +++ src/service/handler.cpp 2015-11-02 00:48:23 +0000 |
| 718 | @@ -119,7 +119,7 @@ |
| 719 | QDBusMessage const message; |
| 720 | shared_ptr<QThreadPool> check_pool; |
| 721 | shared_ptr<QThreadPool> create_pool; |
| 722 | - RateLimiter& limiter; |
| 723 | + shared_ptr<RateLimiter> limiter; |
| 724 | CredentialsCache& creds; |
| 725 | InactivityHandler& inactivity_handler; |
| 726 | unique_ptr<ThumbnailRequest> request; |
| 727 | @@ -139,7 +139,7 @@ |
| 728 | QDBusMessage const& message, |
| 729 | shared_ptr<QThreadPool> const& check_pool, |
| 730 | shared_ptr<QThreadPool> const& create_pool, |
| 731 | - RateLimiter& limiter, |
| 732 | + shared_ptr<RateLimiter> const& limiter, |
| 733 | CredentialsCache& creds, |
| 734 | InactivityHandler& inactivity_handler, |
| 735 | unique_ptr<ThumbnailRequest>&& request, |
| 736 | @@ -162,7 +162,7 @@ |
| 737 | QDBusMessage const& message, |
| 738 | shared_ptr<QThreadPool> const& check_pool, |
| 739 | shared_ptr<QThreadPool> const& create_pool, |
| 740 | - RateLimiter& limiter, |
| 741 | + shared_ptr<RateLimiter> const& limiter, |
| 742 | CredentialsCache& creds, |
| 743 | InactivityHandler& inactivity_handler, |
| 744 | unique_ptr<ThumbnailRequest>&& request, |
| 745 | @@ -227,7 +227,7 @@ |
| 746 | { |
| 747 | try |
| 748 | { |
| 749 | - return FdOrError{check(), nullptr}; |
| 750 | + return p->cancelled ? FdOrError{QDBusUnixFileDescriptor(), "gotCredentials(): cancelled"} : FdOrError{check(), nullptr}; |
| 751 | } |
| 752 | // LCOV_EXCL_START |
| 753 | catch (std::exception const& e) |
| 754 | @@ -249,6 +249,11 @@ |
| 755 | |
| 756 | QDBusUnixFileDescriptor Handler::check() |
| 757 | { |
| 758 | + if (p->cancelled) |
| 759 | + { |
| 760 | + return QDBusUnixFileDescriptor(); |
| 761 | + } |
| 762 | + |
| 763 | string art_image = p->request->thumbnail(); |
| 764 | |
| 765 | if (art_image.empty()) |
| 766 | @@ -273,7 +278,7 @@ |
| 767 | // LCOV_EXCL_START |
| 768 | catch (std::exception const& e) |
| 769 | { |
| 770 | - sendError("Handler::checkFinished(): result error: " + details() + ": " + e.what()); |
| 771 | + sendError("Handler::checkFinished(): exception: " + details() + ": " + e.what()); |
| 772 | return; |
| 773 | } |
| 774 | if (!fd_error.error.isNull()) |
| 775 | @@ -296,7 +301,7 @@ |
| 776 | { |
| 777 | // otherwise move on to the download phase. |
| 778 | p->schedule_start_time = chrono::system_clock::now(); |
| 779 | - p->limiter.schedule([&]{ p->download_start_time = chrono::system_clock::now(); p->request->download(); }); |
| 780 | + p->limiter->schedule([&]{ p->download_start_time = chrono::system_clock::now(); p->request->download(); }); |
| 781 | } |
| 782 | // LCOV_EXCL_START |
| 783 | catch (std::exception const& e) |
| 784 | @@ -307,13 +312,13 @@ |
| 785 | return; |
| 786 | } |
| 787 | |
| 788 | - sendError("Handler::check_finished(): no artwork for " + details() + ": " + status()); |
| 789 | + sendError("Handler::checkFinished(): no artwork for " + details() + ": " + status()); |
| 790 | } |
| 791 | |
| 792 | void Handler::downloadFinished() |
| 793 | { |
| 794 | p->download_finish_time = chrono::system_clock::now(); |
| 795 | - p->limiter.done(); |
| 796 | + p->limiter->done(); |
| 797 | |
| 798 | if (p->cancelled) |
| 799 | { |
| 800 | @@ -324,7 +329,7 @@ |
| 801 | { |
| 802 | try |
| 803 | { |
| 804 | - return FdOrError{create(), nullptr}; |
| 805 | + return p->cancelled ? FdOrError{QDBusUnixFileDescriptor(), nullptr} : FdOrError{create(), nullptr}; |
| 806 | } |
| 807 | catch (std::exception const& e) |
| 808 | { |
| 809 | @@ -341,6 +346,11 @@ |
| 810 | |
| 811 | QDBusUnixFileDescriptor Handler::create() |
| 812 | { |
| 813 | + if (p->cancelled) |
| 814 | + { |
| 815 | + return QDBusUnixFileDescriptor(); |
| 816 | + } |
| 817 | + |
| 818 | string art_image = p->request->thumbnail(); |
| 819 | |
| 820 | if (art_image.empty()) |
| 821 | |
| 822 | === modified file 'src/service/handler.h' |
| 823 | --- src/service/handler.h 2015-09-29 14:05:53 +0000 |
| 824 | +++ src/service/handler.h 2015-11-02 00:48:23 +0000 |
| 825 | @@ -53,7 +53,7 @@ |
| 826 | QDBusMessage const& message, |
| 827 | std::shared_ptr<QThreadPool> const& check_pool, |
| 828 | std::shared_ptr<QThreadPool> const& create_pool, |
| 829 | - RateLimiter& limiter, |
| 830 | + std::shared_ptr<RateLimiter> const& limiter, |
| 831 | CredentialsCache& creds, |
| 832 | InactivityHandler& inactivity_handler, |
| 833 | std::unique_ptr<internal::ThumbnailRequest>&& request, |
| 834 | |
| 835 | === modified file 'src/settings.cpp' |
| 836 | --- src/settings.cpp 2015-08-03 03:35:38 +0000 |
| 837 | +++ src/settings.cpp 2015-11-02 00:48:23 +0000 |
| 838 | @@ -115,6 +115,11 @@ |
| 839 | return get_positive_int("max-backlog", MAX_BACKLOG_DEFAULT); |
| 840 | } |
| 841 | |
| 842 | +bool Settings::trace_client() const |
| 843 | +{ |
| 844 | + return get_bool("trace-client", TRACE_CLIENT_DEFAULT); |
| 845 | +} |
| 846 | + |
| 847 | string Settings::get_string(char const* key, string const& default_value) const |
| 848 | { |
| 849 | if (!settings_ || !g_settings_schema_has_key(schema_.get(), key)) |
| 850 | @@ -164,6 +169,15 @@ |
| 851 | return g_settings_get_int(settings_.get(), key); |
| 852 | } |
| 853 | |
| 854 | +bool Settings::get_bool(char const* key, bool default_value) const |
| 855 | +{ |
| 856 | + if (!settings_ || !g_settings_schema_has_key(schema_.get(), key)) |
| 857 | + { |
| 858 | + return default_value; |
| 859 | + } |
| 860 | + |
| 861 | + return g_settings_get_boolean(settings_.get(), key); |
| 862 | +} |
| 863 | |
| 864 | } // namespace thumbnailer |
| 865 | |
| 866 | |
| 867 | === modified file 'src/thumbnailer.cpp' |
| 868 | --- src/thumbnailer.cpp 2015-10-22 03:00:37 +0000 |
| 869 | +++ src/thumbnailer.cpp 2015-11-02 00:48:23 +0000 |
| 870 | @@ -868,12 +868,33 @@ |
| 871 | return v; |
| 872 | } |
| 873 | |
| 874 | +namespace |
| 875 | +{ |
| 876 | + |
| 877 | +char const* cache_name(Thumbnailer::CacheSelector selector) |
| 878 | +{ |
| 879 | + switch (selector) |
| 880 | + { |
| 881 | + case Thumbnailer::CacheSelector::full_size_cache: |
| 882 | + return "image cache"; |
| 883 | + case Thumbnailer::CacheSelector::thumbnail_cache: |
| 884 | + return "thumbnail cache"; |
| 885 | + case Thumbnailer::CacheSelector::failure_cache: |
| 886 | + return "failure cache"; |
| 887 | + default: |
| 888 | + return "all caches"; |
| 889 | + } |
| 890 | +} |
| 891 | + |
| 892 | +} |
| 893 | + |
| 894 | void Thumbnailer::clear_stats(CacheSelector selector) |
| 895 | { |
| 896 | for (auto c : select_caches(selector)) |
| 897 | { |
| 898 | c->clear_stats(); |
| 899 | } |
| 900 | + qDebug() << "reset statistics for" << cache_name(selector); |
| 901 | } |
| 902 | |
| 903 | void Thumbnailer::clear(CacheSelector selector) |
| 904 | @@ -888,14 +909,17 @@ |
| 905 | // are still within the timeout period. |
| 906 | nw_fail_time_ = chrono::system_clock::time_point(); |
| 907 | } |
| 908 | + qDebug() << "cleared" << cache_name(selector); |
| 909 | } |
| 910 | |
| 911 | void Thumbnailer::compact(CacheSelector selector) |
| 912 | { |
| 913 | + qDebug() << "compacting" << cache_name(selector); |
| 914 | for (auto c : select_caches(selector)) |
| 915 | { |
| 916 | c->compact(); |
| 917 | } |
| 918 | + qDebug() << "completed compacting" << cache_name(selector); |
| 919 | } |
| 920 | |
| 921 | } // namespace internal |
| 922 | |
| 923 | === modified file 'src/vs-thumb/thumbnailextractor.cpp' |
| 924 | --- src/vs-thumb/thumbnailextractor.cpp 2015-10-14 03:57:29 +0000 |
| 925 | +++ src/vs-thumb/thumbnailextractor.cpp 2015-11-02 00:48:23 +0000 |
| 926 | @@ -342,7 +342,7 @@ |
| 927 | { |
| 928 | // LCOV_EXCL_START |
| 929 | g_set_error(error, G_FILE_ERROR, g_file_error_from_errno(errno), |
| 930 | - "cannot write image data %s", g_strerror(errno)); |
| 931 | + "cannot write image data: %s", errno == 0 ? "short write" : g_strerror(errno)); |
| 932 | return false; |
| 933 | // LCOV_EXCL_STOP |
| 934 | } |
| 935 | |
| 936 | === modified file 'tests/libthumbnailer-qt/CMakeLists.txt' |
| 937 | --- tests/libthumbnailer-qt/CMakeLists.txt 2015-09-21 04:53:32 +0000 |
| 938 | +++ tests/libthumbnailer-qt/CMakeLists.txt 2015-11-02 00:48:23 +0000 |
| 939 | @@ -1,4 +1,5 @@ |
| 940 | add_executable(libthumbnailer-qt_test libthumbnailer-qt_test.cpp) |
| 941 | +set_target_properties(libthumbnailer-qt_test PROPERTIES AUTOMOC TRUE) |
| 942 | |
| 943 | qt5_use_modules(libthumbnailer-qt_test DBus Gui Test Network) |
| 944 | target_link_libraries(libthumbnailer-qt_test |
| 945 | @@ -8,6 +9,7 @@ |
| 946 | Qt5::Test |
| 947 | gtest |
| 948 | testutils |
| 949 | + ${GST_DEPS_LDFLAGS} |
| 950 | ) |
| 951 | add_test(libthumbnailer-qt libthumbnailer-qt_test) |
| 952 | add_dependencies(libthumbnailer-qt_test thumbnailer-qt) |
| 953 | |
| 954 | === modified file 'tests/libthumbnailer-qt/libthumbnailer-qt_test.cpp' |
| 955 | --- tests/libthumbnailer-qt/libthumbnailer-qt_test.cpp 2015-09-22 03:36:53 +0000 |
| 956 | +++ tests/libthumbnailer-qt/libthumbnailer-qt_test.cpp 2015-11-02 00:48:23 +0000 |
| 957 | @@ -18,10 +18,26 @@ |
| 958 | |
| 959 | #include <unity/thumbnailer/qt/thumbnailer-qt.h> |
| 960 | |
| 961 | +#include <internal/file_io.h> |
| 962 | #include <utils/artserver.h> |
| 963 | #include <utils/dbusserver.h> |
| 964 | +#include <utils/supports_decoder.h> |
| 965 | + |
| 966 | +#pragma GCC diagnostic push |
| 967 | +#pragma GCC diagnostic ignored "-Wold-style-cast" |
| 968 | +#pragma GCC diagnostic ignored "-Wcast-qual" |
| 969 | +#pragma GCC diagnostic ignored "-Wcast-align" |
| 970 | +#ifdef __clang__ |
| 971 | +#pragma clang diagnostic ignored "-Wparentheses-equality" |
| 972 | +#endif |
| 973 | +#include <gst/gst.h> |
| 974 | +#ifdef __clang__ |
| 975 | +#pragma clang diagnostic pop |
| 976 | +#endif |
| 977 | +#pragma GCC diagnostic pop |
| 978 | |
| 979 | #include <boost/algorithm/string/predicate.hpp> |
| 980 | +#include <boost/filesystem.hpp> |
| 981 | #include <gtest/gtest.h> |
| 982 | |
| 983 | #include <QSignalSpy> |
| 984 | @@ -33,6 +49,7 @@ |
| 985 | |
| 986 | using namespace std; |
| 987 | using namespace unity::thumbnailer::qt; |
| 988 | +using namespace unity::thumbnailer::internal; |
| 989 | |
| 990 | namespace |
| 991 | { |
| 992 | @@ -57,11 +74,11 @@ |
| 993 | art_server_.reset(new ArtServer()); |
| 994 | |
| 995 | // start dbus service |
| 996 | - tempdir.reset(new QTemporaryDir(TESTBINDIR "/dbus-test.XXXXXX")); |
| 997 | + tempdir.reset(new QTemporaryDir(TESTBINDIR "/libthumbnailer-qt.XXXXXX")); |
| 998 | setenv("XDG_CACHE_HOME", (tempdir->path() + "/cache").toUtf8().data(), true); |
| 999 | |
| 1000 | - // set 3 seconds as max idle time |
| 1001 | - setenv("THUMBNAILER_MAX_IDLE", "3000", true); |
| 1002 | + // set 10 seconds as max idle time |
| 1003 | + setenv("THUMBNAILER_MAX_IDLE", "10000", true); |
| 1004 | |
| 1005 | dbus_.reset(new DBusServer()); |
| 1006 | } |
| 1007 | @@ -420,27 +437,181 @@ |
| 1008 | EXPECT_FALSE(reply->isValid()); |
| 1009 | } |
| 1010 | |
| 1011 | -TEST_F(LibThumbnailerTest, cancel_request) |
| 1012 | -{ |
| 1013 | +void make_links(string const& source_path, string const& target_dir, int num_copies) |
| 1014 | +{ |
| 1015 | + using namespace boost::filesystem; |
| 1016 | + |
| 1017 | + assert(num_copies > 0); |
| 1018 | + assert(num_copies <= 2000); // Probably not good to put more files than this into one directory |
| 1019 | + |
| 1020 | + string filename = path(source_path).filename().native(); |
| 1021 | + string new_name = "0" + filename; |
| 1022 | + |
| 1023 | + // Make initial copy |
| 1024 | + string copied_file = target_dir + "/" + new_name; |
| 1025 | + string contents = read_file(source_path); |
| 1026 | + write_file(copied_file, contents); |
| 1027 | + |
| 1028 | + // Make num_copies - 1 links to the copy. |
| 1029 | + for (int i = 1; i < num_copies; ++i) |
| 1030 | + { |
| 1031 | + string link_name = target_dir + "/" + to_string(i) + filename; |
| 1032 | + EXPECT_TRUE(link(copied_file.c_str(), link_name.c_str()) == 0 || errno == EEXIST) << "errno = " << errno; |
| 1033 | + } |
| 1034 | +} |
| 1035 | + |
| 1036 | +// Simple counter class that we use to count all the signals |
| 1037 | +// from the async provider instances. This allows us to use |
| 1038 | +// a single QSignalSpy to wait for the completion of an |
| 1039 | +// arbitrary number of requests. |
| 1040 | + |
| 1041 | +class Counter : public QObject |
| 1042 | +{ |
| 1043 | + Q_OBJECT |
| 1044 | +public: |
| 1045 | + Counter(int limit) |
| 1046 | + : limit_(limit) |
| 1047 | + , count_(0) |
| 1048 | + , cancellations_(0) |
| 1049 | + { |
| 1050 | + } |
| 1051 | + |
| 1052 | + int cancellations() |
| 1053 | + { |
| 1054 | + return cancellations_; |
| 1055 | + } |
| 1056 | + |
| 1057 | +Q_SIGNALS: |
| 1058 | + void counterDone(); |
| 1059 | + |
| 1060 | +public Q_SLOTS: |
| 1061 | + void thumbnailComplete(bool cancelled) |
| 1062 | + { |
| 1063 | + if (cancelled) |
| 1064 | + { |
| 1065 | + ++cancellations_; |
| 1066 | + } |
| 1067 | + if (++count_ == limit_) |
| 1068 | + { |
| 1069 | + Q_EMIT counterDone(); |
| 1070 | + } |
| 1071 | + } |
| 1072 | + |
| 1073 | +private: |
| 1074 | + int limit_; |
| 1075 | + int count_; |
| 1076 | + int cancellations_; |
| 1077 | +}; |
| 1078 | + |
| 1079 | +// Class to run a single request and check that it completed OK. |
| 1080 | +// Notifies the counter on request completion. |
| 1081 | + |
| 1082 | +class AsyncThumbnailProvider : public QObject |
| 1083 | +{ |
| 1084 | + Q_OBJECT |
| 1085 | +public: |
| 1086 | + AsyncThumbnailProvider(unity::thumbnailer::qt::Thumbnailer* tn, Counter& counter) |
| 1087 | + : thumbnailer_(tn) |
| 1088 | + , counter_(counter) |
| 1089 | + { |
| 1090 | + assert(tn); |
| 1091 | + } |
| 1092 | + |
| 1093 | + void getThumbnail(QString const& path, QSize const& size) |
| 1094 | + { |
| 1095 | + request_ = thumbnailer_->getThumbnail(path, size); |
| 1096 | + connect(request_.data(), &unity::thumbnailer::qt::Request::finished, |
| 1097 | + this, &AsyncThumbnailProvider::requestFinished); |
| 1098 | + } |
| 1099 | + |
| 1100 | + void getAlbumArt(QString const& artist, QString const& album, QSize const& size) |
| 1101 | + { |
| 1102 | + request_ = thumbnailer_->getAlbumArt(artist, album, size); |
| 1103 | + connect(request_.data(), &unity::thumbnailer::qt::Request::finished, |
| 1104 | + this, &AsyncThumbnailProvider::requestFinished); |
| 1105 | + } |
| 1106 | + |
| 1107 | + void cancel() |
| 1108 | + { |
| 1109 | + request_->cancel(); |
| 1110 | + } |
| 1111 | + |
| 1112 | + void waitForFinished() |
| 1113 | + { |
| 1114 | + request_->waitForFinished(); |
| 1115 | + } |
| 1116 | + |
| 1117 | +public Q_SLOTS: |
| 1118 | + void requestFinished() |
| 1119 | + { |
| 1120 | + EXPECT_TRUE(request_->isFinished()); |
| 1121 | + if (!request_->isValid()) |
| 1122 | + { |
| 1123 | + EXPECT_TRUE(request_->isCancelled()); |
| 1124 | + EXPECT_TRUE(request_->image().isNull()); |
| 1125 | + EXPECT_TRUE(request_->errorMessage() == QString("Request cancelled")) |
| 1126 | + << request_->errorMessage().toStdString(); |
| 1127 | + } |
| 1128 | + counter_.thumbnailComplete(request_->isCancelled()); |
| 1129 | + } |
| 1130 | + |
| 1131 | +private: |
| 1132 | + unity::thumbnailer::qt::Thumbnailer* thumbnailer_; |
| 1133 | + Counter& counter_; |
| 1134 | + QSharedPointer<unity::thumbnailer::qt::Request> request_; |
| 1135 | +}; |
| 1136 | + |
| 1137 | +void pump(int millisecs) |
| 1138 | +{ |
| 1139 | + QTimer timer; |
| 1140 | + QSignalSpy timer_spy(&timer, &QTimer::timeout); |
| 1141 | + timer.start(millisecs); |
| 1142 | + EXPECT_TRUE(timer_spy.wait(millisecs + 1000)); |
| 1143 | +} |
| 1144 | + |
| 1145 | +TEST_F(LibThumbnailerTest, cancel) |
| 1146 | +{ |
| 1147 | + if (!supports_decoder("audio/mpeg")) |
| 1148 | + { |
| 1149 | + fprintf(stderr, "No support for MP3 decoder\n"); |
| 1150 | + return; |
| 1151 | + } |
| 1152 | + |
| 1153 | Thumbnailer thumbnailer(dbus_->connection()); |
| 1154 | |
| 1155 | - auto reply = thumbnailer.getAlbumArt("metallica", "load", QSize(48, 48)); |
| 1156 | - |
| 1157 | - QSignalSpy spy(reply.data(), &Request::finished); |
| 1158 | - reply->cancel(); |
| 1159 | - |
| 1160 | - // check that we've got exactly one signal |
| 1161 | - ASSERT_EQ(spy.count(), 1); |
| 1162 | - |
| 1163 | - EXPECT_TRUE(reply->isFinished()); |
| 1164 | - EXPECT_FALSE(reply->isValid()); |
| 1165 | - EXPECT_EQ(reply->errorMessage(), "Request cancelled"); |
| 1166 | + string source = "short-track.mp3"; |
| 1167 | + string target_dir = temp_dir(); |
| 1168 | + make_links(string(TESTDATADIR) + "/" + source, target_dir, 1); |
| 1169 | + |
| 1170 | + QString path; |
| 1171 | + |
| 1172 | + Counter counter(2); |
| 1173 | + QSignalSpy spy(&counter, &Counter::counterDone); |
| 1174 | + |
| 1175 | + unique_ptr<AsyncThumbnailProvider> provider(new AsyncThumbnailProvider(&thumbnailer, counter)); |
| 1176 | + path = QString::fromStdString(target_dir + "/0" + source); |
| 1177 | + provider->getThumbnail(path, QSize(512, 512)); |
| 1178 | + |
| 1179 | + pump(500); |
| 1180 | + |
| 1181 | + provider->cancel(); |
| 1182 | + provider->waitForFinished(); // For coverage |
| 1183 | + |
| 1184 | + pump(500); |
| 1185 | + |
| 1186 | + provider.reset(new AsyncThumbnailProvider(&thumbnailer, counter)); |
| 1187 | + provider->getThumbnail(path, QSize(512, 512)); |
| 1188 | + |
| 1189 | + EXPECT_TRUE(spy.wait(1000)); |
| 1190 | + EXPECT_EQ(1, spy.count()); |
| 1191 | } |
| 1192 | |
| 1193 | Q_DECLARE_METATYPE(QProcess::ExitStatus) // Avoid noise from signal spy. |
| 1194 | |
| 1195 | int main(int argc, char** argv) |
| 1196 | { |
| 1197 | + gst_init(&argc, &argv); |
| 1198 | + |
| 1199 | QCoreApplication app(argc, argv); |
| 1200 | qRegisterMetaType<QProcess::ExitStatus>("QProcess::ExitStatus"); // Avoid noise from signal spy. |
| 1201 | qDBusRegisterMetaType<unity::thumbnailer::service::AllStats>(); |
| 1202 | @@ -451,3 +622,5 @@ |
| 1203 | ::testing::InitGoogleTest(&argc, argv); |
| 1204 | return RUN_ALL_TESTS(); |
| 1205 | } |
| 1206 | + |
| 1207 | +#include "libthumbnailer-qt_test.moc" |
| 1208 | |
| 1209 | === added file 'tests/media/testsong_ogg' |
| 1210 | Binary files tests/media/testsong_ogg 1970-01-01 00:00:00 +0000 and tests/media/testsong_ogg 2015-11-02 00:48:23 +0000 differ |
| 1211 | === modified file 'tests/stress/stress_test.cpp' |
| 1212 | --- tests/stress/stress_test.cpp 2015-10-14 04:27:03 +0000 |
| 1213 | +++ tests/stress/stress_test.cpp 2015-11-02 00:48:23 +0000 |
| 1214 | @@ -57,15 +57,25 @@ |
| 1215 | Counter(int limit) |
| 1216 | : limit_(limit) |
| 1217 | , count_(0) |
| 1218 | - { |
| 1219 | + , cancellations_(0) |
| 1220 | + { |
| 1221 | + } |
| 1222 | + |
| 1223 | + int cancellations() |
| 1224 | + { |
| 1225 | + return cancellations_; |
| 1226 | } |
| 1227 | |
| 1228 | Q_SIGNALS: |
| 1229 | void counterDone(); |
| 1230 | |
| 1231 | public Q_SLOTS: |
| 1232 | - void thumbnailComplete() |
| 1233 | + void thumbnailComplete(bool cancelled) |
| 1234 | { |
| 1235 | + if (cancelled) |
| 1236 | + { |
| 1237 | + ++cancellations_; |
| 1238 | + } |
| 1239 | if (++count_ == limit_) |
| 1240 | { |
| 1241 | Q_EMIT counterDone(); |
| 1242 | @@ -75,6 +85,7 @@ |
| 1243 | private: |
| 1244 | int limit_; |
| 1245 | int count_; |
| 1246 | + int cancellations_; |
| 1247 | }; |
| 1248 | |
| 1249 | // Class to run a single request and check that it completed OK. |
| 1250 | @@ -105,15 +116,25 @@ |
| 1251 | this, &AsyncThumbnailProvider::requestFinished); |
| 1252 | } |
| 1253 | |
| 1254 | + void cancel() |
| 1255 | + { |
| 1256 | + request_->cancel(); |
| 1257 | + } |
| 1258 | + |
| 1259 | + void waitForFinished() |
| 1260 | + { |
| 1261 | + request_->waitForFinished(); |
| 1262 | + } |
| 1263 | + |
| 1264 | public Q_SLOTS: |
| 1265 | void requestFinished() |
| 1266 | { |
| 1267 | - EXPECT_TRUE(request_->isValid()) << request_->errorMessage().toStdString(); |
| 1268 | + EXPECT_TRUE(request_->isFinished()); |
| 1269 | if (!request_->isValid()) |
| 1270 | { |
| 1271 | - abort(); |
| 1272 | + EXPECT_TRUE(request_->image().isNull()); |
| 1273 | } |
| 1274 | - counter_.thumbnailComplete(); |
| 1275 | + counter_.thumbnailComplete(request_->isCancelled()); |
| 1276 | } |
| 1277 | |
| 1278 | private: |
| 1279 | @@ -128,11 +149,10 @@ |
| 1280 | StressTest() |
| 1281 | { |
| 1282 | } |
| 1283 | - virtual ~StressTest() |
| 1284 | - { |
| 1285 | - } |
| 1286 | - |
| 1287 | - static void SetUpTestCase() |
| 1288 | + |
| 1289 | + virtual ~StressTest() = default; |
| 1290 | + |
| 1291 | + void SetUp() override |
| 1292 | { |
| 1293 | // start fake server |
| 1294 | art_server_.reset(new ArtServer()); |
| 1295 | @@ -153,7 +173,7 @@ |
| 1296 | ASSERT_EQ(0, mkdir((temp_dir() + "/Pictures").c_str(), 0700)); |
| 1297 | } |
| 1298 | |
| 1299 | - static string temp_dir() |
| 1300 | + string temp_dir() |
| 1301 | { |
| 1302 | return tempdir->path().toStdString(); |
| 1303 | } |
| 1304 | @@ -172,8 +192,8 @@ |
| 1305 | provider->getThumbnail(path, QSize(512, 512)); |
| 1306 | providers.emplace_back(move(provider)); |
| 1307 | } |
| 1308 | - ASSERT_TRUE(spy.wait(120000)); |
| 1309 | - ASSERT_EQ(1, spy.count()); |
| 1310 | + EXPECT_TRUE(spy.wait(120000)); |
| 1311 | + EXPECT_EQ(1, spy.count()); |
| 1312 | } |
| 1313 | |
| 1314 | static void add_stats(int N_REQUESTS, |
| 1315 | @@ -197,7 +217,7 @@ |
| 1316 | cout << stats_; |
| 1317 | } |
| 1318 | |
| 1319 | - static void TearDownTestCase() |
| 1320 | + void TearDown() override |
| 1321 | { |
| 1322 | thumbnailer_.reset(); |
| 1323 | dbus_.reset(); |
| 1324 | @@ -210,17 +230,13 @@ |
| 1325 | show_stats(); |
| 1326 | } |
| 1327 | |
| 1328 | - static unique_ptr<QTemporaryDir> tempdir; |
| 1329 | - static unique_ptr<DBusServer> dbus_; |
| 1330 | - static unique_ptr<unity::thumbnailer::qt::Thumbnailer> thumbnailer_; |
| 1331 | - static unique_ptr<ArtServer> art_server_; |
| 1332 | + unique_ptr<QTemporaryDir> tempdir; |
| 1333 | + unique_ptr<DBusServer> dbus_; |
| 1334 | + unique_ptr<unity::thumbnailer::qt::Thumbnailer> thumbnailer_; |
| 1335 | + unique_ptr<ArtServer> art_server_; |
| 1336 | static string stats_; |
| 1337 | }; |
| 1338 | |
| 1339 | -unique_ptr<QTemporaryDir> StressTest::tempdir; |
| 1340 | -unique_ptr<DBusServer> StressTest::dbus_; |
| 1341 | -unique_ptr<unity::thumbnailer::qt::Thumbnailer> StressTest::thumbnailer_; |
| 1342 | -unique_ptr<ArtServer> StressTest::art_server_; |
| 1343 | string StressTest::stats_; |
| 1344 | |
| 1345 | // Little helper function to hard-link a single file a number of times |
| 1346 | @@ -246,7 +262,7 @@ |
| 1347 | for (int i = 1; i < num_copies; ++i) |
| 1348 | { |
| 1349 | string link_name = target_dir + "/" + to_string(i) + filename; |
| 1350 | - ASSERT_TRUE(link(copied_file.c_str(), link_name.c_str()) == 0 || errno == EEXIST) << "errno = " << errno; |
| 1351 | + EXPECT_TRUE(link(copied_file.c_str(), link_name.c_str()) == 0 || errno == EEXIST) << "errno = " << errno; |
| 1352 | } |
| 1353 | } |
| 1354 | |
| 1355 | @@ -376,12 +392,82 @@ |
| 1356 | add_stats(N_REQUESTS, start, finish); |
| 1357 | } |
| 1358 | |
| 1359 | +TEST_F(StressTest, wait_for_finished_in_queue) |
| 1360 | +{ |
| 1361 | + if (!supports_decoder("audio/mpeg")) |
| 1362 | + { |
| 1363 | + fprintf(stderr, "No support for MP3 decoder\n"); |
| 1364 | + return; |
| 1365 | + } |
| 1366 | + |
| 1367 | + int const N_REQUESTS = 200; |
| 1368 | + |
| 1369 | + string source = "short-track.mp3"; |
| 1370 | + string target_dir = temp_dir() + "/Music"; |
| 1371 | + make_links(string(TESTDATADIR) + "/" + source, target_dir, N_REQUESTS); |
| 1372 | + |
| 1373 | + vector<unique_ptr<AsyncThumbnailProvider>> providers; |
| 1374 | + |
| 1375 | + Counter counter(N_REQUESTS); |
| 1376 | + QSignalSpy spy(&counter, &Counter::counterDone); |
| 1377 | + |
| 1378 | + auto start = chrono::system_clock::now(); |
| 1379 | + for (int i = 0; i < N_REQUESTS; i++) |
| 1380 | + { |
| 1381 | + unique_ptr<AsyncThumbnailProvider> provider(new AsyncThumbnailProvider(thumbnailer_.get(), counter)); |
| 1382 | + QString path = QString::fromStdString(target_dir + "/" + to_string(i) + source); |
| 1383 | + provider->getThumbnail(path, QSize(512, 512)); |
| 1384 | + providers.emplace_back(move(provider)); |
| 1385 | + } |
| 1386 | + |
| 1387 | + // Pump the event loop for a while, to allow some requests to start processing. |
| 1388 | + { |
| 1389 | + QTimer timer; |
| 1390 | + QSignalSpy timer_spy(&timer, &QTimer::timeout); |
| 1391 | + timer.start(); |
| 1392 | + timer_spy.wait(1000); |
| 1393 | + } |
| 1394 | + |
| 1395 | + // For coverage: Wait for a few requests while they are still in the queue (which will cause |
| 1396 | + // them to be scheduled immediately.) |
| 1397 | + providers[N_REQUESTS - 7]->waitForFinished(); |
| 1398 | + providers[N_REQUESTS - 5]->waitForFinished(); |
| 1399 | + providers[N_REQUESTS - 4]->waitForFinished(); |
| 1400 | + |
| 1401 | + // Cancel all of the requests. The ones that we didn't wait for synchronously are partly still in |
| 1402 | + // progress, and partly still in the queue. |
| 1403 | + for (auto& p : providers) |
| 1404 | + { |
| 1405 | + p->cancel(); |
| 1406 | + } |
| 1407 | + |
| 1408 | + if (spy.count() == 0) |
| 1409 | + { |
| 1410 | + spy.wait(120000); |
| 1411 | + } |
| 1412 | + else |
| 1413 | + { |
| 1414 | + // Pump the event loop for a while, to allow all the signals to trickle in. |
| 1415 | + QTimer timer; |
| 1416 | + QSignalSpy timer_spy(&timer, &QTimer::timeout); |
| 1417 | + timer.start(); |
| 1418 | + timer_spy.wait(10000); |
| 1419 | + } |
| 1420 | + EXPECT_EQ(1, spy.count()); |
| 1421 | + auto finish = chrono::system_clock::now(); |
| 1422 | + |
| 1423 | + add_stats(N_REQUESTS, start, finish); |
| 1424 | +} |
| 1425 | + |
| 1426 | int main(int argc, char** argv) |
| 1427 | { |
| 1428 | gst_init(&argc, &argv); |
| 1429 | |
| 1430 | QCoreApplication app(argc, argv); |
| 1431 | |
| 1432 | +#ifndef NDEBUG |
| 1433 | + setenv("MALLOC_CHECK_", "2", true); |
| 1434 | +#endif |
| 1435 | setenv("GSETTINGS_BACKEND", "memory", true); |
| 1436 | setenv("GSETTINGS_SCHEMA_DIR", GSETTINGS_SCHEMA_DIR, true); |
| 1437 | setenv("TN_UTILDIR", TESTBINDIR "/../src/vs-thumb", true); |
| 1438 | |
| 1439 | === modified file 'tests/thumbnailer/thumbnailer_test.cpp' |
| 1440 | --- tests/thumbnailer/thumbnailer_test.cpp 2015-10-22 03:00:37 +0000 |
| 1441 | +++ tests/thumbnailer/thumbnailer_test.cpp 2015-11-02 00:48:23 +0000 |
| 1442 | @@ -50,6 +50,7 @@ |
| 1443 | |
| 1444 | #define TEST_VIDEO TESTDATADIR "/testvideo.ogg" |
| 1445 | #define TEST_SONG TESTDATADIR "/testsong.ogg" |
| 1446 | +#define TEST_SONG_NO_EXTENSION TESTDATADIR "/testsong_ogg" |
| 1447 | |
| 1448 | using namespace std; |
| 1449 | using namespace unity::thumbnailer::internal; |
| 1450 | @@ -422,6 +423,24 @@ |
| 1451 | EXPECT_EQ(200, img.height()); |
| 1452 | } |
| 1453 | |
| 1454 | +TEST_F(ThumbnailerTest, thumbnail_song_no_extension) |
| 1455 | +{ |
| 1456 | + Thumbnailer tn; |
| 1457 | + auto request = tn.get_thumbnail(TEST_SONG_NO_EXTENSION, QSize(400, 400)); |
| 1458 | + ASSERT_NE(nullptr, request.get()); |
| 1459 | + // Audio thumbnails cannot be produced immediately |
| 1460 | + ASSERT_EQ("", request->thumbnail()); |
| 1461 | + |
| 1462 | + QSignalSpy spy(request.get(), &ThumbnailRequest::downloadFinished); |
| 1463 | + request->download(chrono::milliseconds(15000)); |
| 1464 | + ASSERT_TRUE(spy.wait(20000)); |
| 1465 | + string thumb = request->thumbnail(); |
| 1466 | + ASSERT_NE("", thumb); |
| 1467 | + Image img(thumb); |
| 1468 | + EXPECT_EQ(200, img.width()); |
| 1469 | + EXPECT_EQ(200, img.height()); |
| 1470 | +} |
| 1471 | + |
| 1472 | TEST_F(ThumbnailerTest, exceptions) |
| 1473 | { |
| 1474 | string const cache_dir = tempdir_path(); |
| 1475 | |
| 1476 | === modified file 'tools/parse-settings.py' |
| 1477 | --- tools/parse-settings.py 2015-06-12 23:52:13 +0000 |
| 1478 | +++ tools/parse-settings.py 2015-11-02 00:48:23 +0000 |
| 1479 | @@ -27,11 +27,16 @@ |
| 1480 | line = " int const %s_DEFAULT = %s;" % (name.replace("-", "_").upper(), default_value) |
| 1481 | print(line, file=output) |
| 1482 | |
| 1483 | +def bool_parameter(name, default_value, output): |
| 1484 | + line = " bool const %s_DEFAULT = %s;" % (name.replace("-", "_").upper(), default_value) |
| 1485 | + print(line, file=output) |
| 1486 | + |
| 1487 | def process_default(name): |
| 1488 | print("WARNING: unsupported type found for parameter %s\n" % name) |
| 1489 | |
| 1490 | options = {'i' : integer_parameter, |
| 1491 | 's' : string_parameter, |
| 1492 | + 'b' : bool_parameter, |
| 1493 | } |
| 1494 | |
| 1495 | def usage(): |
FAILED: Continuous integration, rev:290
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://