Bug 988410 - Move directory service calls onto MainThread. r=bent, a=lsblakk
authorDave Hylands <dhylands@mozilla.com>
Fri, 04 Apr 2014 12:16:16 -0700
changeset 192149 bd93ca95b9b6d5938b5153001f1d31099aece27d
parent 192148 751264a8a3a073b2f10d6d25cb8a29d6cde3335f
child 192150 36f67ce468559a23811361fad03991046777905b
push id3503
push userraliiev@mozilla.com
push dateMon, 28 Apr 2014 18:51:11 +0000
treeherdermozilla-beta@c95ac01e332e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbent, lsblakk
bugs988410
milestone30.0a2
Bug 988410 - Move directory service calls onto MainThread. r=bent, a=lsblakk
ipc/glue/GeckoChildProcessHost.cpp
ipc/glue/GeckoChildProcessHost.h
--- a/ipc/glue/GeckoChildProcessHost.cpp
+++ b/ipc/glue/GeckoChildProcessHost.cpp
@@ -16,24 +16,26 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/process_watcher.h"
 #ifdef MOZ_WIDGET_COCOA
 #include "chrome/common/mach_ipc_mac.h"
 #include "base/rand_util.h"
 #include "nsILocalFileMac.h"
 #endif
 
+#include "MainThreadUtils.h"
 #include "prprf.h"
 #include "prenv.h"
 
 #include "nsExceptionHandler.h"
 
 #include "nsDirectoryServiceDefs.h"
 #include "nsIFile.h"
 
+#include "mozilla/ClearOnShutdown.h"
 #include "mozilla/ipc/BrowserProcessSubThread.h"
 #include "mozilla/Omnijar.h"
 #include <sys/stat.h>
 
 #ifdef XP_WIN
 #include "nsIWinTaskbar.h"
 #define NS_TASKBAR_CONTRACTID "@mozilla.org/windows-taskbar;1"
 #endif
@@ -57,16 +59,19 @@ static const bool kLowRightsSubprocesses
   // have no plugins or extensions to worry about breaking.
 #ifdef MOZ_WIDGET_GONK
   true
 #else
   false
 #endif
   ;
 
+mozilla::StaticRefPtr<nsIFile> GeckoChildProcessHost::sGreDir;
+mozilla::DebugOnly<bool> GeckoChildProcessHost::sGreDirCached;
+
 static bool
 ShouldHaveDirectoryService()
 {
   return GeckoProcessType_Default == XRE_GetProcessType();
 }
 
 template<>
 struct RunnableMethodTraits<GeckoChildProcessHost>
@@ -115,39 +120,36 @@ GeckoChildProcessHost::~GeckoChildProces
     );
 
 #if defined(MOZ_WIDGET_COCOA)
   if (mChildTask != MACH_PORT_NULL)
     mach_port_deallocate(mach_task_self(), mChildTask);
 #endif
 }
 
