Upstream: Renderers are running in WebAPKs.

content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java

-// Copyright 2012 The Chromium Authors. All rights reserved.
+// Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.content.app;
 
-import android.app.Service;
 import android.content.Context;
 import android.content.Intent;
 import android.graphics.SurfaceTexture;
 import android.os.Bundle;
 import android.os.IBinder;
 import android.os.Parcelable;
 import android.os.Process;
 import android.os.RemoteException;
 import android.view.Surface;
 
 import org.chromium.base.BaseSwitches;
 import org.chromium.base.CommandLine;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.library_loader.LibraryLoader;
 import org.chromium.base.library_loader.Linker;
 import org.chromium.base.library_loader.ProcessInitException;
 import org.chromium.content.browser.ChildProcessConstants;
 import org.chromium.content.browser.ChildProcessCreationParams;
-import org.chromium.content.browser.ChildProcessLauncher;
 import org.chromium.content.browser.FileDescriptorInfo;
 import org.chromium.content.common.ContentSwitches;
 import org.chromium.content.common.IChildProcessCallback;
 import org.chromium.content.common.IChildProcessService;
 import org.chromium.content.common.SurfaceWrapper;
 
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.atomic.AtomicReference;
 
 /**
- * This is the base class for child services; the [Non]SandboxedProcessService0, 1.. etc
- * subclasses provide the concrete service entry points, to enable the browser to connect
- * to more than one distinct process (i.e. one process per service number, up to limit of N).
- * The embedding application must declare these service instances in the application section
- * of its AndroidManifest.xml, for example with N entries of the form:-
- *     <service android:name="org.chromium.content.app.[Non]SandboxedProcessServiceX"
- *              android:process=":[non]sandboxed_processX" />
- * for X in 0...N-1 (where N is {@link ChildProcessLauncher#MAX_REGISTERED_SERVICES})
+ * This class provides all the implementations for {@link ChildProcessService} which owns an

[pkotwicz, 2016/06/14 01:19:48]

How about: "This class implements all of the functionality for {@link
ChildProcessService} which ..."

[Xi Han, 2016/06/14 21:55:04]

Done.

+ * object of {@link ChildProcessServiceImpl}.
+ * It makes possible that WebAPK's ChildProcessService owns a ChildProcessServiceImpl object
+ * and uses the same functionalities to create renderer process for WebAPKs when "--enable-webapk"
+ * flag is turned on.
  */
 @JNINamespace("content")
 @SuppressWarnings("SynchronizeOnNonFinalField")
