Bug 1530190. Use the image io thread to call SHGetFileInfo. r=aosmond
authorTimothy Nikkel <tnikkel@gmail.com>
Mon, 22 Apr 2019 17:30:41 -0500
changeset 474736 2d9c02e85710b2fa095f7542b0b58996c7e8a77a
parent 474735 8010faead0fd05a943e260dc6b5a7d9b024eef88
child 474737 642316b68e6a267ce07e99d5e7506119820b437b
push id113169
push usertnikkel@gmail.com
push dateTue, 21 May 2019 21:38:13 +0000
treeherdermozilla-inbound@17018058233c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaosmond
bugs1530190
milestone69.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 1530190. Use the image io thread to call SHGetFileInfo. r=aosmond Windows doesn't like more than one thread (at the same time?) calling SHGetFileInfoW. So we use the same thread to avoid this. We also need to call CoInitialize on the thread we do this. Differential Revision: https://phabricator.services.mozilla.com/D28423
image/DecodePool.cpp
image/decoders/icon/win/nsIconChannel.cpp
--- a/image/DecodePool.cpp
+++ b/image/DecodePool.cpp
@@ -21,16 +21,20 @@
 #include "nsIXULRuntime.h"
 
 #include "gfxPrefs.h"
 
 #include "Decoder.h"
 #include "IDecodingTask.h"
 #include "RasterImage.h"
 