-void GetPathToBinary(FilePath& exePath)
+//static
+void
+GeckoChildProcessHost::GetPathToBinary(FilePath& exePath)
 {
   if (ShouldHaveDirectoryService()) {
-    nsCOMPtr<nsIProperties> directoryService(do_GetService(NS_DIRECTORY_SERVICE_CONTRACTID));
-    NS_ASSERTION(directoryService, "Expected XPCOM to be available");
-    if (directoryService) {
-      nsCOMPtr<nsIFile> greDir;
-      nsresult rv = directoryService->Get(NS_GRE_DIR, NS_GET_IID(nsIFile), getter_AddRefs(greDir));
-      if (NS_SUCCEEDED(rv)) {
+    MOZ_ASSERT(sGreDirCached);
+    if (sGreDir) {
 #ifdef OS_WIN
-        nsString path;
-        greDir->GetPath(path);
+      nsString path;
+      MOZ_ALWAYS_TRUE(NS_SUCCEEDED(sGreDir->GetPath(path)));
 #else
-        nsCString path;
-        greDir->GetNativePath(path);
+      nsCString path;
+      MOZ_ALWAYS_TRUE(NS_SUCCEEDED(sGreDir->GetNativePath(path)));
 #endif
-        exePath = FilePath(path.get());
+      exePath = FilePath(path.get());
 #ifdef MOZ_WIDGET_COCOA
-        // We need to use an App Bundle on OS X so that we can hide
-        // the dock icon. See Bug 557225.
-        exePath = exePath.AppendASCII(MOZ_CHILD_PROCESS_BUNDLE);
+      // We need to use an App Bundle on OS X so that we can hide
+      // the dock icon. See Bug 557225.
+      exePath = exePath.AppendASCII(MOZ_CHILD_PROCESS_BUNDLE);
 #endif
-      }
     }
   }
 
   if (exePath.empty()) {
 #ifdef OS_WIN
     exePath = FilePath::FromWStringHack(CommandLine::ForCurrentProcess()->program());
 #else
     exePath = FilePath(CommandLine::ForCurrentProcess()->argv()[0]);
@@ -221,19 +223,21 @@ nsresult GeckoChildProcessHost::GetArchi
   return NS_ERROR_NOT_IMPLEMENTED;
 #endif
 }
 
 uint32_t GeckoChildProcessHost::GetSupportedArchitecturesForProcessType(GeckoProcessType type)
 {
 #ifdef MOZ_WIDGET_COCOA
   if (type == GeckoProcessType_Plugin) {
+
     // Cache this, it shouldn't ever change.
     static uint32_t pluginContainerArchs = 0;
     if (pluginContainerArchs == 0) {
+      CacheGreDir();
       FilePath exePath;
       GetPathToBinary(exePath);
       nsresult rv = GetArchitecturesForBinary(exePath.value().c_str(), &pluginContainerArchs);
       NS_ASSERTION(NS_SUCCEEDED(rv) && pluginContainerArchs != 0, "Getting architecture of plugin container failed!");
       if (NS_FAILED(rv) || pluginContainerArchs == 0) {
         pluginContainerArchs = base::GetCurrentProcessArchitecture();
       }
     }
@@ -251,16 +255,52 @@ GeckoChildProcessHost::PrepareLaunch()
   if (CrashReporter::GetEnabled()) {
     CrashReporter::OOPInit();
   }
 #endif
 
 #ifdef XP_WIN
   InitWindowsGroupID();
 #endif
+  CacheGreDir();
+}
+
+//static
+void
+GeckoChildProcessHost::CacheGreDir()
+{
+  // PerformAysncLaunchInternal/GetPathToBinary may be called on the IO thread,
+  // and they want to use the directory service, which needs to happen on the
+  // main thread (in the event that its implemented in JS). So we grab
+  // NS_GRE_DIR here and stash it.
+
+#ifdef MOZ_WIDGET_GONK
+  // Apparently, this ASSERT should be present on all platforms. Currently,
+  // this assert causes mochitest failures on the Mac platform if its left in.
+
+  // B2G overrides the directory service in JS, so this needs to be called
+  // on the main thread.
+  MOZ_ASSERT(NS_IsMainThread());
+#endif
+
+  if (ShouldHaveDirectoryService()) {
+    nsCOMPtr<nsIProperties> directoryService(do_GetService(NS_DIRECTORY_SERVICE_CONTRACTID));
+    MOZ_ASSERT(directoryService, "Expected XPCOM to be available");
+    if (directoryService) {
+      // getter_AddRefs doesn't work with StaticRefPtr, so we need to store the
+      // result in an nsCOMPtr and copy it over.
+      nsCOMPtr<nsIFile> greDir;
+      nsresult rv = directoryService->Get(NS_GRE_DIR, NS_GET_IID(nsIFile), getter_AddRefs(greDir));
+      if (NS_SUCCEEDED(rv)) {
+        sGreDir = greDir;
+        mozilla::ClearOnShutdown(&sGreDir);
+      }
+    }
+  }
+  sGreDirCached = true;
 }
 
 #ifdef XP_WIN
 void GeckoChildProcessHost::InitWindowsGroupID()
 {
   // On Win7+, pass the application user model to the child, so it can
   // register with it. This insures windows created by the container
   // properly group with the parent app on the Win7 taskbar.
@@ -462,17 +502,17 @@ AddAppDirToCommandLine(std::vector<std::
       nsCOMPtr<nsIFile> appDir;
       // NS_XPCOM_CURRENT_PROCESS_DIR really means the app dir, not the
       // current process dir.
       nsresult rv = directoryService->Get(NS_XPCOM_CURRENT_PROCESS_DIR,
                                           NS_GET_IID(nsIFile),
                                           getter_AddRefs(appDir));
       if (NS_SUCCEEDED(rv)) {
         nsAutoCString path;
-        appDir->GetNativePath(path);
+        MOZ_ALWAYS_TRUE(NS_SUCCEEDED(appDir->GetNativePath(path)));
 #if defined(XP_WIN)
         aCmdLine.AppendLooseValue(UTF8ToWide("-appdir"));
         aCmdLine.AppendLooseValue(UTF8ToWide(path.get()));
 #else
         aCmdLine.push_back("-appdir");
         aCmdLine.push_back(path.get());
 #endif
       }
@@ -514,61 +554,56 @@ GeckoChildProcessHost::PerformAsyncLaunc
   if (privs == base::PRIVILEGES_DEFAULT) {
     privs = DefaultChildPrivileges();
   }
   // XPCOM may not be initialized in some subprocesses.  We don't want
   // to initialize XPCOM just for the directory service, especially
   // since LD_LIBRARY_PATH is already set correctly in subprocesses
   // (meaning that we don't need to set that up in the environment).
   if (ShouldHaveDirectoryService()) {
-    nsCOMPtr<nsIProperties> directoryService(do_GetService(NS_DIRECTORY_SERVICE_CONTRACTID));
-    NS_ASSERTION(directoryService, "Expected XPCOM to be available");
-    if (directoryService) {
-      nsCOMPtr<nsIFile> greDir;
-      nsresult rv = directoryService->Get(NS_GRE_DIR, NS_GET_IID(nsIFile), getter_AddRefs(greDir));
-      if (NS_SUCCEEDED(rv)) {
-        nsCString path;
-        greDir->GetNativePath(path);
+    MOZ_ASSERT(sGreDirCached);
+    if (sGreDir) {
+      nsCString path;
+      MOZ_ALWAYS_TRUE(NS_SUCCEEDED(sGreDir->GetNativePath(path)));
 # if defined(OS_LINUX) || defined(OS_BSD)
 #  if defined(MOZ_WIDGET_ANDROID)
-        path += "/lib";
+      path += "/lib";
 #  endif  // MOZ_WIDGET_ANDROID
-        const char *ld_library_path = PR_GetEnv("LD_LIBRARY_PATH");
-        nsCString new_ld_lib_path;
-        if (ld_library_path && *ld_library_path) {
-            new_ld_lib_path.Assign(path.get());
-            new_ld_lib_path.AppendLiteral(":");
-            new_ld_lib_path.Append(ld_library_path);
-            newEnvVars["LD_LIBRARY_PATH"] = new_ld_lib_path.get();
-        } else {
-            newEnvVars["LD_LIBRARY_PATH"] = path.get();
-        }
+      const char *ld_library_path = PR_GetEnv("LD_LIBRARY_PATH");
+      nsCString new_ld_lib_path;
+      if (ld_library_path && *ld_library_path) {
+          new_ld_lib_path.Assign(path.get());
+          new_ld_lib_path.AppendLiteral(":");
+          new_ld_lib_path.Append(ld_library_path);
+          newEnvVars["LD_LIBRARY_PATH"] = new_ld_lib_path.get();
+      } else {
+          newEnvVars["LD_LIBRARY_PATH"] = path.get();
+      }
 # elif OS_MACOSX
-        newEnvVars["DYLD_LIBRARY_PATH"] = path.get();
-        // XXX DYLD_INSERT_LIBRARIES should only be set when launching a plugin
-        //     process, and has no effect on other subprocesses (the hooks in
-        //     libplugin_child_interpose.dylib become noops).  But currently it
-        //     gets set when launching any kind of subprocess.
-        //
-        // Trigger "dyld interposing" for the dylib that contains
-        // plugin_child_interpose.mm.  This allows us to hook OS calls in the
-        // plugin process (ones that don't work correctly in a background
-        // process).  Don't break any other "dyld interposing" that has already
-        // been set up by whatever may have launched the browser.
-        const char* prevInterpose = PR_GetEnv("DYLD_INSERT_LIBRARIES");
-        nsCString interpose;
-        if (prevInterpose) {
-          interpose.Assign(prevInterpose);
-          interpose.AppendLiteral(":");
-        }
-        interpose.Append(path.get());
-        interpose.AppendLiteral("/libplugin_child_interpose.dylib");
-        newEnvVars["DYLD_INSERT_LIBRARIES"] = interpose.get();
+      newEnvVars["DYLD_LIBRARY_PATH"] = path.get();
+      // XXX DYLD_INSERT_LIBRARIES should only be set when launching a plugin
+      //     process, and has no effect on other subprocesses (the hooks in
+      //     libplugin_child_interpose.dylib become noops).  But currently it
+      //     gets set when launching any kind of subprocess.
+      //
+      // Trigger "dyld interposing" for the dylib that contains
+      // plugin_child_interpose.mm.  This allows us to hook OS calls in the
+      // plugin process (ones that don't work correctly in a background
+      // process).  Don't break any other "dyld interposing" that has already
+      // been set up by whatever may have launched the browser.
+      const char* prevInterpose = PR_GetEnv("DYLD_INSERT_LIBRARIES");
+      nsCString interpose;
+      if (prevInterpose) {
+        interpose.Assign(prevInterpose);
+        interpose.AppendLiteral(":");
+      }
+      interpose.Append(path.get());
+      interpose.AppendLiteral("/libplugin_child_interpose.dylib");
+      newEnvVars["DYLD_INSERT_LIBRARIES"] = interpose.get();
 # endif  // OS_LINUX
-      }
     }
   }
 #endif  // OS_LINUX || OS_MACOSX
 
   FilePath exePath;
   GetPathToBinary(exePath);
 
 #ifdef MOZ_WIDGET_ANDROID
--- a/ipc/glue/GeckoChildProcessHost.h
+++ b/ipc/glue/GeckoChildProcessHost.h
@@ -6,22 +6,27 @@
 #define __IPC_GLUE_GECKOCHILDPROCESSHOST_H__
 
 #include "base/file_path.h"
 #include "base/process_util.h"
 #include "base/scoped_ptr.h"
 #include "base/waitable_event.h"
 #include "chrome/common/child_process_host.h"
 
+#include "mozilla/DebugOnly.h"
 #include "mozilla/ipc/FileDescriptor.h"
 #include "mozilla/Monitor.h"
+#include "mozilla/StaticPtr.h"
 
+#include "nsCOMPtr.h"
 #include "nsXULAppAPI.h"        // for GeckoProcessType
 #include "nsString.h"
 
+class nsIFile;
+
 namespace mozilla {
 namespace ipc {
 
 class GeckoChildProcessHost : public ChildProcessHost
 {
 protected:
   typedef mozilla::Monitor Monitor;
   typedef std::vector<std::string> StringVector;
@@ -181,24 +186,30 @@ private:
 
   // Does the actual work for AsyncLaunch, on the IO thread.
   bool PerformAsyncLaunchInternal(std::vector<std::string>& aExtraOpts,
                                   base::ProcessArchitecture arch);
 
   bool RunPerformAsyncLaunch(StringVector aExtraOpts=StringVector(),
 			     base::ProcessArchitecture aArch=base::GetCurrentProcessArchitecture());
 
+  static void GetPathToBinary(FilePath& exePath);
+  static void CacheGreDir();
+
   // In between launching the subprocess and handing off its IPC
   // channel, there's a small window of time in which *we* might still
   // be the channel listener, and receive messages.  That's bad
   // because we have no idea what to do with those messages.  So queue
   // them here until we hand off the eventual listener.
   //
   // FIXME/cjones: this strongly indicates bad design.  Shame on us.
   std::queue<IPC::Message> mQueue;
+
+  static StaticRefPtr<nsIFile> sGreDir;
+  static DebugOnly<bool> sGreDirCached;
 };
 
 #ifdef MOZ_NUWA_PROCESS
 class GeckoExistingProcessHost MOZ_FINAL : public GeckoChildProcessHost
 {
 public:
   GeckoExistingProcessHost(GeckoProcessType aProcessType,
                            base::ProcessHandle aProcess,