This is John's part 2 from bug 558184, spun out into a separate bug.

Comment on attachment 8568785 [details] [diff] [review] Attachment 8535419 [details] [diff] merged to tip >+++ b/dom/base/nsObjectLoadingContent.cpp >@@ -1611,18 +1592,19 @@ nsObjectLoadingContent::UpdateObjectPara >+ nsRefPtr<nsPluginHost> pluginHost = nsPluginHost::GetInst(); > if (StringBeginsWith(classIDAttr, NS_LITERAL_STRING("java:")) && >- PluginExistsForType(javaMIME)) { >+ pluginHost->HavePluginForType(javaMIME)) { Need to null-check pluginHost. >@@ -2663,17 +2645,19 @@ nsObjectLoadingContent::GetTypeOfContent >+ nsRefPtr<nsPluginHost> pluginHost = nsPluginHost::GetInst(); >+ if (caps & eSupportPlugins && >+ pluginHost->HavePluginForType(aMIMEType, nsPluginHost::eExcludeNone)) { And here. >@@ -3201,26 +3185,29 @@ nsObjectLoadingContent::ShouldPlay(Fallb >+ pluginHost->GetBlocklistStateForType(mContentType.get(), Why not have GetBlocklistStateForType also take an XPCOM string? >+++ b/dom/plugins/base/nsIPluginHost.idl >+ const uint32_t EXCLUDE_NONE = 0; >+ const uint32_t EXCLUDE_DISABLED = 1 << 0; So this means the default here will be EXCLUDE_NONE if the argument is not passed, whereas for the C++ bits that were added the default is EXCLUDE_DISABLED. That's probably OK, I guess. >+ // FIXME doc below four Added some documentation. >+++ b/dom/plugins/base/nsPluginHost.cpp >@@ -943,17 +938,18 @@ nsPluginHost::TrySetUpPluginInstance(con >+ const nsDependentCString type(aMimeType); What guarantees aMimeType is not null? Probably better to just change this to take an XPCOM string. >+nsPluginHost::FindNativePluginForType(const nsACString & aMimeType, >+ if (!aMimeType.Length()) { aMimeType.IsEmpty() >+nsPluginHost::FindNativePluginForExtension(const nsACString & aExtension, >+ if (!aExtension.Length()) { aExtension.IsEmpty() >@@ -1370,17 +1337,18 @@ nsresult nsPluginHost::GetPlugin(const c >+ const nsDependentCString type(aMimeType); Nothing guarantees aMimeType is non-null here. Luckily, this needs to change to take nsACString anyway for some of the above review comments. > nsPluginHost::SiteHasData(nsIPluginTag* plugin, const nsACString& domain, >+ // FIXME audit casts FIXME-jsplugins. Not an issue for now, obviously. Similar for other FIXMEs in this patch. >+++ b/dom/plugins/base/nsPluginTags.cpp >+// check comma delimitered extensions "delimited" >+ const char *pComma = strchr(pExt, ','); Seems like a use case for nsCCharSeparatedTokenizer. >+// Search for an extension in an extensions array, and optionally return its >+// matching mime type >+static bool SearchExtensions(const nsTArray<nsCString> & aExtensions, Returning the MIME type is not optional, right? >+ if (ExtensionInList(aExtensions[i].get(), aFindExtension.Data())) { .Data() is not safe, since the result might not be null-terminated. Better to just work with XPCOM strings throughout here. >+MakeNiceFileName(const nsCString & aFileName) >+ NS_ASSERTION(niceNameLength != kNotFound, "mFileName doesn't have a '.'?"); aFileName >+ // entire mFileName (which we've already taken care of, a few lines back) aFileName >+CStringArrayToXPCArray(nsTArray<nsCString> & aArray, >+ if (!count) { >+ return NS_ERROR_NOT_AVAILABLE; That's a behavior change... Maybe it's OK. >+ *aResults = >+ static_cast<char16_t**>(nsMemory::Alloc(count * sizeof(**aResults))); >+ if (!*aResults) nsMemory::Alloc is infallible. >+++ b/dom/plugins/base/nsPluginTags.h >+ bool HasMimeType(const nsACString & aMimeType) const; >+ bool HasExtension(const nsACString & aExtension, >+ /* out */ nsACString & aMatchingType) const; Needs more documentation. >+++ b/layout/build/nsContentDLF.cpp >+ const nsDependentCString contentType(aContentType); What guarantees that aContentType is not null? Filed bug 1136388 on this. >+++ b/uriloader/exthandler/nsExternalHelperAppService.cpp >- nsPluginHost* pluginHost = static_cast<nsPluginHost*>(pluginHostCOM.get()); >+ nsRefPtr<nsPluginHost> pluginHost = nsPluginHost::GetInst(); > if (NS_SUCCEEDED(rv)) { Need to null-check pluginHost now. r=me with the above fixed