[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kfm-devel
Subject:    Re: Dolphin and Baloo
From:       Frank Reininghaus <frank78ac () googlemail ! com>
Date:       2014-02-04 14:45:53
Message-ID: CAFoZWWhL-c8THydzWVkss-stqE5h0cnfEroUe7-pJM1CTZ9vmQ () mail ! gmail ! com
[Download RAW message or body]

Hi,

2014-02-04 Vishesh Handa:
> I just fixed all the issues you mentioned.

cool, thanks! I think we're getting there. I noticed just two more things:

1. There is a HAVE_NEPOMUK left in src/search/dolphinsearchbox.cpp. I
guess the #ifdef'ed code should just be removed?

2. About this code in KFileItemModelRolesUpdater::rolesData():

#ifdef HAVE_BALOO
    if (m_balooFileMonitor)
        m_balooFileMonitor->addFile(item.localPath());

    // We never fill the Baloo roles over here since that would require
    // us to block by spawning a local event loop. Instead we call a slot
    // to fill the values later
    QMetaObject::invokeMethod(this, "applyChangedBalooRoles",
                              Qt::QueuedConnection, Q_ARG(QString,
item.localPath()));
#endif

(a) This will always invoke applyChangedBalooRoles and thus start a
Baloo::FileFetchJob, even if no Baloo roles are shown at all. We could
just include that statement in the "if" block to prevent that, right?

(b) Is there a reason to use a QMetaObject invocation for calling
applyChangedBalooRoles()? Since that function just starts the job, it
should finish quite fast, right?

(c) Unless I'm overlooking something, it looks to me like
rolesData(KFileItem) invokes applyChangedBalooRoles(QString), which
will then finally trigger applyChangedBalooRolesJobFinished(KJob*).
The latter function then calls rolesData(KFileItem) again. This looks
like an infinite recursion to me. It won't cause a crash or block the
GUI because two of the recursive calls are asynchronous, but it will
make Dolphin use 100% of one CPU core permanently. Or am I missing
something here?

Cheers,
Frank
[prev in list] [next in list] [prev in thread] [next in thread]