465556 - Fennec does not create extensions.rdf file

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[(dormant account)]

This causes file io at startup due to _ensureDatasetIntegrity saying things aren't cool. http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/src/nsExtensionManager.js.in#2716

This delays startup by 150ms.

Comment 1

[(dormant account)]

mfinkle points out that not having a .autoreg file causes the .rdf file to not be created.

Comment 2

[Dave Townsend [:mossop]]

So can I clarify, does extensions.rdf never get created, even after extensions have been installed?

Comment 3

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

(In reply to comment #2)
> So can I clarify, does extensions.rdf never get created, even after extensions
> have been installed?

For fennec:
- extensions.rdf does not appear in a fresh profile (no extensions)
- extensions.rdf does not appear in a not-fresh profile (still no extensions)
- extensions.rdf does appear in a profile after installing an extension

Comment 4

[Dave Townsend [:mossop]]

It's possible that I've missed something and we don't create it unless extensions are installed, but that isn't how I read the code. Does this happen with Firefox, and does it only happen with Fennec on the N810 or does it also happen with the desktop version?

One thought that springs to mind is that in Firefox we have the default theme in the app dir. This would effectively mean that we have an extension installed so maybe that could be a difference

Comment 5

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

(In reply to comment #4)
> Does this happen
> with Firefox, and does it only happen with Fennec on the N810 or does it also
> happen with the desktop version?

Same thing happens on the desktop

> 
> One thought that springs to mind is that in Firefox we have the default theme
> in the app dir. This would effectively mean that we have an extension installed
> so maybe that could be a difference

I bet you are correct

Comment 6

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Interesting one this. Because there are no add-ons in any install location, although we go through the trouble of initialising the datasource nothing is added to it so no explicit flush ever happens. This shouldn't be a problem though because just initialising the datasource creates the container in it and so when the inner nsRDFXMLDataSource is destroyed it will flush itself anyway. However the EM currently almost leaks this instance, or at least the nsRDFXMLDataSource is getting destroyed so late in shutdown that it is unable to write out.

This patch makes us properly free up references to the nsRDFXMLDataSource and between the EM and the EMDS during shutdown so this is no longer a problem. A couple of other minor cleanups in there too.

Comment 7

[Robert Strong (they/them - no direct email)]

Comment on attachment 348949 [details] [diff] [review]
patch rev 1

Looks good...

>diff --git a/toolkit/mozapps/extensions/src/nsExtensionManager.js.in b/toolkit/mozapps/extensions/src/nsExtensionManager.js.in
>--- a/toolkit/mozapps/extensions/src/nsExtensionManager.js.in
>+++ b/toolkit/mozapps/extensions/src/nsExtensionManager.js.in
>...
>@@ -8871,17 +8884,24 @@ ExtensionsDataSource.prototype = {
>     if (this._inner instanceof Ci.nsIRDFRemoteDataSource)
>       this._inner.Flush();
>   },
> 
>   FlushTo: function EMDS_FlushTo(uri) {
>   },
> 
>   QueryInterface: XPCOMUtils.generateQI([Ci.nsIRDFDataSource,
>-                                         Ci.nsIRDFRemoteDataSource])
>+                                         Ci.nsIRDFRemoteDataSource]),
>+  classDescription: "Extension Manager Data Source",
>+  contractID: "@mozilla.org/rdf/datasource;1?name=extensions",
>+  classID: Components.ID("{69BB8313-2D4F-45EC-97E0-D39DA58ECCE9}"),
>+  _xpcom_factory: {
>+    createInstance: function() Cc[ExtensionManager.prototype.contractID].
>+                               getService(Ci.nsIExtensionManager).datasource
>+  }
nit: for consistency can you leave QueryInterface: at the end?

Comment 8

[Dave Townsend [:mossop]]

Attachment: patch rev 2

Want to land this for 1.9.1 to improve startup performance for mobile and other xulrunner applications, should be a low risk but we can't automatically test this as the presence of the default theme in the firefox build bypasses this problem.

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 349146 [details] [diff] [review]
patch rev 2

a191=beltzner

Comment 10

[Dave Townsend [:mossop]]

Attachment: patch for checkin

Comment 11

[Shawn Wilsher :sdwilsh]

http://hg.mozilla.org/mozilla-central/rev/18ecd1a5fd1d

Comment 12

[Robert Kaiser]

As a note, this fixed large portions of bug 391318, a long-going leak in SeaMonkey, where we always reported to leak the whole EM RDF datasource due to it being destroyed very late as mentioned in comment #6.

Comment 13

[Dave Townsend [:mossop]]

(In reply to comment #12)
> As a note, this fixed large portions of bug 391318, a long-going leak in
> SeaMonkey, where we always reported to leak the whole EM RDF datasource due to
> it being destroyed very late as mentioned in comment #6.

Yeah I expected it might have some impact there, I was surprised it hadn't been done before since bz had pointed out its effect in bug 391318 comment 28

Comment 14

[Tony Chung [:tchung]]

Since there's no fennec 1.9.1 branch build, can joel or clint verify this fix on a trunk build?   Thanks