Closed Bug 1079451 Opened 10 years ago Closed 10 years ago

Log spew "No valid manifest directive." when uninstalling an add-on

Categories

(Core :: XPCOM, defect)

All
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox32 --- unaffected
firefox33 --- unaffected
firefox34 --- affected
firefox35 --- affected
fennec 34+ ---

People

(Reporter: Margaret, Assigned: mossop)

References

Details

(Keywords: regression)

Attachments

(1 file)

I'm seeing this warning fill the log when uninstalling an add-on. Seems to happen with any add-on.

[JavaScript Warning: "No valid manifest directive." {file: "jar:jar:file:///data/app/org.mozilla.fennec_leibovic-1.apk!/assets/omni.ja!/components/components.manifest" line: 271}]

mfinkle, do you know what this is about? Or where we could look to debug it?
Flags: needinfo?(mark.finkle)
Before all the warnings start, I also see this:

I/GeckoConsole(25069): Could not read chrome manifest 'file:///data/data/org.mozilla.fennec_leibovic/chrome.manifest'.
(In reply to Mark Finkle (:mfinkle) from comment #2)

> Eventually shows up in XPIProvider.jsm as nsJar.getSigningCert in two places:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> internal/XPIProvider.jsm#1219
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> internal/XPIProvider.jsm#5274

Both of those are from bug 1050518. I wonder if this is a regression.
Hm, this happens on Nightly/Aurora, but not Beta/Release, so I don't think it's a regression from that bug.

Mossop, do you know what this could be about?
tracking-fennec: --- → ?
Flags: needinfo?(dtownsend+bugmail)
Keywords: regression
tracking-fennec: ? → 34+
Looks like bug caused this. Affects desktop too.
Blocks: 977026
Component: Add-on Manager → XPCOM
Flags: needinfo?(dtownsend+bugmail) → needinfo?(tlee)
Product: Firefox for Android → Core
Version: Firefox 35 → Trunk
That should have said bug 977026
I'm going to test a patch that backs out the logging from that patch, it looks unnecessary to me.
Assignee: nobody → dtownsend+bugmail
Attached patch patch rev 1Splinter Review
The test for directive->mgrfunc is unnecessary, every directive has either directive->mgrfunc or directive->regfunc. The logging only happens when the directive is for component registration and we are doing chrome-only registration so is not an error.
Attachment #8504142 - Flags: review?(benjamin)
Comment on attachment 8504142 [details] [diff] [review]
patch rev 1

Since not all directives have a mgrfunc, but we are using ->mgrfunc in this case, what makes this safe instead of potentially introducing a null-deref?
Flags: needinfo?(dtownsend+bugmail)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #9)
> Comment on attachment 8504142 [details] [diff] [review]
> patch rev 1
> 
> Since not all directives have a mgrfunc, but we are using ->mgrfunc in this
> case, what makes this safe instead of potentially introducing a null-deref?

It's safe because this is the else case from the previous "if (directive->regfunc)": http://mxr.mozilla.org/mozilla-central/source/xpcom/components/ManifestParser.cpp#752

All directives have either a mgrfunc or regfunc so if regfunc is null mgrfunc must not be null.
Flags: needinfo?(dtownsend+bugmail)
(In reply to Dave Townsend [:mossop] from comment #8)
> Created attachment 8504142 [details] [diff] [review]
> patch rev 1

This patch seems to fix the issue.  My original idea is to print log for every directive not handled.  Since it annoys people, we should remove it, and I am sorry for that.
But, I suggest to have an assertion to make sure kParsingTable is always correct for debug build.
Flags: needinfo?(tlee)
Attachment #8504142 - Flags: review?(benjamin) → review+
https://hg.mozilla.org/mozilla-central/rev/b008a281364d
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.