445924 - Fennec (XULRunner) should be using osso_initialize / osso_deinitialize

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Maemo docs seem to indicate that callss to osso_initialize and osso_deinitialize should be used in Maemo (Hildon) applications.

https://bugs.maemo.org/show_bug.cgi?id=876
http://wiki.forum.nokia.com/index.php/Maemo:_Hildonizing_application_UI#Registering_to_D-Bus

Currently, osso_initialize is not used anywhere in our code. This may be more of a problem when launching Fennec from the Extras > Fennec shortcut because a DBus service file is used to do the launch.

"The application must be registered as a D-Bus service so that it will not be killed automatically by the system. This can be done with osso_initialize(). The first parameter needs to match the service name defined in the service file. Also remember to call osso_deinitialize() before exiting the application."

Comment 1

[Antonio Gomes (tonikitoo)]

minimo used to do the following for bora:
(...)
  // tonikitoo - initial dbus-glib implementation support
#if 1
  osso_context_t *osso_context;
  /* Initialize maemo application */
  osso_context = osso_initialize ("minimo", "0.16", TRUE, NULL);

  /* Check that initilialization was ok */
  if (osso_context == NULL) {
    ErrorAlert ("Could not start Minimo. (8)!");
    return -1;
  }
#endif
(...)


it was basically to avoid the system, to kill me if launched from menu. It is just a very beging for a better dbus support - make fennec able to talk to other via dbus and vice-versa.

Comment 2

[Doug Turner (:dougt)]

this is pretty terrible and probably a cause for most (or many) of our crashes after running for a bit.

Comment 3

[Doug Turner (:dougt)]

Attachment: patch v.1

calls the right osso functions during startup and shutdown.

NOTE:  gAppData->name must match the name used in the Desktop name.  This is spelled out for you in the osso_initialize documentation, but I think i should also mention it here.

Comment 4

[Oleg Romashin (:romaxa)]

> ifdef NS_OSSO
>-EXTRA_DSO_LDOPTS += -llocation -lgpsbt
>+EXTRA_DSO_LDOPTS += -llocation -lgpsbt -losso
> endif

Better to change to 
+EXTRA_DSO_LDOPTS += $(MOZ_LOCATION_LIBS)

+ifdef NS_OSSO
+EXTRA_DSO_LDOPTS += -losso [or NS_OSSO_LIBS]
+endif

Also see https://bugzilla.mozilla.org/attachment.cgi?id=330194&action=edit

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to comment #4)
> > ifdef NS_OSSO
> >-EXTRA_DSO_LDOPTS += -llocation -lgpsbt
> >+EXTRA_DSO_LDOPTS += -llocation -lgpsbt -losso
> > endif
> 
> Better to change to 
> +EXTRA_DSO_LDOPTS += $(MOZ_LOCATION_LIBS)
> 
> +ifdef NS_OSSO
> +EXTRA_DSO_LDOPTS += -losso [or NS_OSSO_LIBS]
> +endif
> 
> Also see https://bugzilla.mozilla.org/attachment.cgi?id=330194&action=edit
> 

or this one https://bugzilla.mozilla.org/attachment.cgi?id=330217&action=edit

Comment 6

[timeless]

Comment on attachment 330192 [details] [diff] [review]
patch v.1

+    osso_deinitialize(m_osso_context);

you'll want to null out m_osso_context, i'm sure the function wouldn't appreciate being called w/ a deinitialized pointer :)

Comment 7

[Doug Turner (:dougt)]

brad/roxama: Separate bugs please.  rearranging the bikes in the bike shed has nothing to do with not initializing the platform correctly.

timeless: i will add a m_osso_content = nsnull in ::Startup()

Comment 8

[Benjamin Smedberg]

Comment on attachment 330192 [details] [diff] [review]
patch v.1

> NS_IMETHODIMP
> nsNativeAppSupportUnix::Start(PRBool *aRetVal)
> {
>+  NS_ENSURE_STATE( gAppData );

Please make this an assertion.

>+#ifdef NS_OSSO
>+  /* Initialize maemo application */
>+  m_osso_context = osso_initialize(gAppData->name, 
>+                                   gAppData->version ? gAppData->version : "1.0",
>+                                   PR_TRUE,
>+                                   nsnull);

Please file a followup on trying to verify that gAppData->name matches the name in the "service file". I imagine this would bite other maemo XR app authors pretty hard.

Comment 9

[Doug Turner (:dougt)]

follow up bug:  https://bugzilla.mozilla.org/show_bug.cgi?id=446018

timeless: on closer look, i do not think we need to do anything special here.

Comment 10

[Doug Turner (:dougt)]

spoke to timeless, he was worried about people calling Stop() twice in a row which would cause us to call osso_deinit with garbage.  In the patch checked in, i null the m_osso_context after calling osso_deinit.

changeset:   16041:ab1888ef44be
tag:         tip
user:        Doug Turner <[e-mail]>
date:        Fri Jul 18 11:17:49 2008 -0700
summary:     Bug 445924. Fennec (XULRunner) should be using osso_initialize / osso_deinitialize. r=bsmedberg

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

xulrunner on maemo has been working well, verified with beta3