Closed Bug 569644 Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: Plug-ins, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED WONTFIX

People

(Reporter: benjamin, Assigned: benjamin)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
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.
Attachment #448788 - Flags: review?(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?
Attachment #448788 - Flags: review?(bzbarsky) → review+
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.....
Oh, and perhaps nsnull?  Or are we using NULL freely now?
I'm using NULL in all new code, although I haven't updated the style guide.
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.
Blocks: 571259
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Could this have caused the 100ms startup regression in bug 574457?
Depends on: 574457
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.
Resolution: FIXED → WONTFIX
The indent on the last branch of the else if looks wrong in that backout diff.
You need to log in before you can comment on or make changes to this bug.