-public class ChildProcessService extends Service {
+public class ChildProcessServiceImpl {
     private static final String MAIN_THREAD_NAME = "ChildProcessMain";
     private static final String TAG = "ChildProcessService";
     protected static final FileDescriptorInfo[] EMPTY_FILE_DESCRIPTOR_INFO = {};
     private IChildProcessCallback mCallback;
 
     // This is the native "Main" thread for the renderer / utility process.
     private Thread mMainThread;
     // Parameters received via IPC, only accessed while holding the mMainThread monitor.
     private String[] mCommandLineParams;
     private int mCpuCount;
     private long mCpuFeatures;
     // File descriptors that should be registered natively.
     private FileDescriptorInfo[] mFdInfos;
     // Linker-specific parameters for this child process service.
     private ChromiumLinkerParams mLinkerParams;
     // Child library process type.
     private int mLibraryProcessType;
 
     private static AtomicReference<Context> sContext = new AtomicReference<Context>(null);
     private boolean mLibraryInitialized = false;
     // Becomes true once the service is bound. Access must synchronize around mMainThread.
     private boolean mIsBound = false;
 
     private final Semaphore mActivitySemaphore = new Semaphore(1);
-
-    // Return a Linker instance. If testing, the Linker needs special setup.
-    private Linker getLinker() {
-        if (Linker.areTestsEnabled()) {
-            // For testing, set the Linker implementation and the test runner
-            // class name to match those used by the parent.
-            assert mLinkerParams != null;
-            Linker.setupForTesting(
-                    mLinkerParams.mLinkerImplementationForTesting,
-                    mLinkerParams.mTestRunnerClassNameForTesting);
-        }
-        return Linker.getInstance();
-    }
-
-    // Binder object used by clients for this service.
     private final IChildProcessService.Stub mBinder = new IChildProcessService.Stub() {
         // NOTE: Implement any IChildProcessService methods here.
         @Override
         public int setupConnection(Bundle args, IChildProcessCallback callback) {
             mCallback = callback;
             getServiceInfo(args);
             return Process.myPid();
         }
 
         @Override
         public void crashIntentionallyForTesting() {
             Process.killProcess(Process.myPid());
         }
     };
 

[pkotwicz, 2016/06/14 01:19:48]

Can you please add a comment to |mClassLoader| and maybe rename the variable to
|mHostClassLoader|?

It is not clear which context |mClassLoader| is for.

[Xi Han, 2016/06/14 21:55:03]

Done.

+    private ClassLoader mClassLoader;
+
+    // Return a Linker instance. If testing, the Linker needs special setup.

[pkotwicz, 2016/06/14 01:19:48]

Nit: Can you move this function so that gerrit does not think that this is a new
function?

[Xi Han, 2016/06/14 21:55:03]

Done.

+    private Linker getLinker() {
+        if (Linker.areTestsEnabled()) {
+            // For testing, set the Linker implementation and the test runner
+            // class name to match those used by the parent.
+            assert mLinkerParams != null;
+            Linker.setupForTesting(
+                    mLinkerParams.mLinkerImplementationForTesting,
+                    mLinkerParams.mTestRunnerClassNameForTesting);
+        }
+        return Linker.getInstance();
+    }
+
     /* package */ static Context getContext() {

[pkotwicz, 2016/06/14 01:19:48]

This function seems unused. I posted a CL to remove it in
https://codereview.chromium.org/2061743003/ We'll find out if the existence of
the function is a hack

[Xi Han, 2016/06/14 21:55:04]

Thanks for creating the clean up CL!

[pkotwicz, 2016/06/15 14:13:50]

It looks like dtrainor@ is at Blimpcon. We'll probably not get a response till
Thursday

         return sContext.get();
     }
 
-    @Override
-    public void onCreate() {
+    /**
+     * Loads Chrome's native libraries and initializes a ChildProcessServiceImapl.
+     * @param context The application context.
+     * @param hostBrowserContext The context of the Host Browser (i.e. Chrome).
+     */
+    public void create(final Context context, final Context hostBrowserContext) {
+        mClassLoader = hostBrowserContext.getClassLoader();
         Log.i(TAG, "Creating new ChildProcessService pid=%d", Process.myPid());
         if (sContext.get() != null) {
             throw new RuntimeException("Illegal child process reuse.");
         }
-        sContext.set(this);
-        super.onCreate();
+        sContext.set(context);
 

[pkotwicz, 2016/06/14 01:19:48]

Can you please add a comment about which context to use. To my surprise deleting
line 119 does not cause a crash when a renderer is launched. I am pretty sure
that the call is necessary, however I do not know why

[Xi Han, 2016/06/14 21:55:03]

Done.

-        ContextUtils.initApplicationContext(getApplicationContext());
+        ContextUtils.initApplicationContext(context);
 
         mMainThread = new Thread(new Runnable() {
             @Override
             @SuppressFBWarnings("DM_EXIT")
             public void run()  {
                 try {
                     // CommandLine must be initialized before everything else.
                     synchronized (mMainThread) {
                         while (mCommandLineParams == null) {
                             mMainThread.wait();
@@ skipping 18 matching lines @@
                         }
                     }
                     boolean isLoaded = false;
                     if (CommandLine.getInstance().hasSwitch(
                             BaseSwitches.RENDERER_WAIT_FOR_JAVA_DEBUGGER)) {
                         android.os.Debug.waitForDebugger();
                     }
 
                     boolean loadAtFixedAddressFailed = false;
                     try {
-                        LibraryLoader.get(mLibraryProcessType).loadNow(getApplicationContext());
+                        LibraryLoader.get(mLibraryProcessType).loadNow(hostBrowserContext);
                         isLoaded = true;
                     } catch (ProcessInitException e) {
                         if (requestedSharedRelro) {
                             Log.w(TAG, "Failed to load native library with shared RELRO, "
                                     + "retrying without");
                             loadAtFixedAddressFailed = true;
                         } else {
                             Log.e(TAG, "Failed to load native library", e);
                         }
                     }
                     if (!isLoaded && requestedSharedRelro) {
                         linker.disableSharedRelros();
                         try {
-                            LibraryLoader.get(mLibraryProcessType).loadNow(getApplicationContext());
+                            LibraryLoader.get(mLibraryProcessType).loadNow(context);

[pkotwicz, 2016/06/14 01:19:48]

Should this be |hostBrowserContext|? (I am unsure if this is the code that you
told me about before leaving for the day)

[Xi Han, 2016/06/14 21:55:04]

It looks like we should use |hostBrowserContext| too, but I don't know why we
didn't update here before. Updated.

It's not the code that I told you. Actually, the changes that I mentioned
already in, I might just looked at a wrong branch. Sorry for the misleading.

                             isLoaded = true;
                         } catch (ProcessInitException e) {
                             Log.e(TAG, "Failed to load native library on retry", e);
                         }
                     }
                     if (!isLoaded) {
                         System.exit(-1);
                     }
                     LibraryLoader.get(mLibraryProcessType)
                             .registerRendererProcessHistogram(requestedSharedRelro,
                                     loadAtFixedAddressFailed);
                     LibraryLoader.get(mLibraryProcessType).initialize();
                     synchronized (mMainThread) {
                         mLibraryInitialized = true;
                         mMainThread.notifyAll();
                         while (mFdInfos == null) {
                             mMainThread.wait();
                         }
                     }
                     for (FileDescriptorInfo fdInfo : mFdInfos) {
                         nativeRegisterGlobalFileDescriptor(
                                 fdInfo.mId, fdInfo.mFd.detachFd(), fdInfo.mOffset, fdInfo.mSize);
                     }
-                    nativeInitChildProcess(ChildProcessService.this, mCpuCount, mCpuFeatures);
+                    nativeInitChildProcessImpl(ChildProcessServiceImpl.this, mCpuCount,
+                            mCpuFeatures);
                     if (mActivitySemaphore.tryAcquire()) {
                         ContentMain.start();
                         nativeExitChildProcess();
                     }
                 } catch (InterruptedException e) {
                     Log.w(TAG, "%s startup failed: %s", MAIN_THREAD_NAME, e);
                 } catch (ProcessInitException e) {
                     Log.w(TAG, "%s startup failed: %s", MAIN_THREAD_NAME, e);
                 }
             }
         }, MAIN_THREAD_NAME);
         mMainThread.start();
     }
 
-    @Override
     @SuppressFBWarnings("DM_EXIT")
-    public void onDestroy() {
+    public void destroy() {
         Log.i(TAG, "Destroying ChildProcessService pid=%d", Process.myPid());
-        super.onDestroy();
         if (mActivitySemaphore.tryAcquire()) {
             // TODO(crbug.com/457406): This is a bit hacky, but there is no known better solution
             // as this service will get reused (at least if not sandboxed).
             // In fact, we might really want to always exit() from onDestroy(), not just from
             // the early return here.
             System.exit(0);
             return;
         }
         synchronized (mMainThread) {
             try {
                 while (!mLibraryInitialized) {
                     // Avoid a potential race in calling through to native code before the library
                     // has loaded.
                     mMainThread.wait();
                 }
             } catch (InterruptedException e) {
                 // Ignore
             }
         }
         // Try to shutdown the MainThread gracefully, but it might not
         // have chance to exit normally.
         nativeShutdownMainThread();
     }
 
-    @Override
-    public IBinder onBind(Intent intent) {
-        // We call stopSelf() to request that this service be stopped as soon as the client
-        // unbinds. Otherwise the system may keep it around and available for a reconnect. The
-        // child processes do not currently support reconnect; they must be initialized from
-        // scratch every time.
-        stopSelf();
+    public IBinder bind(Intent intent) {
         initializeParams(intent);
         return mBinder;
     }
 
     /**
      * Helper method to initialize the params from intent.
      * @param intent Intent to launch the service.
      */
     protected void initializeParams(Intent intent) {
         synchronized (mMainThread) {
             mCommandLineParams =
                     intent.getStringArrayExtra(ChildProcessConstants.EXTRA_COMMAND_LINE);
             // mLinkerParams is never used if Linker.isUsed() returns false.
             // See onCreate().
             mLinkerParams = new ChromiumLinkerParams(intent);
             mLibraryProcessType = ChildProcessCreationParams.getLibraryProcessType(intent);
             mIsBound = true;
             mMainThread.notifyAll();
         }
     }
 
     /**
      * Helper method to get the information about the service from a given bundle.
      * @param bundle Bundle that contains the information to start the service.
      */
     void getServiceInfo(Bundle bundle) {
         // Required to unparcel FileDescriptorInfo.
-        bundle.setClassLoader(getClassLoader());
+        bundle.setClassLoader(mClassLoader);
         synchronized (mMainThread) {
             // Allow the command line to be set via bind() intent or setupConnection, but
             // the FD can only be transferred here.
             if (mCommandLineParams == null) {
                 mCommandLineParams =
                         bundle.getStringArray(ChildProcessConstants.EXTRA_COMMAND_LINE);
             }
             // We must have received the command line by now
             assert mCommandLineParams != null;
             mCpuCount = bundle.getInt(ChildProcessConstants.EXTRA_CPU_COUNT);
@@ skipping 134 matching lines @@
      * This includes the IPC channel, the crash dump signals and resource related
      * files.
      */
     private static native void nativeRegisterGlobalFileDescriptor(
             int id, int fd, long offset, long size);
 
     /**
      * The main entry point for a child process. This should be called from a new thread since
      * it will not return until the child process exits. See child_process_service.{h,cc}
      *
-     * @param service The current ChildProcessService object.
+     * @param serviceImpl The current ChildProcessServiceImpl object.
      * renderer.
      */
-    private static native void nativeInitChildProcess(
-            ChildProcessService service, int cpuCount, long cpuFeatures);
+    private static native void nativeInitChildProcessImpl(
+            ChildProcessServiceImpl serviceImpl, int cpuCount, long cpuFeatures);
 
     /**
      * Force the child process to exit.
      */
     private static native void nativeExitChildProcess();
 
     private native void nativeShutdownMainThread();
 }