569644 - Plugin content viewers should be looked up dynamically instead of using category entries

Status: RESOLVED WONTFIX

(Core Graveyard :: Plug-ins, defect)

Description

[Benjamin Smedberg]

In bug 568691, I'm changing the category manager so that entries cannot be registered dynamically: they can only be registered statically at component registration time. This affects the way the plugin host registers content viewers; currently they are registered in the Gecko-Content-Viewers category dynamically, based on the list of installed plugins. This patch changes things so that we explicitly ask the plugin host whether a plugin is available for a specified type.

Comment 1

[Benjamin Smedberg]

Attachment: Query the plugin host directly for content viewers, rev. 1

We have a general test that full-page plugins work, but I'm still working up a new test to check the fullpage-enabled pref.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 448788 [details] [diff] [review]
Query the plugin host directly for content viewers, rev. 1

I'd prefer we move over the CONTENT_DLF_CONTRACT and PLUGIN_DLF_CONTRACT defines, esp so that we can avoid the two copies of the plugin contract string.

In the docs for the FullPagePluginEnabledType enum, for AVAILABLE, "viewers" should be "viewer".  Or the "a" should be removed.

In nsIPluginHost, we need to rev the iid. And put the new method at the end, ideally.

It might be a good idea to just nix mOverrideInternalTypes, as a followup.

Does it make sense to continue supporting other things that may want to enable/disable viewers somehow?  Perhaps a followup bug on that?

Comment 3

[Boris Zbarsky [:bzbarsky]]

So for example we could extend the plugin host api we added to something others could implement, and have a category of things to ask for "extra" viewers or something.....

Comment 4

[Boris Zbarsky [:bzbarsky]]

Oh, and perhaps nsnull?  Or are we using NULL freely now?

Comment 5

[Benjamin Smedberg]

I'm using NULL in all new code, although I haven't updated the style guide.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Did we decide that at some point and I just missed it?  I don't have a problem either way, but it would be good to make this widely known if it's the new policy.

Comment 7

[Benjamin Smedberg]

http://hg.mozilla.org/mozilla-central/rev/0327e126ea24

Filed bug 571259 for the followup.

Comment 8

[Dietrich Ayala (:dietrich)]

Could this have caused the 100ms startup regression in bug 574457?

Comment 9

[Benjamin Smedberg]

Backed parts of this out at http://hg.mozilla.org/mozilla-central/rev/d19072babaee

I left the contentutils helper method in, but reverted the plugin changes so that we use the category manager again. In static-xpcom-registration I've had to re-implement dynamic category registration (passing persist=false), so I think this is ok.

Comment 10

[Boris Zbarsky [:bzbarsky]]

The indent on the last branch of the else if looks wrong in that backout diff.