905357 - Unable to remove content policy registrations synchronously at runtime

Status: RESOLVED DUPLICATE of bug 753687

(Core :: Security, defect)

Description

[felix.shnir]

Attachment: ghostery.xpi

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/28.0.1500.95 Safari/537.36

Steps to reproduce:

Upgraded an existing addon-sdk built addon that uses content policy


Actual results:

previous content policy is never removed even tho there is relevant code that deletes xpcom category and unregisters the module


Expected results:

Should have been removed or replaced with new content policy.

Comment 1

[felix.shnir]

If you open up the xpi, relevant code is in the background.js lines 130-164.

Because previous policies are never removed, code that should not throw errors will throw dead object reference errors. In most cases, its harmless, but not so for redirects (net-channel-event-sinks) where a thrown error prevents all redirects.

Comment 2

[felix.shnir]

To reproduce, install the attached XPI once, and then install it again without a restart. Browse to any site and look at the error console for xpcom errors.

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Dave, you were talking with the reporter on IRC about this the other day, right?  Any information you can add to this?

Comment 4

[Dave Townsend [:mossop]]

Yes and as far as I can tell this is a core XPCOM bug as I've seen this outside the SDK.

Comment 5

[Benjamin Smedberg]

What precisely is the XPCOM bug? Is the entry not being removed from the category manager? Or is that not firing a notification?

Comment 6

[felix.shnir]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #5)
> What precisely is the XPCOM bug? Is the entry not being removed from the
> category manager? Or is that not firing a notification?

Correct, the entry was not removed.

Comment 7

[Benjamin Smedberg]

Can you provide a minimal testcase (xpcshell would be awesome): the code for DeleteCategoryEntry exists and we even appear to have automated tests for it, so I suspect that is not actually the problem here.

Comment 8

[Dave Townsend [:mossop]]

Attachment: testcase

To clarify the problem is that if you remove the category registration the content policy remains active for a time. The attached testcase verifies that behaviour. What would be better is if the test would pass without the do_execute_soons.

Comment 9

[Benjamin Smedberg]

I expect that synchronous might be hard, or might require that we replace using the category manager with something more proactive. In any case, moving this to Core:Security since it isn't an XPCOM bug.

Comment 10

[Dave Townsend [:mossop]]

Attachment: testcase

I forgot the real problem that this causes. Whether content policies can be removed synchronously or not probably doesn't matter. The problem is that if you attempt to remove the category entry and unregister the component from the component manager then (as a restartless add-on would do to clean up when it is disabled) then something breaks and the content policy service continues to use its reference to the component. This attached testcase fails.

Comment 11

[:Irving Reid (No longer working on Firefox)]

This sounds similar to bug 753687, and I suspect both are implicated in bug 924340.

Comment 12

[Dave Townsend [:mossop]]

I think they're the same really