Comparing KFileItems

[Frank Reininghaus]

Hi everyone,

the operator used for comparing KFileItems is the following:

bool KFileItem::operator==(const KFileItem& other) const
{
    // is this enough?
    return d == other.d;
}

Obviously, KFileItems are only considered equal if their d pointers
are the same.

There is a Dolphin bug where this plays a role and leads to constant
high CPU usage:

https://bugs.kde.org/show_bug.cgi?id=304986

The short version is:

1. The class KFileItemModelRolesUpdater, which is responsible for
organizing the creation of previews, keeps two sets,
m_pendingVisibleItems and m_pendingInvisibleItems of (in)visible items
in the view which do not have previews yet. These KFileItems
implicitly share their d pointers with the KFileItems in
KFileItemModel.

2. When a file is renamed, DolphinView changes the item's name using
KFileItemModel::setData(), which changes the URL of the KFileItem
inside the model, and, unfortunately, detaches it from all other
KFileItems which have the same d pointer. Let's call the KFileItem in
the model 'item1'.

3. The KFileItem 'item2' stored in the set in
KFileItemModelRolesUpdater is apparently updated by KIO's magic and
also gets the new URL of the file. Now we have two KFileItems 'item1'
and 'item2' which share the URL and everything else, but are not
considered equal.

4. KFileItemModelRolesUpdater sees 'item2' in the set of pending items
and starts a PreviewJob, but replaces 'item2' by 'item1', which has
the same URL, in this process (see my comment 30 in the bug report for
an explanation why that happens).

5. When the preview is received for 'item1',
KFileItemModelRolesUpdater removes it from the sets of pending items.
However, 'item1' is different from 'item2', which is in the set.
Therefore, nothing is actually removed, and 'item2' remains in the set
of pending items.

6. KFileItemModelRolesUpdater sees that there is an 'item2' which
apparently has no preview yet -> go back to step 4.

There are ways to fix or work around this problem inside Dolphin, but
I'm wondering if it makes sense to change the way KFileItems are
compared. If two items do not have the same d pointers, one could
check if their URLs are equal and consider them equal in that case.
Or, if that is not considered suffient for equality, do what
KFileItem::cmp(const KFileItem&) does.

I see the following advantages and disadvantages:

Pro:

a) The implicit sharing is an implementation detail of KFileItem. At
least I would not have expected that operator==() depends on this and
only compares the d pointers.
b) It could possibly prevent other bugs in the future and save debugging time.

Con:

Changing the way KFileItems are compared could considerably slow down
comparisons of items which are not equal.

Any opinions about this?

Best regards,
Frank