Merge lp:~jonas-drange/ubuntu-ui-extras/printerloader-thread-affinity into lp:~phablet-team/ubuntu-ui-extras/printer-staging

Proposed by Jonas G. Drange
Status: Merged
Approved by: Andrew Hayzen
Approved revision: 151
Merged at revision: 152
Proposed branch: lp:~jonas-drange/ubuntu-ui-extras/printerloader-thread-affinity
Merge into: lp:~phablet-team/ubuntu-ui-extras/printer-staging
Diff against target: 65 lines (+26/-0)
3 files modified
modules/Ubuntu/Components/Extras/Printers/cups/printerloader.cpp (+3/-0)
modules/Ubuntu/Components/Extras/Printers/printer/printer.cpp (+1/-0)
tests/unittests/Printers/tst_printer.cpp (+22/-0)
To merge this branch: bzr merge lp:~jonas-drange/ubuntu-ui-extras/printerloader-thread-affinity
Reviewer Review Type Date Requested Status
Andrew Hayzen (community) Approve
Review via email: mp+320355@code.launchpad.net

Commit message

corrects thread affinity for printerloaded printers, as well as any qobject children it might have

Description of the change

corrects thread affinity for printerloaded printers, as well as any qobject children it might have

To post a comment you must log in.
Revision history for this message
Andrew Hayzen (ahayzen) wrote :

LGTM, fixes the issue when applying the patch [0], to the examples and the tests still pass :-)

0 - http://pastebin.ubuntu.com/24214888/

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'modules/Ubuntu/Components/Extras/Printers/cups/printerloader.cpp'
--- modules/Ubuntu/Components/Extras/Printers/cups/printerloader.cpp 2017-02-21 10:46:29 +0000
+++ modules/Ubuntu/Components/Extras/Printers/cups/printerloader.cpp 2017-03-20 13:33:08 +0000
@@ -18,6 +18,7 @@
18#include "backend/backend_cups.h"18#include "backend/backend_cups.h"
19#include "printerloader.h"19#include "printerloader.h"
2020
21#include <QApplication>
21#include <QPrinterInfo>22#include <QPrinterInfo>
2223
23class PrinterCupsBackend;24class PrinterCupsBackend;
@@ -48,6 +49,8 @@
4849
49 auto p = QSharedPointer<Printer>(new Printer(backend));50 auto p = QSharedPointer<Printer>(new Printer(backend));
5051
52 p->moveToThread(QApplication::instance()->thread());
53
51 Q_EMIT loaded(p);54 Q_EMIT loaded(p);
52 Q_EMIT finished();55 Q_EMIT finished();
53}56}
5457
=== modified file 'modules/Ubuntu/Components/Extras/Printers/printer/printer.cpp'
--- modules/Ubuntu/Components/Extras/Printers/printer/printer.cpp 2017-03-14 14:47:54 +0000
+++ modules/Ubuntu/Components/Extras/Printers/printer/printer.cpp 2017-03-20 13:33:08 +0000
@@ -29,6 +29,7 @@
29{29{
30 loadAttributes();30 loadAttributes();
3131
32 m_jobs.setParent(this);
32 m_jobs.filterOnPrinterName(name());33 m_jobs.filterOnPrinterName(name());
3334
34 QObject::connect(m_backend, &PrinterBackend::printerStateChanged,35 QObject::connect(m_backend, &PrinterBackend::printerStateChanged,
3536
=== modified file 'tests/unittests/Printers/tst_printer.cpp'
--- tests/unittests/Printers/tst_printer.cpp 2017-03-10 13:15:27 +0000
+++ tests/unittests/Printers/tst_printer.cpp 2017-03-20 13:33:08 +0000
@@ -348,6 +348,28 @@
348 );348 );
349 QCOMPARE(proxy->sourceModel()->objectName(), jobs.objectName());349 QCOMPARE(proxy->sourceModel()->objectName(), jobs.objectName());
350 }350 }
351
352 /* There was a bug causing QML thread assertion to fail. The assertion
353 requires all QObjects accessed from QML to be in the same thread as QML.
354 For newly loaded non-proxy printers, this was not the case. This test
355 serves as a regression test, as well as a place where we can make sure
356 every QObject of the Printer API moves thread when the printer itself
357 moves.
358
359 For POD, this is inconsequential. */
360 void testPrinterMovesProperlyFromThread()
361 {
362 MockPrinterBackend *backend = new MockPrinterBackend(m_printerName);
363
364 Printer p(backend);
365 QThread thread;
366 p.moveToThread(&thread);
367
368 QCOMPARE(p.thread(), &thread);
369
370 // Ideally, all QObjects in the Printer API needs to be tested here.
371 QCOMPARE(p.jobs()->thread(), &thread);
372 }
351private:373private:
352 QString m_printerName = "my-printer";374 QString m_printerName = "my-printer";
353 MockPrinterBackend *m_backend = nullptr;375 MockPrinterBackend *m_backend = nullptr;

Subscribers

People subscribed via source and target branches