D21463: KIMAP: Option to use system proxy settings (Core part)

Daniel Vrátil noreply at phabricator.kde.org
Fri May 31 12:56:30 BST 2019


dvratil requested changes to this revision.
dvratil added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> sessionthread.cpp:67
> +{
> +    m_useProxy = useProxy;
> +}

Either make `m_useProxy` an `std::atomic_bool` or make sure `m_useProxy` is accessed from the `SessionThread`'s thread (`QMetaObject::invokeMethod(this, [this, useProxy]() { m_useProxy = useProxy; }, Qt::QueuedConnection)`)

> sessionthread.cpp:68
> +    m_useProxy = useProxy;
> +}
> +

Unless the socket is disconnected, you should probably force reconnection here to apply the new configuration...

> sessionthread.cpp:175
> +        if (!m_useProxy) {
> +            qCDebug(KIMAP_LOG) << "using no proxy";
> +            QNetworkProxy proxy;

Provide more context: "Connecting to IMAP server without a proxy"

> sessionthread.cpp:180
> +        } else {
> +            qCDebug(KIMAP_LOG) << "using default system proxy";
> +        }

More context: "Connecting to IMAP server using default system proxy"

Shouldn't you also explicitly set the `QNetworkProxy` here as well? The socket doesn't get recreated on reconnect, so if the option is changed, it wouldn't get reset.

REPOSITORY
  R177 PIM: KIMAP

REVISION DETAIL
  https://phabricator.kde.org/D21463

To: marten, #kde_pim, mlaurent, dvratil
Cc: dvratil, mlaurent, kde-pim, dvasin, rodsevich, winterz, vkrause, knauss
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20190531/7379c7e4/attachment.html>


More information about the kde-pim mailing list