Closed Bug 1314955 Opened 8 years ago Closed 8 years ago

Remove support for binary-component manifest instruction

Categories

(Core :: XPCOM, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: benjamin, Assigned: benjamin)

References

Details

Attachments

(3 files)

We no longer want to allow binary components at all. Currently we disallow them in extensions but still allow them in app code. This will remove support completely.
Comment on attachment 8807161 [details] Bug 1314955 part A - Move the binary xpconnect testing components into libxul so that we can remove support for external binary components. https://reviewboard.mozilla.org/r/90424/#review90174
Attachment #8807161 - Flags: review?(mrbkap) → review+
Just to be clear, this will heavily impact Thunderbird, which still relies on (some) binary extensions, yes? Is it feasible to delay this until 53, so Thunderbird can have an ESR cycle to deal with fallout, or is there immediate work that requires binary components to go away Right Now? CC'ing some thunderbird people. (Yes, yes, I know we're not *supposed* to care about Thunderbird, but this is a bigger deal than typical refactorings.)
Flags: needinfo?(benjamin)
According to the dev.platform thread about this, "we [thunderbird devs] have agreed for some months now that Thunderbird 52 will no longer support binary extensions." https://groups.google.com/d/msg/mozilla.dev.platform/qB-9gJl9Jgs/nkb29idgBwAJ They "would like" to wait until 53 but can live without, and in terms of hardening Firefox I really want to have this take effect for ESR52 in Firefox, so I'd like to land this now.
Flags: needinfo?(benjamin)
Comment on attachment 8807162 [details] Bug 1314955 part B - Remove the tests for binary-component which is no longer supported. https://reviewboard.mozilla.org/r/90426/#review90190
Attachment #8807162 - Flags: review?(nfroyd) → review+
Comment on attachment 8807163 [details] Bug 1314955 part C - Make the `binary-component` manifest instruction obsolete: it will continue to be parsed and will report a deprecation notice to the browser console. https://reviewboard.mozilla.org/r/90428/#review90194 You mentioned that you intend for `binary-component` bits in the manifest to print an error to the console; did you intend for that error message to come from `ManifestBinaryComponent`, or the generic parsing code that complains about unknown directives? r-'ing because I'd like clarification on that point. (I think it would be equally straightforward to hide the `binary-component` directive behind a flag, for which the Thunderbird developers could implement appropriate configury. But meh, I suppose there'd be a fair amount of stuff to gate off that flag...) ::: xpcom/components/nsComponentManager.cpp:626 (Diff revision 1) > nsComponentManagerImpl::ManifestBinaryComponent(ManifestProcessingContext& aCx, > int aLineNo, > char* const* aArgv) AFAICT, this function is dead code; the only reference to it was the `kParsingTable` entry in ManifestParser.cpp, which is also removed by this patch.
Attachment #8807163 - Flags: review?(nfroyd) → review-
(In reply to Benjamin Smedberg [:bsmedberg] from comment #6) > According to the dev.platform thread about this, "we [thunderbird devs] have > agreed for some months now that Thunderbird 52 will no longer support binary > extensions." > https://groups.google.com/d/msg/mozilla.dev.platform/qB-9gJl9Jgs/nkb29idgBwAJ > > They "would like" to wait until 53 but can live without, and in terms of > hardening Firefox I really want to have this take effect for ESR52 in > Firefox, so I'd like to land this now. Thank you for the pointer! I had not fully internalized that mailing list thread.
Attachment #8807161 - Attachment is obsolete: true
Attachment #8807162 - Attachment is obsolete: true
Comment on attachment 8807163 [details] Bug 1314955 part C - Make the `binary-component` manifest instruction obsolete: it will continue to be parsed and will report a deprecation notice to the browser console. https://reviewboard.mozilla.org/r/90428/#review91650 Thanks!
Attachment #8807163 - Flags: review?(nfroyd) → review+
Attachment #8807161 - Attachment is obsolete: false
Attachment #8807162 - Attachment is obsolete: false
You didn't land this on Gecko52 after all despite comment #6(?).
No I ran out of time and there are tests still orange from my tree.
Pushed by bsmedberg@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c78f97c0b771 part A - Move the binary xpconnect testing components into libxul so that we can remove support for external binary components. Ideally we'd only compile these into the xul-gtest version, but currently we can't run xpcshell tests against that version, so I'm going to put them into the release xul, r=mrbkap https://hg.mozilla.org/integration/mozilla-inbound/rev/a99c6ce96b6f part B - Remove the tests for binary-component which is no longer supported. r=froydnj https://hg.mozilla.org/integration/mozilla-inbound/rev/b99edf918b96 part C - Make the `binary-component` manifest instruction obsolete: it will continue to be parsed and will report a deprecation notice to the browser console. r=froydnj
Sorry had to back out these for Android XPCShell failures, i.e., https://treeherder.mozilla.org/logviewer.html#?job_id=39253398&repo=mozilla-inbound#L1603
Flags: needinfo?(benjamin)
Backout by ihsiao@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/024dd64e56c9 Backed out changeset b99edf918b96 for Android XPCShell failures https://hg.mozilla.org/integration/mozilla-inbound/rev/ee36b7c4f4be Backed out changeset a99c6ce96b6f https://hg.mozilla.org/integration/mozilla-inbound/rev/8e8aa918d2f5 Backed out changeset c78f97c0b771
Pushed by bsmedberg@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/82f8e8d7eb68 part A - Move the binary xpconnect testing components into libxul so that we can remove support for external binary components. Ideally we'd only compile these into the xul-gtest version, but currently we can't run xpcshell tests against that version, so I'm going to put them into the release xul, r=mrbkap https://hg.mozilla.org/integration/mozilla-inbound/rev/c1554712210f part B - Remove the tests for binary-component which is no longer supported. r=froydnj https://hg.mozilla.org/integration/mozilla-inbound/rev/25f407284342 part C - Make the `binary-component` manifest instruction obsolete: it will continue to be parsed and will report a deprecation notice to the browser console. r=froydnj
Depends on: 1318195
Flags: needinfo?(benjamin)
Blocks: 1330947
Blocks: 1335163
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: