Bug 1471025: Part 3b - Refactor Android shared FD API to require fewer modifications per change. r=jld
☠☠ backed out by bd747e24063d ☠ ☠
authorKris Maglione <maglione.k@gmail.com>
Mon, 02 Jul 2018 15:01:25 -0700
changeset 426562 4b7a8a35ed956159e2f443c6211164c0cbf3d926
parent 426561 e3bbc87b71af2f2ce1fa8bdf2cf26857c071ba5e
child 426563 38f690f30e78764763bb012045073fa781efa691
push id34275
push usernerli@mozilla.com
push dateFri, 13 Jul 2018 21:53:18 +0000
treeherdermozilla-central@b79457b703d9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjld
bugs1471025
milestone63.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 1471025: Part 3b - Refactor Android shared FD API to require fewer modifications per change. r=jld Adding or removing an FD from this API currently requires changes in about a half dozen places. Ignoring the Java side of things. This patch changes the API to pass a struct, rather than additional arguments for each FD, so that adding and removing FDs only requires changing one declaration, and the two call sites that add and consume the FDs. MozReview-Commit-ID: CToSEVp1oqP
mozglue/android/APKOpen.cpp
toolkit/xre/Bootstrap.cpp
toolkit/xre/Bootstrap.h
toolkit/xre/nsEmbedFunctions.cpp
xpcom/build/nsXULAppAPI.h
--- a/mozglue/android/APKOpen.cpp
+++ b/mozglue/android/APKOpen.cpp
@@ -403,17 +403,17 @@ Java_org_mozilla_gecko_mozglue_GeckoLoad
       FreeArgv(argv, argc);
       return;
     }
 
     ElfLoader::Singleton.ExpectShutdown(false);
     gBootstrap->GeckoStart(jenv, argv, argc, sAppData);
     ElfLoader::Singleton.ExpectShutdown(true);
   } else {
-    gBootstrap->XRE_SetAndroidChildFds(jenv, prefsFd, ipcFd, crashFd, crashAnnotationFd);
+    gBootstrap->XRE_SetAndroidChildFds(jenv, { prefsFd, ipcFd, crashFd, crashAnnotationFd });
     gBootstrap->XRE_SetProcessType(argv[argc - 1]);
 
     XREChildData childData;
     gBootstrap->XRE_InitChildProcess(argc - 1, argv, &childData);
   }
 
   gBootstrap.reset();
   FreeArgv(argv, argc);
--- a/toolkit/xre/Bootstrap.cpp
+++ b/toolkit/xre/Bootstrap.cpp
@@ -73,18 +73,18 @@ public:
     ::XRE_EnableSameExecutableForContentProc();
   }
 
 #ifdef MOZ_WIDGET_ANDROID
   virtual void GeckoStart(JNIEnv* aEnv, char** argv, int argc, const StaticXREAppData& aAppData) override {
     ::GeckoStart(aEnv, argv, argc, aAppData);
   }
 
