412186 - Last widget close should not cause XPCOM shutdown.

Status: RESOLVED WONTFIX

(Core Graveyard :: Embedding: GTK Widget, defect)

Description

[Oleg Romashin (:romaxa)]

Closing of the last gtk widget will shutdown XPCOM:
http://mxr.mozilla.org/seamonkey/source/embedding/browser/gtk/src/EmbedPrivate.cpp#202

I guess it should be possible to close last widget without terminate XRE Embedding, and allow terminate XRE by gtk_moz_embed_pop_startup call.

Comment 1

[Oleg Romashin (:romaxa)]

Attachment: Proposed fix

Comment 2

[Reed Loden [:reed]]

romaxa, please remember to check the "take bug" checkbox when attaching a patch for a bug so that the bug gets reassigned to you. Thanks!

Comment 3

[timeless]

Comment on attachment 297340 [details] [diff] [review]
Proposed fix

tabs:
+  if (sWidgetCount > 0 || sXpcomStarted)
+		return;

this pretty much neuters 

EmbedPrivate::PushStartup
EmbedPrivate::PopStartup

I don't think that's such a good thing.

It sounds like the embedder here should call gtk_moz_embed_push_startup/gtk_moz_embed_pop_startup before initializing anything else. The one extra push/pop pair before poking EmbedPrivate should trigger the more proper behavior without breaking anyone else.

Comment 4

[timeless]

Comment on attachment 297340 [details] [diff] [review]
Proposed fix

with that, i'm going to mark my own personal r- and suggest this bug is invalid. however any new module owners are free to clear my review and do whatever they please.

Comment 5

[Oleg Romashin (:romaxa)]

I just want to works next chain of calls:

push_startup
gtk_moz_embed_new()
destroy(GtkMozEmbedWidget)
gtk_moz_embed_new()
destroy(GtkMozEmbedWidget)
pop_startup();

exit();

Comment 6

[Benjamin Smedberg]

Comment on attachment 297340 [details] [diff] [review]
Proposed fix

I agree this is incorrect. If you don't want to shut down when the last widget closes you should explicitly push_startup before doing anything else.

Comment 7

[Benjamin Smedberg]

Or perhaps I don't understand... romaxa are you saying that your sequence doesn't work? If so, can you explain why not? It looks like it should work.

Comment 8

[Oleg Romashin (:romaxa)]

> If so, can you explain why not? It looks like it should work.

I'm not sure where is the real problem, but after second XPCOM restart and attempt to create new GtkMozEmbed widget it crashes...

I will try to locate problem (probably some static variables in XPCOM or in EmbedPrivate) and create new bug against proper component.

Comment 9

[Benjamin Smedberg]

No, no, I mean, we should be starting XPCOM once and shutting it down once:

push_startup -- start XPCOM here
gtk_moz_embed_new()
destroy(GtkMozEmbedWidget)
gtk_moz_embed_new()
destroy(GtkMozEmbedWidget)
pop_startup(); -- shutdown XPCOM here