From kfm-devel  Wed Feb 05 11:36:18 2014
From: Vishesh Handa <me () vhanda ! in>
Date: Wed, 05 Feb 2014 11:36:18 +0000
To: kfm-devel
Subject: Re: Dolphin and Baloo
Message-Id: <16767485.yKYBQaVJ6n () vlap>
X-MARC-Message: https://marc.info/?l=kfm-devel&m=139160013125504

On Tuesday 04 February 2014 15:45:53 Frank Reininghaus wrote:
> 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?
> 

Not entirely sure. Will look more into it.

> 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?
> 

Fixed.

> (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?
> 

Right. Fixed it.

> (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?
> 

You're absolutely right.

I've changed it based on what you recommended, but I'm not too sure why that 
works.

> Cheers,
> Frank

-- 
Vishesh Handa