After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 755341 - Searching in gnome-shell -> epiphany-search-provider Segmentation fault
Searching in gnome-shell -> epiphany-search-provider Segmentation fault
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: General
3.17.x
Other Linux
: Normal critical
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-09-21 11:16 UTC by sangu
Modified: 2015-09-22 14:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ephy-completion-model: Make it possible not to use formatting markup (8.67 KB, patch)
2015-09-21 15:54 UTC, Claudio Saavedra
none Details | Review
ephy-completion-model: Make it possible not to use formatting markup (8.57 KB, patch)
2015-09-21 17:11 UTC, Michael Catanzaro
none Details | Review
ephy-completion-model: Make it possible not to use formatting markup (8.21 KB, patch)
2015-09-21 17:22 UTC, Michael Catanzaro
accepted-commit_after_freeze Details | Review

Description sangu 2015-09-21 11:16:58 UTC
$ coredumpctl gdb /usr/libexec/epiphany-search-provider

Core was generated by `/usr/libexec/epiphany-search-provider'.
Program terminated with signal SIGSEGV, Segmentation fault.
  • #0 _gtk_settings_get_style_cascade
    at gtksettings.c line 1771
  • #0 _gtk_settings_get_style_cascade
    at gtksettings.c line 1771
  • #1 gtk_style_context_init
    at gtkstylecontext.c line 343
  • #2 g_type_create_instance
    at gtype.c line 1870
  • #3 g_object_new_internal
    at gobject.c line 1779
  • #4 g_object_newv
    at gobject.c line 1926
  • #5 g_object_new
    at gobject.c line 1619
  • #6 gtk_style_context_new
    at gtkstylecontext.c line 561
  • #7 query_completed_cb
    at ephy-completion-model.c line 304
  • #8 query_completed_cb
    at ephy-completion-model.c line 331
  • #9 query_completed_cb
    at ephy-completion-model.c line 508
  • #10 ephy_history_service_execute_job_callback
    at ephy-history-service.c line 548
  • #11 g_main_context_dispatch
  • #12 g_main_context_dispatch
    at gmain.c line 3769
  • #13 g_main_context_iterate
    at gmain.c line 3840
  • #14 g_main_context_iteration
    at gmain.c line 3901
  • #15 g_application_run
    at gapplication.c line 2311
  • #16 main
    at ephy-search-provider-main.c line 52

---
epiphany-3.17.91-2.fc23.x86_64
gtk3-3.17.9-1.fc23.x86_64
glib2-2.45.8-1.fc23.x86_64
gnome-shell-3.17.92-1.fc23.x86_64
Comment 1 Claudio Saavedra 2015-09-21 15:54:25 UTC
Created attachment 311769 [details] [review]
ephy-completion-model: Make it possible not to use formatting markup

Since the completion model is used in the search provider as well,
which doesn't need the markup and also doesn't initialize GTK+,
we need to ensure that the completion model can still be used
without it.

Definitely not the best approach but safe enough for .0 release.
Comment 2 Michael Catanzaro 2015-09-21 16:05:39 UTC
Review of attachment 311769 [details] [review]:

::: src/ephy-completion-model.c
@@ +288,3 @@
+    text = get_row_text (row->location, row->title, subtitle_color);
+  else
+    text = row->title;

So you got rid of the newline and the URL. I guess GNOME has simply been ignoring everything after our newline?

I prefer to use g_strdup() here, to simply the memory management...

@@ +300,3 @@
+
+  if (model->priv->use_markup)
+    g_free (text);

...and you don't have to make this conditional.

@@ +355,2 @@
   for (i = 0; new_rows != NULL; i++) {
     PotentialRow *row = (PotentialRow*)new_rows->data;

Not quite sure how this works, because here you go into a for loop with null subtitle_color, which winds up calling get_row_text() again, which you avoided above. And this function is called always in query_completed_cb(). Perhaps the loop is never entered?
Comment 3 Claudio Saavedra 2015-09-21 16:41:47 UTC
(In reply to Michael Catanzaro from comment #2)
> Review of attachment 311769 [details] [review] [review]:
> 
> ::: src/ephy-completion-model.c
> @@ +288,3 @@
> +    text = get_row_text (row->location, row->title, subtitle_color);
> +  else
> +    text = row->title;
> 
> So you got rid of the newline and the URL. I guess GNOME has simply been
> ignoring everything after our newline?

From reading the code changes in 592e5, I'd say that only the title was being used in the search provider.

> 
> I prefer to use g_strdup() here, to simply the memory management...
> 
> @@ +300,3 @@
> +
> +  if (model->priv->use_markup)
> +    g_free (text);
> 
> ...and you don't have to make this conditional.

OK.

> 
> @@ +355,2 @@
>    for (i = 0; new_rows != NULL; i++) {
>      PotentialRow *row = (PotentialRow*)new_rows->data;
> 
> Not quite sure how this works, because here you go into a for loop with null
> subtitle_color, which winds up calling get_row_text() again, which you
> avoided above. And this function is called always in query_completed_cb().
> Perhaps the loop is never entered?

NULL subtitle color is a result of not calling the style/settings code that was causing the crash. Later on we don't use the NULL subtitle color at all, because we do not use the markup either.
Comment 4 Michael Catanzaro 2015-09-21 17:09:59 UTC
OK, I will upload the same patch with just the g_strdup change, since you had to leave.
Comment 5 Michael Catanzaro 2015-09-21 17:11:31 UTC
Created attachment 311780 [details] [review]
ephy-completion-model: Make it possible not to use formatting markup

Since the completion model is used in the search provider as well,
which doesn't need the markup and also doesn't initialize GTK+,
we need to ensure that the completion model can still be used
without it.

Definitely not the best approach but safe enough for .0 release.
Comment 6 Michael Catanzaro 2015-09-21 17:22:32 UTC
Created attachment 311783 [details] [review]
ephy-completion-model: Make it possible not to use formatting markup

Whitespace fix
Comment 7 Michael Catanzaro 2015-09-21 23:38:53 UTC
Got the freeze break