Bug 1471025: Part 3b - Refactor Android shared FD API to require fewer modifications per change. r=jld
☠☠ backed out by 04458cadaaa1 ☠ ☠
authorKris Maglione <maglione.k@gmail.com>
Mon, 02 Jul 2018 15:01:25 -0700
changeset 426615 ff41551f5ff1b98b72ed771a6f2a3f66a8b79a57
parent 426614 46a6f9d0773ba2d756d8801cee79bfa994165d44
child 426616 7c03b7dd00e9675f9ac045ed1ea733eb0486904f
push id34278
push useraciure@mozilla.com
push dateSun, 15 Jul 2018 09:53:15 +0000
treeherdermozilla-central@2a8f94a45fd3 [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,