Bug 1275229 - Stop using do_CreateInstance("@mozilla.org/image/loader;1") to create gecko internal imgLoader instances. r=tikkel
authorJonathan Watt <jwatt@jwatt.org>
Thu, 19 May 2016 13:31:15 +0100
changeset 327780 a03bd5fc36c0cd66762e0d41e5115900079d83fb
parent 327777 b8daa119382d762bba06f077541e360b5e2ba870
child 327781 2f3fc8f9ec43faf0c41be3a55d51f9d604be0e0d
push idunknown
push userunknown
push dateunknown
reviewerstikkel
bugs1275229
milestone49.0a1
Bug 1275229 - Stop using do_CreateInstance("@mozilla.org/image/loader;1") to create gecko internal imgLoader instances. r=tikkel
image/build/nsImageModule.cpp
image/build/nsImageModule.h
image/imgLoader.cpp
image/imgLoader.h
--- a/image/build/nsImageModule.cpp
+++ b/image/build/nsImageModule.cpp
@@ -24,16 +24,18 @@
 #include "nsICOEncoder.h"
 #include "nsPNGEncoder.h"
 #include "nsJPEGEncoder.h"
 #include "nsBMPEncoder.h"
 
 // objects that just require generic constructors
 using namespace mozilla::image;
 
+// XXX We would like to get rid of the imgLoader factory constructor.  See the
+// comment documenting the imgLoader constructor.
 NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(imgLoader, Init)
 NS_GENERIC_FACTORY_CONSTRUCTOR(imgRequestProxy)
 NS_GENERIC_FACTORY_CONSTRUCTOR(imgTools)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsICOEncoder)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsJPEGEncoder)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsPNGEncoder)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsBMPEncoder)
 NS_DEFINE_NAMED_CID(NS_IMGLOADER_CID);
@@ -80,19 +82,24 @@ static const mozilla::Module::CategoryEn
   { "Gecko-Content-Viewers", IMAGE_PNG, "@mozilla.org/content/document-loader-factory;1" },
   { "Gecko-Content-Viewers", IMAGE_X_PNG, "@mozilla.org/content/document-loader-factory;1" },
   { "content-sniffing-services", "@mozilla.org/image/loader;1", "@mozilla.org/image/loader;1" },
   { nullptr }
 };
 
 static bool sInitialized = false;
 nsresult
-mozilla::image::InitModule()
+mozilla::image::EnsureModuleInitialized()
 {
   MOZ_ASSERT(NS_IsMainThread());
+
+  if (sInitialized) {
+    return NS_OK;
+  }
+
   // Make sure the preferences are initialized
   gfxPrefs::GetSingleton();
 
   mozilla::image::ShutdownTracker::Initialize();
   mozilla::image::ImageFactory::Initialize();
   mozilla::image::DecodePool::Initialize();
   mozilla::image::SurfaceCache::Initialize();
   mozilla::image::SurfacePipe::Initialize();
@@ -113,16 +120,16 @@ mozilla::image::ShutdownModule()
 }
 
 static const mozilla::Module kImageModule = {
   mozilla::Module::kVersion,
   kImageCIDs,
   kImageContracts,
   kImageCategories,
   nullptr,
-  mozilla::image::InitModule,
+  mozilla::image::EnsureModuleInitialized,
   // We need to be careful about shutdown ordering to avoid intermittent crashes
   // when hashtable enumeration decides to destroy modules in an unfortunate
   // order. So our shutdown is invoked explicitly during layout module shutdown.
   nullptr
 };
 
 NSMODULE_DEFN(nsImageLib2Module) = &kImageModule;
--- a/image/build/nsImageModule.h
+++ b/image/build/nsImageModule.h
@@ -6,15 +6,15 @@
 #ifndef mozilla_image_build_nsImageModule_h
 #define mozilla_image_build_nsImageModule_h
 
 #include "nsError.h"
 
 namespace mozilla {
 namespace image {
 
-nsresult InitModule();
+nsresult EnsureModuleInitialized();
 void ShutdownModule();
 
 } /* namespace image */
 } /* namespace mozilla */
 
 #endif // mozilla_image_build_nsImageModule_h
--- a/image/imgLoader.cpp
+++ b/image/imgLoader.cpp
@@ -6,16 +6,17 @@
 
 #include "mozilla/Attributes.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/Move.h"
 #include "mozilla/Preferences.h"
  #include "mozilla/ChaosMode.h"
 
 #include "ImageLogging.h"
+#include "nsImageModule.h"
 #include "nsPrintfCString.h"
 #include "imgLoader.h"
 #include "imgRequestProxy.h"
 
 #include "nsCOMPtr.h"
 
 #include "nsContentPolicyUtils.h"
 #include "nsContentUtils.h"
@@ -1131,30 +1132,44 @@ uint32_t imgLoader::sCacheMaxSize;
 imgMemoryReporter* imgLoader::sMemReporter;
 
 NS_IMPL_ISUPPORTS(imgLoader, imgILoader, nsIContentSniffer, imgICache,
                   nsISupportsWeakReference, nsIObserver)
 
 static imgLoader* gSingleton = nullptr;
 static imgLoader* gPBSingleton = nullptr;
 
