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 298779 a03bd5fc36c0cd66762e0d41e5115900079d83fb
parent 298778 b8daa119382d762bba06f077541e360b5e2ba870
child 298780 2f3fc8f9ec43faf0c41be3a55d51f9d604be0e0d
push id77309
push userjwatt@jwatt.org
push dateTue, 24 May 2016 21:42:11 +0000
treeherdermozilla-inbound@2f3fc8f9ec43 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstikkel
bugs1275229
milestone49.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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,