Bug 1471025: Part 3b - Refactor Android shared FD API to require fewer modifications per change. r?jld draft
authorKris Maglione <maglione.k@gmail.com>
Mon, 02 Jul 2018 15:01:25 -0700
changeset 815375 2e574a3f1c4234758d9dd2f6b222f64c3aeb1971
parent 815374 778ba8b508184b21c746449cc9740438a0d3b7ed
child 815376 7413e7fd2f52bf837910bdb00154ae84d6844368
push id115503
push usermaglione.k@gmail.com
push dateSat, 07 Jul 2018 19:50:37 +0000
reviewersjld
bugs1471025
milestone63.0a1
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 comsume 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,