-  virtual void XRE_SetAndroidChildFds(JNIEnv* aEnv, int aPrefsFd, int aIPCFd, int aCrashFd, int aCrashAnnotationFd) override {
-    ::XRE_SetAndroidChildFds(aEnv, aPrefsFd, aIPCFd, aCrashFd, aCrashAnnotationFd);
+  virtual void XRE_SetAndroidChildFds(JNIEnv* aEnv, const XRE_AndroidChildFds& aFds) override {
+    ::XRE_SetAndroidChildFds(aEnv, aFds);
   }
 #endif
 
 #ifdef LIBFUZZER
   virtual void XRE_LibFuzzerSetDriver(LibFuzzerDriver aDriver) override {
     ::XRE_LibFuzzerSetDriver(aDriver);
   }
 #endif
--- a/toolkit/xre/Bootstrap.h
+++ b/toolkit/xre/Bootstrap.h
@@ -108,17 +108,17 @@ public:
 
   virtual nsresult XRE_InitChildProcess(int argc, char* argv[], const XREChildData* aChildData) = 0;
 
   virtual void XRE_EnableSameExecutableForContentProc() = 0;
 
 #ifdef MOZ_WIDGET_ANDROID
   virtual void GeckoStart(JNIEnv* aEnv, char** argv, int argc, const StaticXREAppData& aAppData) = 0;
 
-  virtual void XRE_SetAndroidChildFds(JNIEnv* aEnv, int aPrefsFd, int aIPCFd, int aCrashFd, int aCrashAnnotationFd) = 0;
+  virtual void XRE_SetAndroidChildFds(JNIEnv* aEnv, const XRE_AndroidChildFds& fds) = 0;
 #endif
 
 #ifdef LIBFUZZER
   virtual void XRE_LibFuzzerSetDriver(LibFuzzerDriver aDriver) = 0;
 #endif
 
 #ifdef MOZ_IPDL_TESTS
   virtual int XRE_RunIPDLTest(int argc, char **argv) = 0;
--- a/toolkit/xre/nsEmbedFunctions.cpp
+++ b/toolkit/xre/nsEmbedFunctions.cpp
@@ -239,23 +239,23 @@ XRE_ChildProcessTypeToString(GeckoProces
 namespace mozilla {
 namespace startup {
 GeckoProcessType sChildProcessType = GeckoProcessType_Default;
 } // namespace startup
 } // namespace mozilla
 
 #if defined(MOZ_WIDGET_ANDROID)
 void
-XRE_SetAndroidChildFds (JNIEnv* env, int prefsFd, int ipcFd, int crashFd, int crashAnnotationFd)
+XRE_SetAndroidChildFds (JNIEnv* env, const XRE_AndroidChildFds& fds)
 {
   mozilla::jni::SetGeckoThreadEnv(env);
-  mozilla::dom::SetPrefsFd(prefsFd);
-  IPC::Channel::SetClientChannelFd(ipcFd);
-  CrashReporter::SetNotificationPipeForChild(crashFd);
-  CrashReporter::SetCrashAnnotationPipeForChild(crashAnnotationFd);
+  mozilla::dom::SetPrefsFd(fds.mPrefsFd);
+  IPC::Channel::SetClientChannelFd(fds.mIpcFd);
+  CrashReporter::SetNotificationPipeForChild(fds.mCrashFd);
+  CrashReporter::SetCrashAnnotationPipeForChild(fds.mCrashAnnotationFd);
 }
 #endif // defined(MOZ_WIDGET_ANDROID)
 
 void
 XRE_SetProcessType(const char* aProcessTypeString)
 {
   static bool called = false;
   if (called) {
--- a/xpcom/build/nsXULAppAPI.h
+++ b/xpcom/build/nsXULAppAPI.h
@@ -392,18 +392,26 @@ static const char* const kGeckoProcessTy
 static_assert(MOZ_ARRAY_LENGTH(kGeckoProcessTypeString) ==
               GeckoProcessType_End,
               "Array length mismatch");
 
 XRE_API(const char*,
         XRE_ChildProcessTypeToString, (GeckoProcessType aProcessType))
 
 #if defined(MOZ_WIDGET_ANDROID)
+struct XRE_AndroidChildFds
+{
+  int mPrefsFd;
+  int mIpcFd;
+  int mCrashFd;
+  int mCrashAnnotationFd;
+};
+
 XRE_API(void,
-        XRE_SetAndroidChildFds, (JNIEnv* env, int prefsFd, int ipcFd, int crashFd, int crashAnnotationFd))
+        XRE_SetAndroidChildFds, (JNIEnv* env, const XRE_AndroidChildFds& fds))
 #endif // defined(MOZ_WIDGET_ANDROID)
 
 XRE_API(void,
         XRE_SetProcessType, (const char* aProcessTypeString))
 
 // Used in the "master" parent process hosting the crash server
 XRE_API(bool,
         XRE_TakeMinidumpForChild, (uint32_t aChildPid, nsIFile** aDump,