GNOME Bugzilla – Bug 673273
ephy-embed-prefs: check webkit_settings before using
Last modified: 2012-04-10 19:36:14 UTC
See the attached patch comment. I seem to need this to prevent a crash in EphyEmbedSingle tests: TEST: test-ephy-embed-single... (pid=31942) /embed/ephy-embed-single/new: OK /embed/ephy-embed-single/get_from_shell: OK /embed/ephy-embed-single/form_auth: OK (test-ephy-embed-single:31942): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed In /new we create a EphyEmbedSingle and destroy it. Then we get the one from the shell, which is only a pointer to the one that was created in _ephy_shell_create_instance. When we reach /form_auth, the g_object_unref that we called in /new has called ephy_embed_prefs_shutdown (), which means that webkit_settings no longer is valid. This causes the assertion fail. I would prefer to move ephy_embed_prefs_init/shutdown somewhere else. Just like we use ephy_file_helpers_init/shutdown. I don't know where would be the best place to put this though. I am tempted to use src/ephy-main.c Anyway, filing this with this patch to raise the issue. I think the patch could be the worst-case solution if we want to keep everything "working" like now.
Created attachment 211056 [details] [review] ephy-embed-prefs: check webkit_settings before using ephy_embed_prefs_shutdown is called every time a EphyEmbedSingle is finalized. In tests EphyEmbedSingle can be created multiple times, which means that as soon as the first one finalizes ephy_embed_prefs_shutdown is gonna operate on an invalid webkit_settings. In the same spirit, protect ourselves from multiple calls to ephy_embed_prefs_init by checking webkit_settings.
Created attachment 211058 [details] [review] ephy-embed-single: do not handle ephy-embed-prefs ephy-embed-prefs acts like a singleton. Instead of calling init and shutdown in EphyEmbedSingle instances. Handle it in ephy-main as a true init/shutdown API like ephy-file-helpers. ---- Thought about it. I prefer this solution.
First dumb comment from my side... this used to work, what changed?
Created attachment 211123 [details] [review] ephy-embed-single-test: unref the created EphyShell This commit, correcting a forgotten unref, will trigger the problem described in this bug.
Review of attachment 211123 [details] [review]: OK. May I use this opportunity to suggest again to write a ephy_test_init () ephy_test_shutdown () thing that does all this.
Review of attachment 211058 [details] [review]: OK.
(In reply to comment #5) > Review of attachment 211123 [details] [review]: > > OK. May I use this opportunity to suggest again to write a ephy_test_init () > ephy_test_shutdown () thing that does all this. Will look into that.
Attachment 211058 [details] pushed as 27c169f - ephy-embed-single: do not handle ephy-embed-prefs Attachment 211123 [details] pushed as bbb6e24 - ephy-embed-single-test: unref the created EphyShell