+#if defined(XP_WIN)
+#  include <objbase.h>
+#endif
+
 using std::max;
 using std::min;
 
 namespace mozilla {
 namespace image {
 
 ///////////////////////////////////////////////////////////////////////////////
 // DecodePool implementation.
@@ -328,16 +332,31 @@ DecodePool* DecodePool::Singleton() {
   }
 
   return sSingleton;
 }
 
 /* static */
 uint32_t DecodePool::NumberOfCores() { return sNumCores; }
 
+#if defined(XP_WIN)
+class IOThreadIniter final : public Runnable {
+ public:
+  explicit IOThreadIniter() : Runnable("image::IOThreadIniter") {}
+
+  NS_IMETHOD Run() override {
+    MOZ_ASSERT(!NS_IsMainThread());
+
+    CoInitialize(nullptr);
+
+    return NS_OK;
+  }
+};
+#endif
+
 DecodePool::DecodePool() : mMutex("image::IOThread") {
   // Determine the number of threads we want.
   int32_t prefLimit = gfxPrefs::ImageMTDecodingLimit();
   uint32_t limit;
   if (prefLimit <= 0) {
     int32_t numCores = NumberOfCores();
     if (numCores <= 1) {
       limit = 1;
@@ -374,17 +393,26 @@ DecodePool::DecodePool() : mMutex("image
     idleTimeout = TimeDuration::FromMilliseconds(prefIdleTimeout);
     idleLimit = (limit + 1) / 2;
   }
 
   // Initialize the thread pool.
   mImpl = new DecodePoolImpl(limit, idleLimit, idleTimeout);
 
   // Initialize the I/O thread.
+#if defined(XP_WIN)
+  // On Windows we use the io thread to get icons from the system. Any thread
+  // that makes system calls needs to call CoInitialize. And these system calls
+  // (SHGetFileInfo) should only be called from one thread at a time, in case
+  // we ever create more than on io thread.
+  nsCOMPtr<nsIRunnable> initer = new IOThreadIniter();
+  nsresult rv = NS_NewNamedThread("ImageIO", getter_AddRefs(mIOThread), initer);
+#else
   nsresult rv = NS_NewNamedThread("ImageIO", getter_AddRefs(mIOThread));
+#endif
   MOZ_RELEASE_ASSERT(NS_SUCCEEDED(rv) && mIOThread,
                      "Should successfully create image I/O thread");
 
   nsCOMPtr<nsIObserverService> obsSvc = services::GetObserverService();
   if (obsSvc) {
     obsSvc->AddObserver(this, "xpcom-shutdown-threads", false);
   }
 }
--- a/image/decoders/icon/win/nsIconChannel.cpp
+++ b/image/decoders/icon/win/nsIconChannel.cpp
@@ -26,17 +26,16 @@
 #include "nsDirectoryServiceDefs.h"
 #include "nsProxyRelease.h"
 #include "nsContentSecurityManager.h"
 #include "nsContentUtils.h"
 #include "nsNetUtil.h"
 #include "nsThreadUtils.h"
 
 #include "Decoder.h"
-#include "IDecodingTask.h"
 #include "DecodePool.h"
 
 // we need windows.h to read out registry information...
 #include <windows.h>
 #include <shellapi.h>
 #include <shlobj.h>
 #include <objbase.h>
 #include <wchar.h>
@@ -61,51 +60,48 @@ struct ICONENTRY {
   uint32_t ieFileOffset;
 };
 
 // Match stock icons with names
 static SHSTOCKICONID GetStockIconIDForName(const nsACString& aStockName) {
   return aStockName.EqualsLiteral("uac-shield") ? SIID_SHIELD : SIID_INVALID;
 }
 
-class nsIconChannel::IconAsyncOpenTask final : public IDecodingTask {
+class nsIconChannel::IconAsyncOpenTask final : public Runnable {
  public:
-  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(IconAsyncOpenTask, override)
-
   IconAsyncOpenTask(nsIconChannel* aChannel, nsIEventTarget* aTarget,
                     nsCOMPtr<nsIFile>&& aLocalFile, nsAutoString& aPath,
                     UINT aInfoFlags)
-      : mChannel(aChannel),
+      : Runnable("IconAsyncOpenTask"),
+        mChannel(aChannel),
+
         mTarget(aTarget),
         mLocalFile(std::move(aLocalFile)),
         mPath(aPath),
         mInfoFlags(aInfoFlags) {}
 
-  void Run() override;
-  bool ShouldPreferSyncRun() const override { return false; }
-  TaskPriority Priority() const override { return TaskPriority::eLow; }
+  NS_IMETHOD Run() override;
 
  private:
-  ~IconAsyncOpenTask() override = default;
-
   RefPtr<nsIconChannel> mChannel;
   nsCOMPtr<nsIEventTarget> mTarget;
   nsCOMPtr<nsIFile> mLocalFile;
   nsAutoString mPath;
   UINT mInfoFlags;
 };
 
-void nsIconChannel::IconAsyncOpenTask::Run() {
+NS_IMETHODIMP nsIconChannel::IconAsyncOpenTask::Run() {
   HICON hIcon = nullptr;
   nsresult rv =
       mChannel->GetHIconFromFile(mLocalFile, mPath, mInfoFlags, &hIcon);
   nsCOMPtr<nsIRunnable> task = NewRunnableMethod<HICON, nsresult>(
       "nsIconChannel::FinishAsyncOpen", mChannel,
       &nsIconChannel::FinishAsyncOpen, hIcon, rv);
   mTarget->Dispatch(task.forget(), NS_DISPATCH_NORMAL);
+  return NS_OK;
 }
 
 // nsIconChannel methods
 nsIconChannel::nsIconChannel() {}
 
 nsIconChannel::~nsIconChannel() {
   if (mLoadInfo) {
     NS_ReleaseOnMainThreadSystemGroup("nsIconChannel::mLoadInfo",
@@ -417,19 +413,20 @@ nsresult nsIconChannel::GetHIconFromFile
     mimeService->GetPrimaryExtension(contentType, fileExt, defFileExt);
     // If the mime service does not know about this mime type, we show
     // the generic icon.
     // In any case, we need to insert a '.' before the extension.
     filePath = NS_LITERAL_STRING(".") + NS_ConvertUTF8toUTF16(defFileExt);
   }
 
   if (aNonBlocking) {
+    RefPtr<nsIEventTarget> target = DecodePool::Singleton()->GetIOEventTarget();
     RefPtr<IconAsyncOpenTask> task = new IconAsyncOpenTask(
         this, mListenerTarget, std::move(localFile), filePath, infoFlags);
-    DecodePool::Singleton()->AsyncRun(task);
+    target->Dispatch(task.forget(), NS_DISPATCH_NORMAL);
     return NS_OK;
   }
 
   return GetHIconFromFile(localFile, filePath, infoFlags, hIcon);
 }
 
 nsresult nsIconChannel::GetHIconFromFile(nsIFile* aLocalFile,
                                          const nsAutoString& aPath,