Review Request: EngineController: fixes to canDecode() and supportedMimeTypes(): make them non-static, thread-safe even on first call. (squached patches, recent on top)

[Matěj Laitl]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105524/
-----------------------------------------------------------

Review request for Amarok, Ralf Engels and Sven Krohlas.


Description
-------

EngineController: use QFileInfo instead of apparently non-reentrant KFileItem

...remote files are not needed here, so QFileInfo does a good job. This
is the cause of last crashes in testm3uplaylist. (I have about 25%
probability of it crashing until this commit)

EngineController: make supportedMimeTypes() thread-safe even on first call

If supportedMimeTypes() is called for the first time in a thread,
Phonon::BackendCapabilities::availableMimeTypes() creates some QObject
subclasses in non-main thread and that causes problems later on, and is
the cause of testm3uplaylist failures. Add signal/slot/QSemaphore
trickery that causes Phonon::BackendCapabilities::availableMimeTypes()
is called from the main thread without performance penalty for 2nd and
next calls.

Test is added for EngineController::supportedMimeTypes() so that this
fragile code (hopefully) never breaks again.

This fixes (perhaps along with the next commit) 2 failing tests (
testm3uplaylist and apparently testplaylistmodels), so the fail count
is down to 4 here.

EngineController: make canDecode() and supportedMimeTypes() non-static

Static methods have no sense in a singleton class. Additionally, it was
very hacky (and impossible in corner-cases) to keep these thread-safe,
see testm3uplaylist failures. Making them non-static will allow us to
do some tricks so that the calls are more robust.

Never-used and confusing destory() method is removed, too.

Care is taken not to break existing tests.


Diffs
-----

  src/EngineController.h ad2e0c4a5e7c80c79bf448bf74cd6b52cd1f0ed3 
  src/EngineController.cpp 83f0a6ed0a92ae992e1809800cee65d9349dc680 
  src/browsers/filebrowser/FileBrowser.cpp 567ff799df7ef8bfcd93de73ee120bfc5be634b7 
  src/browsers/filebrowser/FileView.cpp 8eae4b6731ebfcb7310d3d719687989be125de92 
  src/core-impl/collections/support/CollectionManager.cpp 9085b5a27d9a7ce94d2325d94bec5fce8d126abe 
  tests/CMakeLists.txt 901c716a600bca63639939f4e62dbd89d9db707f 
  tests/TestEngineController.h PRE-CREATION 
  tests/TestEngineController.cpp PRE-CREATION 
  tests/core-impl/playlists/types/file/m3u/TestM3UPlaylist.cpp 4bbf29be01aef9043a64075a196ac04a544134cb 
  tests/core/meta/TestMetaTrack.cpp 0adb7633538533202c50487b1eaff8e4ead150e9 
  tests/playlistmanager/sql/TestSqlUserPlaylistProvider.cpp bb09515d617e8149e7e66e804ff46aa5ddb76045 

Diff: http://git.reviewboard.kde.org/r/105524/diff/


Testing
-------

Works, fixes testm3uplaylist


Thanks,

Matěj Laitl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120712/ac9e65e5/attachment.html>