[Merge] lp:~jonas-drange/ubuntu-settings-components/buffer-printer-state-events into lp:~phablet-team/ubuntu-settings-components/printer-components

[Andrew Hayzen]

Review: Needs Information

Looks good and resolves the issue, one question about a possible racy situation - don't think it would happen often (if at all) but maybe we should protect?

Diff comments:

> 
> === added file 'plugins/Ubuntu/Settings/Printers/printer/printersignalhandler.cpp'
> --- plugins/Ubuntu/Settings/Printers/printer/printersignalhandler.cpp	1970-01-01 00:00:00 +0000
> +++ plugins/Ubuntu/Settings/Printers/printer/printersignalhandler.cpp	2017-02-16 14:10:27 +0000
> @@ -0,0 +1,69 @@
> +/*
> + * Copyright (C) 2017 Canonical, Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; version 3.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "printersignalhandler.h"
> +
> +PrinterSignalHandler::PrinterSignalHandler(int triggerEventDelay,
> +                                           QObject *parent)
> +    : QObject(parent)
> +{
> +    m_timer.setInterval(triggerEventDelay);
> +    connect(&m_timer, SIGNAL(timeout()), this, SLOT(process()));
> +}
> +
> +PrinterSignalHandler::~PrinterSignalHandler()
> +{
> +}
> +
> +void PrinterSignalHandler::process()
> +{
> +    Q_FOREACH(auto printer, m_unprocessed) {
> +        Q_EMIT printerModified(printer);
> +    }
> +    m_unprocessed.clear();

I wonder if this could be racy? Could after printerModified(NPrinter) a further modification happen and then the m_unprocessed and m_timer are cleared? Meaning that change was 'lost'.  Maybe this should block onPrinterModified and onPrinterStateChanged while doing this?

> +    m_timer.stop();
> +}
> +
> +void PrinterSignalHandler::onPrinterModified(
> +    const QString &text, const QString &printerUri,
> +    const QString &printerName, uint printerState,
> +    const QString &printerStateReason, bool acceptingJobs)
> +{
> +
> +    Q_UNUSED(text);
> +    Q_UNUSED(printerUri);
> +    Q_UNUSED(printerState);
> +    Q_UNUSED(printerStateReason);
> +    Q_UNUSED(acceptingJobs);
> +
> +    m_unprocessed << printerName;
> +    m_timer.start();
> +}
> +
> +void PrinterSignalHandler::onPrinterStateChanged(
> +    const QString &text, const QString &printerUri,
> +    const QString &printerName, uint printerState,
> +    const QString &printerStateReason, bool acceptingJobs)
> +{
> +    Q_UNUSED(text);
> +    Q_UNUSED(printerUri);
> +    Q_UNUSED(printerState);
> +    Q_UNUSED(printerStateReason);
> +    Q_UNUSED(acceptingJobs);
> +
> +    m_unprocessed << printerName;
> +    m_timer.start();
> +}


-- 
https://code.launchpad.net/~jonas-drange/ubuntu-settings-components/buffer-printer-state-events/+merge/317485
Your team Ubuntu Phablet Team is subscribed to branch lp:~phablet-team/ubuntu-settings-components/printer-components.