+/* static */ already_AddRefed<imgLoader>
+imgLoader::CreateImageLoader()
+{
+  // In some cases, such as xpctests, XPCOM modules are not automatically
+  // initialized.  We need to make sure that our module is initialized before
+  // we hand out imgLoader instances and code starts using them.
+  mozilla::image::EnsureModuleInitialized();
+
+  RefPtr<imgLoader> loader = new imgLoader();
+  loader->Init();
+
+  return loader.forget();
+}
+
 imgLoader*
 imgLoader::Singleton()
 {
   if (!gSingleton) {
-    gSingleton = imgLoader::Create().take();
+    gSingleton = CreateImageLoader().take();
   }
   return gSingleton;
 }
 
 imgLoader*
 imgLoader::PBSingleton()
 {
   if (!gPBSingleton) {
-    gPBSingleton = imgLoader::Create().take();
+    gPBSingleton = CreateImageLoader().take();
     gPBSingleton->RespectPrivacyNotifications();
   }
   return gPBSingleton;
 }
 
 imgLoader::imgLoader()
 : mUncachedImagesMutex("imgLoader::UncachedImages"), mRespectPrivacy(false)
 {
--- a/image/imgLoader.h
+++ b/image/imgLoader.h
@@ -244,34 +244,44 @@ public:
   NS_DECL_IMGILOADER
   NS_DECL_NSICONTENTSNIFFER
   NS_DECL_IMGICACHE
   NS_DECL_NSIOBSERVER
 
   static imgLoader* Singleton();
   static imgLoader* PBSingleton();
 
+  /**
+   * Gecko code should use Singleton() or PBSingleton() to get an image loader.
+   *
+   * This constructor is public because the XPCOM module code that creates
+   * instances of "@mozilla.org/image/loader;1" / "@mozilla.org/image/cache;1"
+   * for nsIComponentManager.createInstance()/nsIServiceManager.getService()
+   * calls (now only made by add-ons) needs access to it.
+   *
+   * XXX We would like to get rid of the nsIServiceManager.getService (and
+   * nsIComponentManager.createInstance) method of creating imgLoader objects,
+   * but there are add-ons that are still using it.  These add-ons don't
+   * actually do anything useful with the loaders that they create since nobody
+   * who creates an imgLoader using this method actually QIs to imgILoader and
+   * loads images.  They all just QI to imgICache and either call clearCache()
+   * or findEntryProperties().  Since they're doing this on an imgLoader that
+   * has never loaded images, these calls are useless.  It seems likely that
+   * the code that is doing this is just legacy code left over from a time when
+   * there was only one imgLoader instance for the entire process.  (Nowadays
+   * the correct method to get an imgILoader/imgICache is to call
+   * imgITools::getImgCacheForDocument/imgITools::getImgLoaderForDocument.)
+   * All the same, even though what these add-ons are doing is a no-op,
+   * removing the nsIServiceManager.getService method of creating/getting an
+   * imgLoader objects would cause an exception in these add-ons that could
+   * break things.
+   */
   imgLoader();
-
   nsresult Init();
 
-  static already_AddRefed<imgLoader>
-  Create()
-  {
-      // Unfortunately, we rely on XPCOM module init happening
-      // before imgLoader creation. For now, it's easier
-      // to just call CallCreateInstance() which will init
-      // the image module instead of calling new imgLoader
-      // directly.
-      nsCOMPtr<imgILoader> loader = do_CreateInstance("@mozilla.org/image/loader;1");
-      // There's only one imgLoader implementation so we
-      // can safely cast to it.
-      return loader.forget().downcast<imgLoader>();
-  }
-
   nsresult LoadImage(nsIURI* aURI,
                      nsIURI* aInitialDocumentURI,
                      nsIURI* aReferrerURI,
                      ReferrerPolicy aReferrerPolicy,
                      nsIPrincipal* aLoadingPrincipal,
                      nsILoadGroup* aLoadGroup,
                      imgINotificationObserver* aObserver,
                      nsINode* aContext,
@@ -365,16 +375,18 @@ public:
   // HasObservers(). The request's cache entry will be re-set before this
   // happens, by calling imgRequest::SetCacheEntry() when an entry with no
   // observers is re-requested.
   bool SetHasNoProxies(imgRequest* aRequest, imgCacheEntry* aEntry);
   bool SetHasProxies(imgRequest* aRequest);
 
 private: // methods
 
+  static already_AddRefed<imgLoader> CreateImageLoader();
+
   bool ValidateEntry(imgCacheEntry* aEntry, nsIURI* aKey,
                      nsIURI* aInitialDocumentURI, nsIURI* aReferrerURI,
                      ReferrerPolicy aReferrerPolicy,
                      nsILoadGroup* aLoadGroup,
                      imgINotificationObserver* aObserver, nsISupports* aCX,
                      nsLoadFlags aLoadFlags,
                      nsContentPolicyType aContentPolicyType,
                      bool aCanMakeNewChannel,