Andrea Azzarone authored Jul 30, 2016 and Iain Lane committed Aug 01, 2016

The problem is that in the function canvas_container_set_workarea the screen width
and height are in "application pixels" while the workarea ones are in "device
pixels" so when the scaling is > 1, the margins are not properly setted.

We need to scale-down the workarea geometries to "application pixels".

https://bugzilla.gnome.org/show_bug.cgi?id=769302

The progress info destination is obtainable through a getter function that
returns a reference to it. This reference is created without checking if the
destination is not null, which leads to critical warnings. In order to fix this,
check the destination before creating the reference.

https://bugzilla.gnome.org/show_bug.cgi?id=762609

To snap an icon to the closest grid cell, we were trying to round
the x position with an offset of 0.5. However, cells are not
normalized, so that 0.5 is actually doing little to round the
position.

This is making the icons to snap to the right closest cell once the
user drags more the icon left to the current cell.

The actual size of the cell is SNAP_SIZE_*, so to fix this use half
of the SNAP_SIZE to "round" the number.

https://bugzilla.gnome.org/show_bug.cgi?id=750446

The view doesn't display files while it's loading, and that's
mostly fine and what we want to avoid jumping while seeing new
files.

However, the when searching, it's expected to not all files be there,
and the first ones reported by the search system have a higher score
to appear, so that means it's fine if the user doesn't see files with
low score while files with high score are presented.
On the other hand, is not fine to wait until the search system
stops to show all the files that were found.

So for search we want to show files while they are being loaded, that
means the directory doesn't have the all_files_seen flag set.

Previously, we were solving this issue setting the all_files_seen
flag in the search directory so the view will show them when hits
are added. However, that also means the view thinks the loading is
complete, removing the floating bar and any hint that it was loading.

So what we actually want is special case the search, and show th...

We were marking as all_files_seen when hits were added, so the view
will react on it and display the files that were recently added.

However, that's not consistent, and the view calls done_loading
if all files are seen. This has, as a UI effect, removing the floating
bar, and therefore any hint that the view is still loading/searching.

So let's be consistent here and in the upcoming patch we will fix
this issue with the search.

It's just a cosmetic change (I hope), but let's get things consistent.

There is code that is used in case FAM is not available. But
we use inotify since 2006, so I'm confident we are fine without
checking for it.

This remove some not needed code.

These days XDG_CURRENT_DESKTOP can contain multiple desktop names
seperated by ':'.

We were not disconnecting them, so they could be called after
the search-directory is freed, causing a crash.

