Splitting out from bug 558184 - various spots in the tree are checking mime type strings for java/flash, some less consistently than others.

Adds nsPluginHost::IsSpecialType() to check for all the various MIME types we sniff, removes a ton of said checks from various spots. Many checks were not checking all possible mime types.

New and improved, now compiles on OSes other than my own.

(In reply to John Schoenick [:johns] from comment #3) > https://tbpl.mozilla.org/?tree=Try&rev=10c9cfce49c7 (In reply to John Schoenick [:johns] from comment #4) > Created attachment 8486591 [details] [diff] [review] > Move checking for special-cased plugin types to a central spot > > New and improved, now compiles on OSes other than my own. Let's try that again https://tbpl.mozilla.org/?tree=Try&rev=a9f6fe815968

And then we found out that Preferences::GetCString() can return invalid strings that crash. Woo. Attempt #3 went better https://tbpl.mozilla.org/?tree=Try&rev=8222336cd7d4 https://tbpl.mozilla.org/?tree=Try&rev=96ea8a21e1c5

Comment on attachment 8487354 [details] [diff] [review] Move checking for special-cased plugin types to a central spot Review of attachment 8487354 [details] [diff] [review]: ----------------------------------------------------------------- That's a nice cleanup, adding some driveby commentary :) ::: content/base/src/nsObjectLoadingContent.cpp @@ +115,5 @@ > > static bool > +IsJavaMIME(const nsACString & aMIMEType) > +{ > + return nsPluginHost::IsSpecialType(aMIMEType) == \ Nit: Left-over back-slash? ::: dom/plugins/base/nsNPAPIPluginInstance.cpp @@ +537,5 @@ > // bug 108347, flash plugin on linux doesn't like window->width <= > // 0, but Java needs wants this call. > + if (window && window->type == NPWindowTypeWindow && > + (window->width <= 0 || window->height <= 0) && > + (nsPluginHost::IsSpecialType(nsDependentCString(mMIMEType)) != \ Nit: Left-over backslash? ::: dom/plugins/base/nsPluginHost.cpp @@ +1577,5 @@ > + return eSpecialType_Silverlight; > + } > + > + if (aMIMEType.LowerCaseEqualsASCII("audio/x-pn-realaudio-plugin")) { > + NS_WARNING("You are loading real player"); Nit: Why does this plugin specifically get a warning? ::: dom/plugins/base/nsPluginHost.h @@ +172,5 @@ > + // Native widget quirks > + eSpecialType_PDF, > + // Native widget quirks > + eSpecialType_RealPlayer }; > + static SpecialType IsSpecialType(const nsACString & aMIMEType); Usually |IsFoo()| functions would be expected to return a boolean though, maybe |GetSpecialType()|? ::: dom/plugins/base/nsPluginInstanceOwner.cpp @@ +830,5 @@ > > // Flash invokes InvalidateRect for us. > const char* mime = nullptr; > + if (NS_SUCCEEDED(mInstance->GetMIMEType(&mime)) && mime && > + nsPluginHost::IsSpecialType(nsDependentCString(mime)) == \ Nit: backslash ::: dom/plugins/base/nsPluginTags.cpp @@ +162,5 @@ > + break; > + case nsPluginHost::eSpecialType_Flash: > + mIsFlashPlugin = true; > + break; > + case nsPluginHost::eSpecialType_None: This line seems redundant?

"You are loading real player" should probably be "You are loading RealPlayer".

Comment on attachment 8487354 [details] [diff] [review] Move checking for special-cased plugin types to a central spot Review of attachment 8487354 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/plugins/base/nsPluginTags.cpp @@ +155,5 @@ > continue; > } > > // Look for certain special plugins. > + switch (nsPluginHost::IsSpecialType(mimeType)) { Looking here again from the bug 1074822 context... can't we just store this as |nsPluginTag::mSpecialType| instead and get rid of mIsFlashPlugin & mIsJavaPlugin and the switching here? We could still have helper functions for convenience if needed.

Boris: bsmedberg r+'d this bug's jsplugin-related patches before johns left, but they never landed. Should these patches land with your jsplugins work?

Probably, yes. Thanks for the pointer!

Addresses the review comments, except comment 9: we can't really use nsPluginHost::SpecialType there, so needs a bit more thought on how best to store this

https://hg.mozilla.org/integration/mozilla-inbound/rev/469e07a25f44 https://hg.mozilla.org/integration/mozilla-inbound/rev/234d306dfbba

https://hg.mozilla.org/mozilla-central/rev/469e07a25f44 https://hg.mozilla.org/mozilla-central/rev/234d306dfbba