To be honest, not sure how it survived until now, probably because
previously (I didn't check) the search engine was freed when stopped.

We were leaking it in these cases. But we need to make sure
we don't free the directory if we are using the same as the
internal one. So manage that in the load_directory as a generic
function.

https://bugzilla.gnome.org/show_bug.cgi?id=759717

This is the last patch of a series of patches to fix all the
related issues with search synchronization, robustness on search, etc.
This patch orders fix when the view is requested to load a search
directory.

This is tricky because the views expect real locations, and when a
caller sets the view to start searching the view expect to already have
a real location loaded, so it can set the invented search directory with
the model as a backing uri.

In the case that window slot request a search directory, we are screwed,
because either we load first the real backing uri, which will screw
the window slot and all of the connected users of the view when the
is-location signal is done, or we first set the query and then load the
search directory, which will screw it anyway because the real location
set up at that point in the view is not the one associated with the
search directory.

So we effectively need to do both at the same time.
To do that, implement a set_search_query_internal which will allow us to
pass a backing uri, so when the slot sets a location that is a search
directory, instead of loading the directory itself, we only set the
query indicating the real location behind, so it can set up everything
in the search directory before loading it.

This patch was done with a few things in mind, and it actually fixes
few other issues. For example, now then we reuse the directory from the
slot, we actually create a new search directory, so we actually don't
reuse it. The point for this is because if we reuse the directory, any
stop to the search directory will stop the future search that we just
set. This happens when the slot stops the old view to load (which had
the same search directory before this patch) after setting the new
view to load.

A better fix would be to get rid of the nautilus-search directory
invention or to make it only internal, and make the window-slot be aware
when a location was also searching (with a struct instead of using
nautilus-bookmarks).

https://bugzilla.gnome.org/show_bug.cgi?id=759717

So when searching, window-slot is able to get the location.

https://bugzilla.gnome.org/show_bug.cgi?id=759717

Since now they are the same, just use the public api.

https://bugzilla.gnome.org/show_bug.cgi?id=759717

Nautilus slot ends any location change before starting a new
one or when is done loading.

The done loading signal is emitted when the view finalize to load
a directory.

However, when loading a new location, if the previous one was not
finalized to load, nautilus was crashing, due to the is-loading signal
in the view first emitting it for finalizing the previous location
change.

Then the callback of window slot was freeing the new_content_view,
because this callback can be done when canceling a location change.
However, at that time, the new_content_view is actually the one which
is going to be used as a content view when we are just changing
locations, for example, while searching.

It's kind of strange that the callback of the view ending a load
frees the new_content_view, since this is already managed by setup_view.
And since we already have a cancel_location_change, we can use that
on the majority of situations when we actually want to cancel a change,
and let the end_location callback for the specific case of ending a load
of the view.

So now only the cancel_location_change frees the new_content_view, and
we avoid a crash freeing the current view while loading it when changing
locations.

This is the last patch of the series of patches to fix the crashing
nautilus while searching.

https://bugzilla.gnome.org/show_bug.cgi?id=759717

So we were relying on the model to be NULL to not update all
the view widgets and details in the done_loading callback.

However, we need the model alive so in the signal of is-loading
emitted on the done_loading function, external objects can peek
the model, etc. even in destruction time.

So instead of setting the model as NULL on destroy and relying on it,
use a explicit boolean to not update view widgets on destroy.

https://bugzilla.gnome.org/show_bug.cgi?id=759717

Even if we disconnect them on the subclasses, as long as we connect them
here we should disconnect them as well.

https://bugzilla.gnome.org/show_bug.cgi?id=759717

So we have all the disconnection in one place and we avoid
random callbacks while destroying.

https://bugzilla.gnome.org/show_bug.cgi?id=759717

Dispose is intended to let the object still valid for external
calls.
Freeing the action group on dispose makes the view somehow invalid,
so some calls were made at that time and were making the view to crash.

This change to dispose was made in commit 347369d1 to fix a crash as
well. However, an upcoming patch fixed it as well as noted in the
commit message.
So although dispose seemed good, it's not if it makes the object
invalid.

On the other hand, instead of dispose we should use destroy, since we
are already using it, and implementing dispose was kind of an error.

In upcoming patches we will move some disconnect_signal calls from
finalized to destroy.

https://bugzilla.gnome.org/show_bug.cgi?id=759717

It's good to have a way to search for assignments with grep "whatever ="

We can return to an old location which happens to be a search
location.
The issue is, we were not updating all the associated properties
of the search when that happened so the window slot and the
files view was in a inconsistent state.

For that, we need the files view to also accept to change to search
locations, in which case it needs to set it's query appropiately
to the query the search location was using.

Thanks to Georges for the initial work on this.

https://bugzilla.gnome.org/show_bug.cgi?id=756183

How search works:
The main engine starts.
It starts in order all the search providers which start one
or more threads.
Then the owner of the engine can decide to stop, and therefore
requesting the providers to stop.
Then the providers take their time in the different threads to cancel
and to report to the engine, which is the main thread, that they
finished.
At that point the engine signals that the engine is stopped and
finished.

However, if one of the search providers fail to report it's finalization
the engine is hanging forever, making everything stopping to work.

This was the issue when the engine requests the model provider to stop,
and then start again before it got time to request the directory info,
since we add the iddle but never rested it's id, so never signal a finish.

This was working most of the times because, this idle is only requested
when we stop the model provider and it's still running, but usually the
work the model has to do is so little that always gets finished before
stopping it.

So to fix this issue, reset the idle id when finished.

https://bugzilla.gnome.org/show_bug.cgi?id=756183

When thumbnailing a directory with jp2 some times nautilus
was crashing.
However tests on gdk_pixbuf were successful and gnome-desktop-thumbnails
generation tests were also working.

Also, nautilus is using raw pthreads in the thumbnail generation
code, and seems the crash was actually only happening when inside the
pthread and when using gdk-pixbuf, not only the gnome-desktop-thumbnail.

Looking at the implementation of glib in threads and nautilus, one of
the differences is that nautilus sets a stack size.
The crash is happening because, unluckely, libjasper with some big
images is using more stack size than the one nautilus is setting, which
leads to a crash in libjasper.

To fix it, remove the stack size set by nautilus, similarly to what glib
does, not setting an actual stack size.

Obviously the right thing to do is rewrite nautilus code to use the
glib threads, but I want to let that as a newcomer bug to do.

What we are doing:
Directory starts io.
It cancels the cancellables associated to each async operation.
It starts new operations.
When the new operation finishes, either because it's cancelled or
because it successfully finished, it modifies the directory private
data and sets its associated cancellable as NULL to indicate the
directory that it's all done, so the directory just checks for the
cancellables in its private data to know if there is some operation
going on.

However, what can happens is:
Directory starts io.
It cancels the cancellables and sets as null to start a new operation.
It starts a new operation.
The old operation finishes cancelled, and sets as null the private
cancellable of the directory.
Now the directory thinks there is no operation going on, but actually
the new operation is still going on.
The directory starts io and checks if there is something to stop, but
sees there is no cancellable and keeps going.
Then the new operation finishes and hits an assert when realizes that
the directory state is inconsistent.

To fix this, don't set as null the cancellable in the private data of
the directory when the operation has been cancelled.

It's okay to set as null when the operation finishes succesfully, since
it's ensured that only one operation can be running withouth being
cancelled.

https://bugzilla.gnome.org/show_bug.cgi?id=759608

When a file gets moved, nautilus directory cancel the loading of
the attributes of that file, in case that file is in the work queue
of that directory, since it will move to another directory.

For that, we cancel the cancellable associated to the async request
of the attributes of that file.

However, since this is threaded, if some client of the directory kick of
I/O with the nautilus_directory_async_state_changed and the cancellable
didn't reach the callback, the file attributes are still going on, and
we reach an assert when trying to stop them again, since the file
requesting for those attributes is no longer the one we want to stop
loading.

This was causing problems when the race kicks in, for example, the
move to context menu action.

To fix it, apart of cancel the cancellable associated, set the directory
private data files as null, to avoid trying to stop the already stopped
request.

https://bugzilla.gnome.org/show_bug.cgi?id=756253

now that nobody looks

Files attributes in nautilus are cached, and sometimes having a cached
value its problematic for special cases like mounting due to how window
slot mounts locations, which relies in the file not having the mount
information ready, which is not true when the file doesn't get
finalized and doesn't get acknowledge by the volume monitor when its
root gets unmounted.

To avoid that, invalidate all file attributes when changing locations.

A better solution for this specific case would be to make the root
volume monitor to acknowledge all its children when unmounted, and keep
it alive as long as there are children.

But this fix is anyway needed since previous 3.18 is how it was done and
seems it's a needed for now "workaround all" issues with cached values.

https://bugzilla.gnome.org/show_bug.cgi?id=757555